Replace dependsOn with @lifetime #76834
Conversation
meg-gupta
force-pushed
the
lifetimedepmultiple
branch
5 times, most recently
from
53895b6 to
3a82b2c
Compare
|
@swift-ci test |
meg-gupta
force-pushed
the
lifetimedepmultiple
branch
from
3a82b2c to
dd27ef6
Compare
|
@swift-ci test |
meg-gupta
force-pushed
the
lifetimedepmultiple
branch
4 times, most recently
from
03843d4 to
cb1f841
Compare
|
@swift-ci test |
Lifetime dependencies will now be represented with @Lifetime attribute in the language. dependsOn is a type modifier and was represented as a LifetimeDependentTypeRepr in the AST. I am deleting dependsOn syntax parsing support and retaining LifetimeDependentTypeRepr support. We may want to represent lifetime dependencies in a function type with a type attribute in the future. If we use a decl attribute instead, then support for LifetimeDependentTypeRepr can be deleted.
meg-gupta
force-pushed
the
lifetimedepmultiple
branch
from
cb1f841 to
5e88075
Compare
|
@swift-ci test |
meg-gupta
requested review from
xymus,
hborla,
slavapestov,
xedin,
eeckstein,
jckarter,
ahoppen,
bnbarham,
CodaFi,
DougGregor,
hamishknight,
rintaro and
zoecarver
as code owners
meg-gupta
requested review from
hyp,
egorzhdan,
Xazax-hun,
AnthonyLatsis and
tshortli
as code owners
lib/AST/LifetimeDependence.cpp
Outdated
| loc, diag::lifetime_dependence_cannot_use_inferred_scoped_consuming); | ||
| return LifetimeDependenceKind::Scope; | ||
| } | ||
| if (type->isEscapable() && !isBitwiseCopyable(type, ctx)) { |
There was a problem hiding this comment.
There's an !isBitwiseCopyable check here that didn't used to be. I'm not sure why. We cannot inherit a lifetime from a BitwiseCopyable thing.
lib/Parse/ParseDecl.cpp
Outdated
| auto lParenLoc = consumeToken(); | ||
|
|
||
| std::optional<LifetimeDescriptor> targetDescriptor; | ||
| if (Tok.isAny(tok::identifier, tok::integer_literal, tok::kw_self) && |
There was a problem hiding this comment.
I just want to clarify. Is @lifetime(self: something) and @lifetime(1: something) considered valid syntax? Because the latter won’t parse in the new parser since 1 is not a valid argument label.
There was a problem hiding this comment.
We were planning to use integer indices in the case of dependsOn especially for function types which may not have parameter names. @lifetime(1: 0) doesn't read as well as a positional dependsOn(0) would. We likely can delete support for integer target indices. SIL tests still use the positional @lifetime with integer index for source. I'll follow with a separate PR for this.
There was a problem hiding this comment.
I would be in favor of deleting integer target indices.
|
|
||
| if (!Tok.isFollowingLParen()) { | ||
| diagnose(Tok, diag::expected_lparen_after_lifetime_dependence); | ||
| if (!Tok.isFollowingLParen()) { |
There was a problem hiding this comment.
Could you disallow spaces between the attribute name and the (, similar to what I did in #71237?
There was a problem hiding this comment.
Ok, I am now using consumeAttributeLParen()
@Lifetime(target: source1, source2...) where target can be any parameter or 'self'. We cannot have @Lifetime attributes with duplicate targets. Also, update the internal data structures. Previously LifetimeEntry stored pairwise (target, source) dependencies. Now, LifetimeEntry will store an optional target descriptor and an array of source descriptors.
Lifetime dependencies in SIL tests continue to be represented as a type modifier on the target. As before, they are represented as a LifetimeDependentTypeRepr in the AST.
meg-gupta
force-pushed
the
lifetimedepmultiple
branch
from
5e88075 to
2711434
Compare
SIL's @Lifetime continues to be positional. Don't parse 'target:' in this case.
|
@ahoppen swiftlang/swift-syntax#2876 deletes |
|
@swift-ci test |
Adds support for specifying the target for lifetime dependence with :
@lifetime(target: source1...source)Delete
dependsOnsyntaxUpdate all tests with
dependsOnwith@lifetime