[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG21ccc684ff4c: [AST][RecoveryExpr] Build RecoveryExpr for
undef_var cases. (authored by hokein).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80733/new/
https://reviews.llvm.org/D80733
Files:
clang/lib/Sema/SemaExprCXX.cpp
clang/test/AST/ast-dump-recovery.cpp
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
// CHECK-NEXT:`-IntegerLiteral {{.*}} 123
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+ // CHECK: CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+ some_func(undef1, undef2+1);
+
+ // CHECK: BinaryOperator {{.*}} '' contains-errors '='
+ // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+ // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+ s = some_func(undef1);
+ // CHECK: `-VarDecl {{.*}} invalid var 'int'
+ // FIXME: preserve the broken call.
+ int var = some_func(undef1);
+}
int ambig_func(double);
int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@
}
FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
- if (FullExpr.isInvalid())
-return ExprError();
+ if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+ TyposReplace(Sema ) : TreeTransform(SemaRef) {}
+ ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+ }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+ }
CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
// CHECK-NEXT:`-IntegerLiteral {{.*}} 123
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+ // CHECK: CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+ some_func(undef1, undef2+1);
+
+ // CHECK: BinaryOperator {{.*}} '' contains-errors '='
+ // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+ // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+ s = some_func(undef1);
+ // CHECK: `-VarDecl {{.*}} invalid var 'int'
+ // FIXME: preserve the broken call.
+ int var = some_func(undef1);
+}
int ambig_func(double);
int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@
}
FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
- if (FullExpr.isInvalid())
-return ExprError();
+ if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+ TyposReplace(Sema ) : TreeTransform(SemaRef) {}
+ ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+ }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+ }
CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.
hokein added inline comments.
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8310
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
sammccall wrote:
> This seems reasonably straightforward to fix - there shouldn't be a lot of
> TypoExprs so we can just add a member. Worth doing I think.
yeah, agree. Will address it in a followup.
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8314
+ using Base = TreeTransform;
+ TyposReplace(Sema ) : Base(SemaRef) {}
+ ExprResult TransformTypoExpr(TypoExpr *E) {
sammccall wrote:
> Can't you just write this as `: TreeTransform(SemaRef) {}`, and drop the
> using?
yes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80733/new/
https://reviews.llvm.org/D80733
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.
hokein updated this revision to Diff 267873.
hokein marked 3 inline comments as done.
hokein added a comment.
Address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80733/new/
https://reviews.llvm.org/D80733
Files:
clang/lib/Sema/SemaExprCXX.cpp
clang/test/AST/ast-dump-recovery.cpp
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
// CHECK-NEXT:`-IntegerLiteral {{.*}} 123
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+ // CHECK: CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+ some_func(undef1, undef2+1);
+
+ // CHECK: BinaryOperator {{.*}} '' contains-errors '='
+ // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+ // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+ s = some_func(undef1);
+ // CHECK: `-VarDecl {{.*}} invalid var 'int'
+ // FIXME: preserve the broken call.
+ int var = some_func(undef1);
+}
int ambig_func(double);
int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@
}
FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
- if (FullExpr.isInvalid())
-return ExprError();
+ if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+ TyposReplace(Sema ) : TreeTransform(SemaRef) {}
+ ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+ }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+ }
CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
// CHECK-NEXT:`-IntegerLiteral {{.*}} 123
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+ // CHECK: CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+ some_func(undef1, undef2+1);
+
+ // CHECK: BinaryOperator {{.*}} '' contains-errors '='
+ // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+ // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+ s = some_func(undef1);
+ // CHECK: `-VarDecl {{.*}} invalid var 'int'
+ // FIXME: preserve the broken call.
+ int var = some_func(undef1);
+}
int ambig_func(double);
int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,21 @@
}
FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
- if (FullExpr.isInvalid())
-return ExprError();
+ if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+ TyposReplace(Sema ) : TreeTransform(SemaRef) {}
+ ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+ }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+ }
CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Nice!
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8310
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
This seems reasonably straightforward to fix - there shouldn't be a lot of
TypoExprs so we can just add a member. Worth doing I think.
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8314
+ using Base = TreeTransform;
+ TyposReplace(Sema ) : Base(SemaRef) {}
+ ExprResult TransformTypoExpr(TypoExpr *E) {
Can't you just write this as `: TreeTransform(SemaRef) {}`, and drop the using?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80733/new/
https://reviews.llvm.org/D80733
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D80733: [AST][RecoveryExpr] Build RecoveryExpr for "undef_var" cases.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a reviewer: jdoerfert.
Herald added a subscriber: sstefan1.
Herald added a project: clang.
For a no-function-like unresolved expression, clang builds a TypoExpr
for it, and tries to correct it afterwards. If the typo-correction
fails, clang just drops the whole expr.
This patch improves the recovery strategy -- if the typo-correction
fails, we preserve the AST by degrading the typo exprs to recovery
exprs.
This would improve toolings for "undef_var" broken cases:
void foo();
void test() {
fo^o(undef_var); // go-to-def, hover still works.
}
TESTED=ran tests with this patch + turn-on-recovery-ast patch, it breaks
one declare_variant_messages testcase (the diagnostics are slightly
changed), I think it is acceptable.
Error: 'error' diagnostics seen but not expected:
File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 16:
expected 'match' clause on 'omp declare variant' directive
File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 57:
expected 'match' clause on 'omp declare variant' directive
error: 'warning' diagnostics expected but not seen:
File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 47:
the context selector 'kind' in the context set 'device' cannot have a score
(''); score ignored
File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 87:
the context selector 'kind' in the context set 'device' cannot have a score
(''); score ignored
error: 'warning' diagnostics seen but not expected:
File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 47:
the context selector 'kind' in the context set 'device' cannot have a score
('()'); score ignored
File llvm-project/clang/test/OpenMP/declare_variant_messages.cpp Line 87:
the context selector 'kind' in the context set 'device' cannot have a score
('()'); score ignored
6 errors generated.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80733
Files:
clang/lib/Sema/SemaExprCXX.cpp
clang/test/AST/ast-dump-recovery.cpp
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
// CHECK-NEXT:`-IntegerLiteral {{.*}} 123
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+ // CHECK: CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+ some_func(undef1, undef2+1);
+
+ // CHECK: BinaryOperator {{.*}} '' contains-errors '='
+ // CHECK-NEXT: |-DeclRefExpr {{.*}} 's'
+ // CHECK-NEXT: `-CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+ s = some_func(undef1);
+ // CHECK: `-VarDecl {{.*}} invalid var 'int'
+ // FIXME: preserve the broken call.
+ int var = some_func(undef1);
+}
int ambig_func(double);
int ambig_func(float);
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8304,8 +8304,22 @@
}
FullExpr = CorrectDelayedTyposInExpr(FullExpr.get());
- if (FullExpr.isInvalid())
-return ExprError();
+ if (FullExpr.isInvalid()) {
+// Typo-correction fails, we rebuild the broken AST with the typos degraded
+// to RecoveryExpr.
+// FIXME: we lose source locations for RecoveryExpr, as TypoExpr doesn't
+// track source locations.
+struct TyposReplace : TreeTransform {
+ using Base = TreeTransform;
+ TyposReplace(Sema ) : Base(SemaRef) {}
+ ExprResult TransformTypoExpr(TypoExpr *E) {
+return this->SemaRef.CreateRecoveryExpr(E->getBeginLoc(),
+E->getEndLoc(), {});
+ }
+} TT(*this);
+
+return TT.TransformExpr(FE);
+ }
CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
Index: clang/test/AST/ast-dump-recovery.cpp
===
--- clang/test/AST/ast-dump-recovery.cpp
+++ clang/test/AST/ast-dump-recovery.cpp
@@ -10,6 +10,25 @@
// CHECK-NEXT:`-IntegerLiteral {{.*}} 123
// DISABLED-NOT: -RecoveryExpr {{.*}} contains-errors
int invalid_call = some_func(123);
+void test_invalid_call(int s) {
+ // CHECK: CallExpr {{.*}} '' contains-errors
+ // CHECK-NEXT: |-UnresolvedLookupExpr {{.*}} 'some_func'
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ // CHECK-NEXT: `-BinaryOperator {{.*}} <, col:28>
+ // CHECK-NEXT: |-RecoveryExpr {{.*}} <>
+ //
