[SR-2757][Sema] Mark VarDecl in capture lists #6490
Conversation
| @@ -22,6 +22,11 @@ var closure6 = $0 // expected-error {{anonymous closure argument not contained | |||
| var closure7 : Int = | |||
| { 4 } // expected-error {{function produces expected type 'Int'; did you mean to call it with '()'?}} {{9-9=()}} | |||
|
|
|||
| var capturedVariable = 1 | |||
| var closure8 = { [closureVariable] in | |||
| capturedVariable += 1 // expected-error {{left side of mutating operator isn't mutable: 'capturedVariable' is an immutable capture list variable}} | |||
There was a problem hiding this comment.
How about just "is an immutable capture"?
|
Done! Thanks, @slavapestov!! Can't wait for CI to reopen, and for my first commit to Sema :) |
| @@ -3750,7 +3750,7 @@ void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const { | |||
| // since the user has to choose to apply this anyway. | |||
| if (auto *PBD = getParentPatternBinding()) { | |||
| // Don't touch generated or invalid code. | |||
| if (PBD->getLoc().isInvalid() || PBD->isImplicit()) | |||
| if (PBD->getLoc().isInvalid() || PBD->isImplicit() || isCaptureList()) | |||
There was a problem hiding this comment.
You don't need PBD here, you can hoist this out.
Maybe, right after if (!isLet()) return; line:
// Don't touch capture list
if (isCaptureList()) return;
modocache
force-pushed
the
ast
branch
2 times, most recently
from
9c8155b to
cbe423d
Compare
| @@ -3743,7 +3743,10 @@ void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const { | |||
|
|
|||
| // Besides self, don't suggest mutability for explicit function parameters. | |||
| if (isa<ParamDecl>(this)) return; | |||
|
|
|||
|
|
|||
| // Don't suggest any fixes for caoture list elements. | |||
| bool isCaptureList() const { return DeclBits.CaptureList; } | ||
|
|
||
| /// \brief Mark this declaration as an element in a capture list. | ||
| void setCaptureList(bool captureList = true) { |
There was a problem hiding this comment.
Since this only seems to be effectively read only, can this setter be removed and the bit set in the constructor?
There was a problem hiding this comment.
That's a good idea, thanks for the suggestion. Will do!
Fixes SR-2757. Variables in capture lists are treated as 'let' constants, which can result in misleading, incorrect diagnostics. Mark them as such in order to produce better diagnostics, by adding an extra parameter to the VarDecl initializer. Alternatively, these variables could be marked as implicit, but that results in other diagnostic problems: capture list variables that are never used produce warnings, but these warnings aren't normally emitted for implicit variables. Other assertions in the compiler also misfire when these variables are treated as implicit. Another alternative would be to walk up the AST and determine whether the `VarDecl`, but there doesn't appear to be a way to do so.
|
Thanks for the suggestion to move the |
|
@swift-ci please smoke test and merge. |
|
Looks like LLDB is calling this constructor and will require an update. If you want to revert the change I requested to get this merged sooner, I think that would be fine. Otherwise, see the Linux test logs for the constructor that needs to be updated. |
|
No rush, I'd be happy to update the LLDB callsite as well. Expect an update before Monday. |
|
@swift-ci please smoke test Linux platform |
swiftlang/swift#6490 modified the VarDecl to add a boolean parameter to specify whether the variable was an element in a closure capture list, in order to improve diagnostics in the Swift compiler. Use this new parameter.
modocache
changed the title
|
Please test with following pull request: @swift-ci Please test |
|
Wonderful ✨ . Thanks for the secondary patch. ⛵️ |
|
Thank you for the reviews and the suggestion to modify the initializer! |
Fixes SR-2757.
Variables in capture lists are treated as 'let' constants, which can result in misleading, incorrect diagnostics. Mark them as such in order to produce better diagnostics.
Alternatively, these variables could be marked as implicit, but that results in other diagnostic problems: capture list variables that are never used produce warnings, but these warnings aren't normally emitted for implicit variables. Other assertions in the compiler also misfire when these variables are treated as implicit.
Another alternative would be to walk up the AST and determine whether the
VarDecl, but there doesn't appear to be a way to do so.