Skip to content

Commit 0d2b2f2

Browse files
committed
Stabilize VarDecl::isLet for ParamDecls
For ParamDecl instances, the value of this property is not just a function of the introducer (let/var which is a poorly-defined concept for parameters), it's a function of the specifier (inout/__owned/__shared etc). However, computing the specifier also has the side effect of flipping the introducer bits. This appears to be because while the AST uses `isLet` in a syntactic sense "did the user write 'let'?", SIL uses it in a semantic sense "is this property semantically immutable?". These two queries need to be split from one another and the callers migrated. But that is a much larger task for a later time. For now, provide the value of `ParamDecl::isImmutable` to callers since that's the more conservative of the two behaviors. The bug here is that it's possible for `getSpecifier` to *not* be called before `isLet` is called (usually in SIL). This manifested as a test output divergence on the non-asserts bots since the ASTVerifier was always calling getSpecifier, and most engineers do not build without asserts on at their desk. rdar://89237318
1 parent 2d256cb commit 0d2b2f2

File tree

4 files changed

+19
-7
lines changed

4 files changed

+19
-7
lines changed

include/swift/AST/Decl.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5248,9 +5248,8 @@ class VarDecl : public AbstractStorageDecl {
52485248

52495249
/// Is this an immutable 'let' property?
52505250
///
5251-
/// If this is a ParamDecl, isLet() is true iff
5252-
/// getSpecifier() == Specifier::Default.
5253-
bool isLet() const { return getIntroducer() == Introducer::Let; }
5251+
/// For \c ParamDecl instances, using \c isImmutable is preferred.
5252+
bool isLet() const;
52545253

52555254
/// Is this an "async let" property?
52565255
bool isAsyncLet() const;

lib/AST/Decl.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6373,6 +6373,19 @@ bool VarDecl::isMemberwiseInitialized(bool preferDeclaredProperties) const {
63736373
return true;
63746374
}
63756375

6376+
bool VarDecl::isLet() const {
6377+
// An awful hack that stabilizes the value of 'isLet' for ParamDecl instances.
6378+
//
6379+
// All of the callers in SIL are actually looking for the semantic
6380+
// "is immutable" predicate (present on ParamDecl) and should be migrated to
6381+
// a high-level request. Once this is done, all callers of the introducer and
6382+
// specifier setters can be removed.
6383+
if (auto *PD = dyn_cast<ParamDecl>(this)) {
6384+
return PD->isImmutable();
6385+
}
6386+
return getIntroducer() == Introducer::Let;
6387+
}
6388+
63766389
bool VarDecl::isAsyncLet() const {
63776390
return getAttrs().hasAttribute<AsyncAttr>();
63786391
}

test/DebugInfo/debug_value_addr.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func use<T>(_ t : T) {}
2828
// CHECK-SIL: sil hidden @$s16debug_value_addr11GenericSelfV1xACyxGx_tcfC : $@convention(method) <T> (@in T, @thin GenericSelf<T>.Type) -> GenericSelf<T> {
2929
// CHECK-SIL: bb0(%0 : $*T, %1 : $@thin GenericSelf<T>.Type):
3030
//
31-
// CHECK-SIL-NEXT: alloc_stack [lexical] $GenericSelf<T>, {{var|let}}, name "self", implicit, loc {{.*}}
31+
// CHECK-SIL-NEXT: alloc_stack [lexical] $GenericSelf<T>, var, name "self", implicit, loc {{.*}}
3232
// CHECK-SIL-NEXT: debug_value %0 : $*T, let, name "x", argno 1, expr op_deref, loc {{.*}}
3333
struct GenericSelf<T> {
3434
init(x: T) {

test/SILOptimizer/move_function_kills_copyable_values.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,10 @@ public struct Pair {
227227
// have invalidated a part of pair. We can be less restrictive in the future.
228228
//
229229
// TODO: Why are we emitting two uses here.
230-
public func performMoveOnOneEltOfPair(_ p: __owned Pair) {
230+
public func performMoveOnOneEltOfPair(_ p: __owned Pair) { // expected-error {{'p' used after being moved}}
231231
let _ = p.z
232-
let _ = _move(p.x) // expected-error {{_move applied to value that the compiler does not support checking}}
233-
nonConsumingUse(p.y)
232+
let _ = _move(p.x) // expected-note {{move here}}
233+
nonConsumingUse(p.y) // expected-note 2 {{use here}}
234234
}
235235

236236
public class KlassPair {

0 commit comments

Comments
 (0)