Skip to content

[clang][AST] Invalidate DecompositionDecl if it has invalid initializer. #72428

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 4 commits into from
Jan 17, 2024
Merged

[clang][AST] Invalidate DecompositionDecl if it has invalid initializer. #72428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c273c4
[clang][AST] Invalidate DecompositionDecl if it has invalid initializer.
hokein Nov 15, 2023
cf947cb
Address comments.
hokein Nov 17, 2023
cacf0e3
Add release note.
hokein Dec 19, 2023
a0ad535
Fix a nit.
hokein Jan 17, 2024
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
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,9 @@ Bug Fixes in This Version
Fixes (`#77583 <https://github.com/llvm/llvm-project/issues/77583>`_)
- Fix an issue where CTAD fails for function-type/array-type arguments.
Fixes (`#51710 <https://github.com/llvm/llvm-project/issues/51710>`_)
- Fix crashes when using the binding decl from an invalid structured binding.
Fixes (`#67495 <https://github.com/llvm/llvm-project/issues/67495>`_) and
(`#72198 <https://github.com/llvm/llvm-project/issues/72198>`_)

Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/Sema/SemaDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13602,6 +13602,15 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), Args);
if (RecoveryExpr.get())
VDecl->setInit(RecoveryExpr.get());
// In general, for error recovery purposes, the initalizer doesn't play
// part in the valid bit of the declaration. There are a few exceptions:
// 1) if the var decl has a deduced auto type, and the type cannot be
// deduced by an invalid initializer;
// 2) if the var decl is decompsition decl with a non-deduced type, and
// the initialization fails (e.g. `int [a] = {1, 2};`);
// Case 1) was already handled elsewhere.
if (isa<DecompositionDecl>(VDecl)) // Case 2)
VDecl->setInvalidDecl();
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should move the 1) part of the comment line 13496, or just remove it entirely.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should have the comment in both places. This is very helpful to understanding the code.

I agree that having the comment far away from the code that it is commenting on is less useful. The code could easily be moved in a refactor or in other ways and become completely disconnected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this comment is helpful (especially the context is not obvious here). I'd like to keep it.

The code handling the case 1) is https://github.com/llvm/llvm-project/blob/1c05fe350064aa3a1784bb09829a07d501842d97/clang/lib/Sema/SemaDecl.cpp#L13363C3-L13384, and it is easier to understand the purpose by reading the code. I'd avoid having two similar comments in the codebase.

return;
}

Expand Down
17 changes: 16 additions & 1 deletion clang/test/AST/ast-dump-invalid-initialized.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,19 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
}
}

void GH72198() {
// CHECK: DecompositionDecl {{.*}} invalid 'int'
int [_, b] = {0, 0};
[b]{};
}

namespace GH67495 {
int get_point();
void f() {
// CHECK: DecompositionDecl {{.*}} invalid 'int &'
auto& [x, y] = get_point();
[x, y] {};
}
}