[clang] Don't preserve the typo expr in the recovery expr for invalid VarDecls #90948
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VarDecls. With the commit d530894, we now preserve the initializer for invalid decls with the recovery-expr. However there is a chance that the original init expr is a typo-expr, we should not preserve it in the final AST, as typo-expr is an internal AST nodes. We should use the one after the typo correction. This is spotted by a clangd hover crash on the testcase.
llvmbot
added
clang
|
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang-tools-extra Author: Haojian Wu (hokein) ChangesWith the commit d530894, we now preserve the initializer for invalid decls with the recovery-expr. However there is a chance that the original init expr is a typo-expr, we should not preserve it in the final AST, as typo-expr is an internal AST nodes. We should use the one after the typo correction. This is spotted by a clangd hover crash on the testcase. Full diff: https://github.com/llvm/llvm-project/pull/90948.diff 3 Files Affected:
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 5ead74748f550c..28df24f34827c0 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -965,6 +965,19 @@ class Foo final {})cpp";
// Bindings are in theory public members of an anonymous struct.
HI.AccessSpecifier = "public";
}},
+ {// Don't crash on invalid decl with invalid init expr.
+ R"cpp(
+ Unknown [[^abc]] = invalid;
+ // error-ok
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "abc";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.NamespaceScope = "";
+ HI.Definition = "int abc = <recovery - expr>()";
+ HI.Type = "int";
+ HI.AccessSpecifier = "public";
+ }},
{// Extra info for function call.
R"cpp(
void fun(int arg_a, int &arg_b) {};
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 19968452f0d566..169df60b56bb4b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13530,9 +13530,12 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
}
if (VDecl->isInvalidDecl()) {
- CorrectDelayedTyposInExpr(Init, VDecl);
+ ExprResult Res = CorrectDelayedTyposInExpr(Init, VDecl);
+ std::vector<Expr *> SubExprs;
+ if (Res.isUsable())
+ SubExprs.push_back(Res.get());
ExprResult Recovery =
- CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), {Init});
+ CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), SubExprs);
if (Expr *E = Recovery.get())
VDecl->setInit(E);
return;
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index 77527743fe8577..a88dff471d9f04 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -419,6 +419,11 @@ void InitializerOfInvalidDecl() {
// CHECK: VarDecl {{.*}} invalid InvalidDecl
// CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>' contains-errors
// CHECK-NEXT: `-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'ValidDecl'
+
+ Unknown InvalidDeclWithInvalidInit = Invalid;
+ // CHECK: VarDecl {{.*}} invalid InvalidDeclWithInvalidInit
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>' contains-errors
+ // CHECK-NOT: `-TypoExpr
}
void RecoverToAnInvalidDecl() {
|
|
@llvm/pr-subscribers-clang Author: Haojian Wu (hokein) ChangesWith the commit d530894, we now preserve the initializer for invalid decls with the recovery-expr. However there is a chance that the original init expr is a typo-expr, we should not preserve it in the final AST, as typo-expr is an internal AST nodes. We should use the one after the typo correction. This is spotted by a clangd hover crash on the testcase. Full diff: https://github.com/llvm/llvm-project/pull/90948.diff 3 Files Affected:
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 5ead74748f550c..28df24f34827c0 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -965,6 +965,19 @@ class Foo final {})cpp";
// Bindings are in theory public members of an anonymous struct.
HI.AccessSpecifier = "public";
}},
+ {// Don't crash on invalid decl with invalid init expr.
+ R"cpp(
+ Unknown [[^abc]] = invalid;
+ // error-ok
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.Name = "abc";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.NamespaceScope = "";
+ HI.Definition = "int abc = <recovery - expr>()";
+ HI.Type = "int";
+ HI.AccessSpecifier = "public";
+ }},
{// Extra info for function call.
R"cpp(
void fun(int arg_a, int &arg_b) {};
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 19968452f0d566..169df60b56bb4b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13530,9 +13530,12 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) {
}
if (VDecl->isInvalidDecl()) {
- CorrectDelayedTyposInExpr(Init, VDecl);
+ ExprResult Res = CorrectDelayedTyposInExpr(Init, VDecl);
+ std::vector<Expr *> SubExprs;
+ if (Res.isUsable())
+ SubExprs.push_back(Res.get());
ExprResult Recovery =
- CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), {Init});
+ CreateRecoveryExpr(Init->getBeginLoc(), Init->getEndLoc(), SubExprs);
if (Expr *E = Recovery.get())
VDecl->setInit(E);
return;
diff --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index 77527743fe8577..a88dff471d9f04 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -419,6 +419,11 @@ void InitializerOfInvalidDecl() {
// CHECK: VarDecl {{.*}} invalid InvalidDecl
// CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>' contains-errors
// CHECK-NEXT: `-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'ValidDecl'
+
+ Unknown InvalidDeclWithInvalidInit = Invalid;
+ // CHECK: VarDecl {{.*}} invalid InvalidDeclWithInvalidInit
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>' contains-errors
+ // CHECK-NOT: `-TypoExpr
}
void RecoverToAnInvalidDecl() {
|
cor3ntin
reviewed
May 3, 2024
clang/lib/Sema/SemaDecl.cpp
Outdated
| @@ -13530,9 +13530,12 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) { | |||
| } | |||
|
|
|||
| if (VDecl->isInvalidDecl()) { | |||
| CorrectDelayedTyposInExpr(Init, VDecl); | |||
| ExprResult Res = CorrectDelayedTyposInExpr(Init, VDecl); | |||
| std::vector<Expr *> SubExprs; | |||
There was a problem hiding this comment.
This can be an ArrayRef (or at the minimum a SmallVector)
There was a problem hiding this comment.
Switched to SmallVector.
HighCommander4
approved these changes
May 3, 2024
cor3ntin
approved these changes
May 7, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With the commit d530894, we now preserve the initializer for invalid decls with the recovery-expr.
However there is a chance that the original init expr is a typo-expr, we should not preserve it in the final AST, as typo-expr is an internal AST node. We should use the one after the typo correction.
This is spotted by a clangd hover crash on the testcase.