[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/hokein closed https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
@@ -13570,6 +13570,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 (llvm::isa(VDecl)) // Case 2)
AaronBallman wrote:
```suggestion
if (isa(VDecl)) // Case 2)
```
https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/AaronBallman approved this pull request. LGTM with a tiny nit. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
hokein wrote: Friendly ping. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/hokein updated
https://github.com/llvm/llvm-project/pull/72428
>From 662be3d00eed883db6b1babe489b981847e9b907 Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Wed, 15 Nov 2023 20:31:12 +0100
Subject: [PATCH 1/3] [clang][AST] Invalidate DecompositionDecl if it has
invalid initializer.
Fix #67495, #72198
---
clang/lib/Sema/SemaDecl.cpp | 9 +
clang/test/AST/ast-dump-invalid-initialized.cpp | 15 ++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 8e46c4984d93dc9..f9fa9dc53cb2b47 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13570,6 +13570,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
return;
}
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index 1c374ae716a9db5..a71a02f0f60039e 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -24,4 +24,17 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
-}
\ No newline at end of file
+}
+
+void pr72198() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int'
+ int [_, b] = {0, 0};
+ [b]{};
+}
+
+int get_point();
+void pr67495() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int &'
+ auto& [x, y] = get_point();
+ [x, y] {};
+}
>From 851aa492d5e241abd9d4d621b84c137164845890 Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Fri, 17 Nov 2023 10:54:33 +0100
Subject: [PATCH 2/3] Address comments.
---
clang/lib/Sema/SemaDecl.cpp | 6 +++---
clang/test/AST/ast-dump-invalid-initialized.cpp | 6 --
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f9fa9dc53cb2b47..7c88731bebc9f2e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13574,9 +13574,9 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr
*Init, bool DirectInit) {
// 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 concrete type (e.g.
- //`int [a, b] = 1;`), and the initializer is invalid;
- // Case 1) is already handled earlier in this function.
+ // 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 (llvm::isa(VDecl)) // Case 2)
VDecl->setInvalidDecl();
return;
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index a71a02f0f60039e..7fcbc41a7be4001 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -26,15 +26,17 @@ void test() {
auto b5 = A{1};
}
-void pr72198() {
+void GH72198() {
// CHECK: DecompositionDecl {{.*}} invalid 'int'
int [_, b] = {0, 0};
[b]{};
}
+namespace GH67495 {
int get_point();
-void pr67495() {
+void f() {
// CHECK: DecompositionDecl {{.*}} invalid 'int &'
auto& [x, y] = get_point();
[x, y] {};
}
+}
>From 9eb93e95664b70092a56cf5e1a06c9d36c75ae3a Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Tue, 19 Dec 2023 10:11:50 +0100
Subject: [PATCH 3/3] Add release note.
---
clang/docs/ReleaseNotes.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c9b577bd549b1e2..da79b0a2f63fbed 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -696,6 +696,9 @@ Bug Fixes in This Version
- Clang now accepts recursive non-dependent calls to functions with deduced
return type.
Fixes (`#71015 `_)
+- Fix crashes when using the binding decl from an invalid structured binding.
+ Fixes (`#67495 `_) and
+ (`#72198
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/hokein updated
https://github.com/llvm/llvm-project/pull/72428
>From f114c48948d8b56a5e04e50b7f27ce499e60bc77 Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Wed, 15 Nov 2023 20:31:12 +0100
Subject: [PATCH 1/3] [clang][AST] Invalidate DecompositionDecl if it has
invalid initializer.
Fix #67495, #72198
---
clang/lib/Sema/SemaDecl.cpp | 9 +
clang/test/AST/ast-dump-invalid-initialized.cpp | 15 ++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index ffbe317d559995..0986a5f2219b01 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13573,6 +13573,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
return;
}
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index 1c374ae716a9db..a71a02f0f60039 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -24,4 +24,17 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
-}
\ No newline at end of file
+}
+
+void pr72198() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int'
+ int [_, b] = {0, 0};
+ [b]{};
+}
+
+int get_point();
+void pr67495() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int &'
+ auto& [x, y] = get_point();
+ [x, y] {};
+}
>From c0e2dbc23def2dc2c19ca129aa9abf6351de705e Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Fri, 17 Nov 2023 10:54:33 +0100
Subject: [PATCH 2/3] Address comments.
---
clang/lib/Sema/SemaDecl.cpp | 6 +++---
clang/test/AST/ast-dump-invalid-initialized.cpp | 6 --
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 0986a5f2219b01..bb4b2b1990dbdd 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13577,9 +13577,9 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr
*Init, bool DirectInit) {
// 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 concrete type (e.g.
- //`int [a, b] = 1;`), and the initializer is invalid;
- // Case 1) is already handled earlier in this function.
+ // 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 (llvm::isa(VDecl)) // Case 2)
VDecl->setInvalidDecl();
return;
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index a71a02f0f60039..7fcbc41a7be400 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -26,15 +26,17 @@ void test() {
auto b5 = A{1};
}
-void pr72198() {
+void GH72198() {
// CHECK: DecompositionDecl {{.*}} invalid 'int'
int [_, b] = {0, 0};
[b]{};
}
+namespace GH67495 {
int get_point();
-void pr67495() {
+void f() {
// CHECK: DecompositionDecl {{.*}} invalid 'int &'
auto& [x, y] = get_point();
[x, y] {};
}
+}
>From 7ecf5d7af270fba5cdd2d5daebd5e3449b142197 Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Tue, 19 Dec 2023 10:11:50 +0100
Subject: [PATCH 3/3] Add release note.
---
clang/docs/ReleaseNotes.rst | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 39b9176865fc04..dd4377bcf7a194 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -688,7 +688,9 @@ Bug Fixes in This Version
Fixes (#69987 `_)
- Fix an issue where CTAD fails for explicit type conversion.
Fixes (#64347 `_)
-
+- Fix crashes when using the binding decl from an invalid structured binding.
+ Fixes (`#67495
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
hokein wrote: > We need a release note and please add a more detailed summary. A description > of the problem being solved and the solution to the fix provides. Done. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/hokein updated
https://github.com/llvm/llvm-project/pull/72428
>From e182e764778eb137f465812270fcdf03d0fe2da1 Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Wed, 15 Nov 2023 20:31:12 +0100
Subject: [PATCH 1/3] [clang][AST] Invalidate DecompositionDecl if it has
invalid initializer.
Fix #67495, #72198
---
clang/lib/Sema/SemaDecl.cpp | 9 +
clang/test/AST/ast-dump-invalid-initialized.cpp | 15 ++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index edf44bbc52119b..13cd4d9644d727 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13576,6 +13576,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
return;
}
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index 1c374ae716a9db..a71a02f0f60039 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -24,4 +24,17 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
-}
\ No newline at end of file
+}
+
+void pr72198() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int'
+ int [_, b] = {0, 0};
+ [b]{};
+}
+
+int get_point();
+void pr67495() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int &'
+ auto& [x, y] = get_point();
+ [x, y] {};
+}
>From 94a6db1d1f513af792b1764961c897d44b9cba5f Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Fri, 17 Nov 2023 10:54:33 +0100
Subject: [PATCH 2/3] Address comments.
---
clang/lib/Sema/SemaDecl.cpp | 6 +++---
clang/test/AST/ast-dump-invalid-initialized.cpp | 6 --
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 13cd4d9644d727..c7ace117e8bced 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13580,9 +13580,9 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr
*Init, bool DirectInit) {
// 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 concrete type (e.g.
- //`int [a, b] = 1;`), and the initializer is invalid;
- // Case 1) is already handled earlier in this function.
+ // 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 (llvm::isa(VDecl)) // Case 2)
VDecl->setInvalidDecl();
return;
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index a71a02f0f60039..7fcbc41a7be400 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -26,15 +26,17 @@ void test() {
auto b5 = A{1};
}
-void pr72198() {
+void GH72198() {
// CHECK: DecompositionDecl {{.*}} invalid 'int'
int [_, b] = {0, 0};
[b]{};
}
+namespace GH67495 {
int get_point();
-void pr67495() {
+void f() {
// CHECK: DecompositionDecl {{.*}} invalid 'int &'
auto& [x, y] = get_point();
[x, y] {};
}
+}
>From ff9d909135edcc5851b234663fe392fe85070a61 Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Tue, 19 Dec 2023 10:11:50 +0100
Subject: [PATCH 3/3] Add release note.
---
clang/docs/ReleaseNotes.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index edb97347f07716..d0a81a44a650e4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -686,6 +686,9 @@ Bug Fixes in This Version
- Fix an issue where clang doesn't respect detault template arguments that
are added in a later redeclaration for CTAD.
Fixes (#69987 `_)
+- Fix crashes when using the binding decl from an invalid structured binding.
+ Fixes (`#67495 `_) and
+
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/hokein edited https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/shafik commented: We need a release note and please add a more detailed summary. A description of the problem being solved and the solution to the fix provides. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
hokein wrote: Friendly ping. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
hnrklssn wrote: > > What differentiates the DecompositionDecl such that we need to mark the > > decl invalid when there's an error in the RHS, while for other decls we > > don't? It seems inconsistent. > > > > DecompositionDecl (aka structure binding) is special here, a legal > DecompositionDecl must be declared with a deduced `auto` type, so the its > initializer should play part of the decl's invalid bit. For the error case > where a DecompositionDecl with a non-deduced type, clang still builds the > Decl AST node (with invalid bit) for better recovery. There are some > oversight cases. This patch is fixing those. Ah that makes sense then. I didn't realise it had to be `auto`. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
hokein wrote: > What differentiates the DecompositionDecl such that we need to mark the decl > invalid when there's an error in the RHS, while for other decls we don't? It > seems inconsistent. DecompositionDecl (aka structure binding) is special here, a legal DecompositionDecl must be declared with a deduced `auto` type, so the its initializer should play part of the decl's invalid bit. For the error case where a DecompositionDecl with a non-deduced type, clang still builds the Decl AST node (with invalid bit) for better recovery. There are some oversight cases. This patch is fixing those. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
@@ -13540,6 +13540,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
hokein wrote:
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.
https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/hokein updated
https://github.com/llvm/llvm-project/pull/72428
>From ac06843b97cb93d476f0bf8e0474fa270d80631f Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Wed, 15 Nov 2023 20:31:12 +0100
Subject: [PATCH 1/2] [clang][AST] Invalidate DecompositionDecl if it has
invalid initializer.
Fix #67495, #72198
---
clang/lib/Sema/SemaDecl.cpp | 9 +
clang/test/AST/ast-dump-invalid-initialized.cpp | 15 ++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 3876eb501083acb..b89031425e4b5ac 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13540,6 +13540,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
return;
}
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index 1c374ae716a9db5..a71a02f0f60039e 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -24,4 +24,17 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
-}
\ No newline at end of file
+}
+
+void pr72198() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int'
+ int [_, b] = {0, 0};
+ [b]{};
+}
+
+int get_point();
+void pr67495() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int &'
+ auto& [x, y] = get_point();
+ [x, y] {};
+}
>From 067a93ae9aea8a3032091fbbb19d5748ece22361 Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Fri, 17 Nov 2023 10:54:33 +0100
Subject: [PATCH 2/2] Address comments.
---
clang/lib/Sema/SemaDecl.cpp | 6 +++---
clang/test/AST/ast-dump-invalid-initialized.cpp | 6 --
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index b89031425e4b5ac..27b485119ae8615 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13544,9 +13544,9 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr
*Init, bool DirectInit) {
// 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 concrete type (e.g.
- //`int [a, b] = 1;`), and the initializer is invalid;
- // Case 1) is already handled earlier in this function.
+ // 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 (llvm::isa(VDecl)) // Case 2)
VDecl->setInvalidDecl();
return;
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index a71a02f0f60039e..7fcbc41a7be4001 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -26,15 +26,17 @@ void test() {
auto b5 = A{1};
}
-void pr72198() {
+void GH72198() {
// CHECK: DecompositionDecl {{.*}} invalid 'int'
int [_, b] = {0, 0};
[b]{};
}
+namespace GH67495 {
int get_point();
-void pr67495() {
+void f() {
// CHECK: DecompositionDecl {{.*}} invalid 'int &'
auto& [x, y] = get_point();
[x, y] {};
}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
@@ -13540,6 +13540,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
shafik wrote:
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.
https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
@@ -24,4 +24,17 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
-}
\ No newline at end of file
+}
+
+void pr72198() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int'
+ int [_, b] = {0, 0};
+ [b]{};
+}
+
+int get_point();
+void pr67495() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int &'
+ auto& [x, y] = get_point();
+ [x, y] {};
+}
cor3ntin wrote:
```suggestion
namespace GH67495 {
int get_point();
void f() {
// CHECK: DecompositionDecl {{.*}} invalid 'int &'
auto& [x, y] = get_point();
[x, y] {};
}
}
```
https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
@@ -24,4 +24,17 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
-}
\ No newline at end of file
+}
+
+void pr72198() {
cor3ntin wrote:
```suggestion
void GH72198() {
```
https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
@@ -13540,6 +13540,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
cor3ntin wrote:
Maybe we should move the `1)` part of the comment line 13496, or just remove it
entirely.
https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
hnrklssn wrote: What differentiates the DecompositionDecl such that we need to mark the decl invalid when there's an error in the RHS, while for other decls we don't? It seems inconsistent. https://github.com/llvm/llvm-project/pull/72428 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Haojian Wu (hokein)
Changes
Fix #67495, #72198
CC @ADKaster, @ecnelises
---
Full diff: https://github.com/llvm/llvm-project/pull/72428.diff
2 Files Affected:
- (modified) clang/lib/Sema/SemaDecl.cpp (+9)
- (modified) clang/test/AST/ast-dump-invalid-initialized.cpp (+14-1)
``diff
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 3876eb501083acb..b89031425e4b5ac 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13540,6 +13540,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
return;
}
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index 1c374ae716a9db5..a71a02f0f60039e 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -24,4 +24,17 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
-}
\ No newline at end of file
+}
+
+void pr72198() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int'
+ int [_, b] = {0, 0};
+ [b]{};
+}
+
+int get_point();
+void pr67495() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int &'
+ auto& [x, y] = get_point();
+ [x, y] {};
+}
``
https://github.com/llvm/llvm-project/pull/72428
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][AST] Invalidate DecompositionDecl if it has invalid initializer. (PR #72428)
https://github.com/hokein created
https://github.com/llvm/llvm-project/pull/72428
Fix #67495, #72198
CC @ADKaster, @ecnelises
>From 13b97a2239c93fe528174ec9ecc20f3d3ca49e23 Mon Sep 17 00:00:00 2001
From: Haojian Wu
Date: Wed, 15 Nov 2023 20:31:12 +0100
Subject: [PATCH] [clang][AST] Invalidate DecompositionDecl if it has invalid
initializer.
Fix #67495, #72198
---
clang/lib/Sema/SemaDecl.cpp | 9 +
clang/test/AST/ast-dump-invalid-initialized.cpp | 15 ++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 3876eb501083acb..b89031425e4b5ac 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13540,6 +13540,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 concrete type (e.g.
+ //`int [a, b] = 1;`), and the initializer is invalid;
+ // Case 1) is already handled earlier in this function.
+ if (llvm::isa(VDecl)) // Case 2)
+VDecl->setInvalidDecl();
return;
}
diff --git a/clang/test/AST/ast-dump-invalid-initialized.cpp
b/clang/test/AST/ast-dump-invalid-initialized.cpp
index 1c374ae716a9db5..a71a02f0f60039e 100644
--- a/clang/test/AST/ast-dump-invalid-initialized.cpp
+++ b/clang/test/AST/ast-dump-invalid-initialized.cpp
@@ -24,4 +24,17 @@ void test() {
auto b4 = A(1);
// CHECK: `-VarDecl {{.*}} invalid b5 'auto'
auto b5 = A{1};
-}
\ No newline at end of file
+}
+
+void pr72198() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int'
+ int [_, b] = {0, 0};
+ [b]{};
+}
+
+int get_point();
+void pr67495() {
+ // CHECK: DecompositionDecl {{.*}} invalid 'int &'
+ auto& [x, y] = get_point();
+ [x, y] {};
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
