hokein created this revision.
hokein added reviewers: rsmith, sammccall.
Herald added a subscriber: kristof.beyls.
Herald added a project: clang.
hokein requested review of this revision.
Because of typo-correction, the AST can be transformed, and the transformed
AST is marginally useful for diagnostics purpose, the following
diagnostics usually do harm than good (easily cause confusions).
Given the following code:
void abcc();
void test() {
if (abc());
// diagnostic 1 (for the typo-correction): the typo is correct to `abcc()`,
so the code is treate as `if (abcc())` in AST perspective;
// diagnostic 2 (for mismatch type): we perform an type-analysis on `if`,
discover the type is not match
}
The secondary diagnostic "convertable to bool" is likely bogus to users.
The idea is to use RecoveryExpr (clang's dependent mechanism) to preserve the
recovery behavior but suppress all follow-up diagnostics.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89946
Files:
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
clang/test/Modules/submodules-merge-defs.cpp
clang/test/SemaCXX/enable_if.cpp
clang/test/SemaCXX/typo-correction-delayed.cpp
Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===================================================================
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -209,6 +209,15 @@
// expected-error-re@-1 {{use of undeclared identifier 'N'{{$}}}}
}
+namespace noSecondaryDiags {
+void abcc(); // expected-note {{'abcc' declared here}}
+
+void test() {
+ // Verify the secondary diagnostic ".. convertiable to 'bool'" is suppressed.
+ if (abc()) {} // expected-error {{use of undeclared identifier 'abc'; did you mean 'abcc'?}}
+}
+}
+
// PR 23285. This test must be at the end of the file to avoid additional,
// unwanted diagnostics.
// expected-error-re@+2 {{use of undeclared identifier 'uintmax_t'{{$}}}}
Index: clang/test/SemaCXX/enable_if.cpp
===================================================================
--- clang/test/SemaCXX/enable_if.cpp
+++ clang/test/SemaCXX/enable_if.cpp
@@ -356,9 +356,9 @@
__attribute__((enable_if(num != 1, "")));
} // namespace ns
-using ns::Function; // expected-note 3{{declared here}}
+using ns::Function; // expected-note 2{{declared here}}
void Run() {
- Functioon(0); // expected-error{{use of undeclared identifier}} expected-error{{too few arguments}}
+ Functioon(0); // expected-error{{use of undeclared identifier}}
Functioon(0, 1); // expected-error{{use of undeclared identifier}}
Functioon(0, 1, 2); // expected-error{{use of undeclared identifier}}
}
Index: clang/test/Modules/submodules-merge-defs.cpp
===================================================================
--- clang/test/Modules/submodules-merge-defs.cpp
+++ clang/test/Modules/submodules-merge-defs.cpp
@@ -18,7 +18,7 @@
// expected-error-re@-1 {{missing '#include "{{.*}}-defs.h"'; 'A' must be declared}}
// expected-note@defs.h:1 +{{here}}
extern class A pre_a2;
-int pre_use_a = use_a(pre_a2); // expected-error 2{{'A' must be defined}} expected-error {{'use_a' must be declared}}
+int pre_use_a = use_a(pre_a2); // expected-error {{'use_a' must be declared}}
// expected-note@defs.h:2 +{{here}}
B::Inner2 pre_bi; // expected-error +{{must be declared}} expected-error +{{must be defined}}
Index: clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
===================================================================
--- clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
+++ clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p4.cpp
@@ -21,12 +21,10 @@
}
namespace C {
- class C {}; // expected-note {{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'B::B' to 'const C::C &' for 1st argument}}
+ class C {};
#if __cplusplus >= 201103L // C++11 or later
- // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: no known conversion from 'B::B' to 'C::C &&' for 1st argument}}
#endif
- void func(C); // expected-note {{'C::func' declared here}} \
- // expected-note {{passing argument to parameter here}}
+ void func(C); // expected-note {{'C::func' declared here}}
C operator+(C,C);
D::D operator+(D::D,D::D);
}
@@ -38,13 +36,7 @@
namespace Test {
void test() {
func(A::A());
- // FIXME: namespace-aware typo correction causes an extra, misleading
- // message in this case; some form of backtracking, diagnostic message
- // delaying, or argument checking before emitting diagnostics is needed to
- // avoid accepting and printing out a typo correction that proves to be
- // incorrect once argument-dependent lookup resolution has occurred.
- func(B::B()); // expected-error {{use of undeclared identifier 'func'; did you mean 'C::func'?}} \
- // expected-error {{no viable conversion from 'B::B' to 'C::C'}}
+ func(B::B()); // expected-error {{use of undeclared identifier 'func'; did you mean 'C::func'?}}
func(C::C());
A::A() + A::A();
B::B() + B::B();
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -12798,6 +12798,16 @@
if (NewFn.isInvalid())
return ExprError();
+ // Offering "follow-up" diagnostics on these recovery call expressions seems
+ // like a bad risk/reward tradeoff, e.g. following diagnostics on a
+ // typo-correct expression can easily cause confusions.
+ // We keep the recovery behavior but suppress all following diagnostics by
+ // using RecoveryExpr.
+ NewFn = SemaRef.CreateRecoveryExpr(NewFn.get()->getBeginLoc(),
+ NewFn.get()->getEndLoc(), {NewFn.get()});
+ if (NewFn.isInvalid())
+ return ExprError();
+
// This shouldn't cause an infinite loop because we're giving it
// an expression with viable lookup results, which should never
// end up here.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
