Skip to content

[CS] Invalidate nested unresolved VarDecls when ignoring completion argument #85142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 27, 2025
+15 −4
Merged

[CS] Invalidate nested unresolved VarDecls when ignoring completion argument #85142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/swift/AST/Expr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,10 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
/// walk into the body of it (unless it is single-expression).
void forEachChildExpr(llvm::function_ref<Expr *(Expr *)> callback);

/// Apply the specified function to all variables referenced in any
/// child UnresolvedPatternExprs.
void forEachUnresolvedVariable(llvm::function_ref<void(VarDecl *)> f) const;

/// Determine whether this expression refers to a type by name.
///
/// This distinguishes static references to types, like Int, from metatype
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/Pattern.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class alignas(8) Pattern : public ASTAllocated<Pattern> {
/// Collect the set of variables referenced in the given pattern.
void collectVariables(SmallVectorImpl<VarDecl *> &variables) const;

/// apply the specified function to all variables referenced in this
/// Apply the specified function to all variables referenced in this
/// pattern.
void forEachVariable(llvm::function_ref<void(VarDecl *)> f) const;

Expand Down
6 changes: 4 additions & 2 deletions lib/AST/Pattern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,7 @@ namespace {
const std::function<void(VarDecl*)> &fn;
public:

WalkToVarDecls(const std::function<void(VarDecl*)> &fn)
: fn(fn) {}
WalkToVarDecls(const std::function<void(VarDecl*)> &fn) : fn(fn) {}

/// Walk everything that's available; there shouldn't be macro expansions
/// that matter anyway.
Expand Down Expand Up @@ -239,6 +238,9 @@ namespace {
};
} // end anonymous namespace

void Expr::forEachUnresolvedVariable(llvm::function_ref<void(VarDecl *)> f) const {
const_cast<Expr *>(this)->walk(WalkToVarDecls(f));
}

/// apply the specified function to all variables referenced in this
/// pattern.
Expand Down
5 changes: 5 additions & 0 deletions lib/Sema/CSGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4354,6 +4354,11 @@ namespace {
auto &CS = CG.getConstraintSystem();

if (CS.isArgumentIgnoredForCodeCompletion(expr)) {
// Make sure we invalidate any nested VarDecls to ensure the body
// VarDecl for a case statement still gets a type assigned.
expr->forEachUnresolvedVariable([&](VarDecl *VD) {
Copy link
Contributor

@xedin xedin Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a more robust long-term solution here might be to determine whether arguments are effectively ignored during pre-check and replace each one with something like a placeholder expression (something we'd need to add) that would generate a type variable or maybe remove them outright (I think we added some logic to the matcher at some point to not add "missing argument" fixes for cases like that).

Copy link
Contributor Author

@hamishknight hamishknight Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this is made trickier by the fact that we populate the case body variables when we create the AST, so we can't just drop AST nodes to fix this since the case body variables are still accessible to name lookup and won't have types assigned. Maybe we can come up with a better way of handling this

Copy link
Contributor

@xedin xedin Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do

 expr->forEachUnresolvedVariable([&](VarDecl *VD) { ... };

as part of pre-check removing of the expressions...

CS.setType(VD, ErrorType::get(CS.getASTContext()));
});
CG.setTypeForArgumentIgnoredForCompletion(expr);
return Action::SkipNode(expr);
}
Expand Down
2 changes: 1 addition & 1 deletion ...ers/ConstraintSystem-getType-64cbd4.swift → ...xed/ConstraintSystem-getType-64cbd4.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// {"kind":"complete","signature":"swift::constraints::ConstraintSystem::getType(swift::ASTNode) const","signatureAssert":"Assertion failed: (found != NodeTypes.end() && \"Expected type to have been set!\"), function getType"}
// RUN: not --crash %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s
// RUN: %target-swift-ide-test -code-completion -batch-code-completion -skip-filecheck -code-completion-diagnostics -source-filename %s
{ switch { case let c(#^COMPLETE^# b) a