Update spelling for representing lifetime dependencies to @_lifetime #82067
Conversation
meg-gupta
force-pushed
the
lifetimeunderscored
branch
from
0262cd8 to
2835df3
Compare
meg-gupta
requested review from
xymus,
hborla,
slavapestov,
xedin,
kavon,
eeckstein,
jckarter,
bnbarham,
CodaFi,
DougGregor,
hamishknight,
rintaro,
hnrklssn,
Xazax-hun,
zoecarver,
egorzhdan,
j-hui,
fahadnayyar,
susmonteiro,
AnthonyLatsis and
rjmccall
as code owners
meg-gupta
force-pushed
the
lifetimeunderscored
branch
from
2835df3 to
a773ce3
Compare
meg-gupta
force-pushed
the
lifetimeunderscored
branch
from
a773ce3 to
854ff84
Compare
meg-gupta
force-pushed
the
lifetimeunderscored
branch
5 times, most recently
from
3d565da to
ff11d4c
Compare
|
@swift-ci test |
meg-gupta
force-pushed
the
lifetimeunderscored
branch
from
ff11d4c to
74e4c2e
Compare
|
@swift-ci test |
|
@swift-ci test macOS platform |
There was a problem hiding this comment.
C++ interop specific parts mostly look good to me. I have one question about backward compatibility in the SwiftifyImport macro. The rest of the questions are independent of this PR. I'd like @egorzhdan to take a look at the modules changes as he is more familiar with that part.
| if (!attr->isUnderscored()) { | ||
| // Allow @lifetime only in the stdlib, cxx and backward compatibility | ||
| // modules under -enable-experimental-feature LifetimeDependence | ||
| if (!Ctx.MainModule->isStdlibModule() && !Ctx.MainModule->isCxxModule() && |
There was a problem hiding this comment.
@egorzhdan Could you double check if this makes sense to you? (Including the recently added isCxxModule.)
There was a problem hiding this comment.
Note that this check is meant to cover @lifetime uses in CxxSpan.swift/ CxxVector.swift etc that may end up in the swiftinterface files.
| void AttributeChecker::visitLifetimeAttr(LifetimeAttr *attr) {} | ||
| void AttributeChecker::visitLifetimeAttr(LifetimeAttr *attr) { | ||
| if (!attr->isUnderscored()) { | ||
| // Allow @lifetime only in the stdlib, cxx and backward compatibility |
There was a problem hiding this comment.
Do we have a stable release with @lifetime annotated code in stdlib/cxx modules? Or is this a temporary workaround for early adopters?
There was a problem hiding this comment.
We want early adopters to use @_lifetime instead of @lifetime. stdlib will continue using @lifetime in the short term.
There was a problem hiding this comment.
release/6.2 will be the first stable release with lifetime dependencies as a supported experimental feature.
|
|
||
| // RUN: %target-swift-ide-test -print-module -module-to-print=CountedByLifetimeboundClang -plugin-path %swift-plugin-dir -I %S/Inputs -source-filename=x -enable-experimental-feature SafeInteropWrappers -Xcc -Wno-nullability-completeness | %FileCheck %s | ||
|
|
||
| // swift-ide-test doesn't currently typecheck the macro expansions, so run the compiler as well | ||
| // RUN: %empty-directory(%t) | ||
| // RUN: %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -o %t/CountedByLifetimebound.swiftmodule -I %S/Inputs -enable-experimental-feature SafeInteropWrappers -enable-experimental-feature LifetimeDependence -strict-memory-safety -warnings-as-errors -Xcc -Werror -Xcc -Wno-nullability-completeness %s | ||
| // RUN: %target-swift-frontend -emit-module -plugin-path %swift-plugin-dir -o %t/CountedByLifetimebound.swiftmodule -I %S/Inputs -enable-experimental-feature SafeInteropWrappers -enable-experimental-feature Lifetimes -strict-memory-safety -warnings-as-errors -Xcc -Werror -Xcc -Wno-nullability-completeness %s |
There was a problem hiding this comment.
@hnrklssn Do we actually need Lifetimes/LifetimeDependence feature enabled for this to work? I think we decided that this should work regardless, so if that is the case I think we should remove this from tests. If not, we should decide if this is intentional or want to treat it as a bug.
There was a problem hiding this comment.
The macro expansions themselves are fine without it, but I think at one point it was needed for the functions we use to call the overloads to be able to use inout MutableSpan. That may have been relaxed since then, but I'm not 100% on that
| @@ -1445,7 +1445,7 @@ func containsLifetimeAttr(_ attrs: AttributeListSyntax, for paramName: TokenSynt | |||
| guard let attr = elem.as(AttributeSyntax.self) else { | |||
| continue | |||
| } | |||
| if attr.attributeName != "lifetime" { | |||
| if attr.attributeName != "_lifetime" { | |||
There was a problem hiding this comment.
I see traces of still supporting @lifetime in some restricted scenarios in this PR. This makes me wonder if this is also a codepath that should at least temporarily support both @lifetime and @_lifetime.
There was a problem hiding this comment.
This codepath does not seem to be exercised in the stdlib/cxx modules - but only in case we are importing C++ as Swift in which we want @_lifetime instead of @lifetime.
cc @hnrklssn
|
@swift-ci test macOS platform |
LifetimeDependence is a supported experimental feature for which we have to maintain source compatibility. During language evolution if we decide to update its semantics or syntax under
@lifetimeattribute, it can introduce significant confusion and maintenance problems to both early adopters and compiler developers.To mitigate this, this PR introduces an underscore, effectively making the annotation
@_lifetime, to provide a clear distinction and avoid potential conflicts during future development. Standard library will continue to use@lifetimein the near term.@_lifetimeis implemented as a suppressible language feature and@lifetimeis printed in the suppressed path since the oldest compilers we have to support with a new sdk have@lifetimesupport. This will ensure when the standard library migrates to@_lifetimewe will continue to support older compiler.rdar://151901744