[clang] efa9aaa - [clang] Suppress "follow-up" diagnostics on recovery call expressions.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-10-26T12:40:00+01:00
New Revision: efa9aaad703e6b150980ed1a74b4e7c9da7d85a2

URL: https://github.com/llvm/llvm-project/commit/efa9aaad703e6b150980ed1a74b4e7c9da7d85a2
DIFF: https://github.com/llvm/llvm-project/commit/efa9aaad703e6b150980ed1a74b4e7c9da7d85a2.diff

LOG: [clang] Suppress "follow-up" diagnostics on recovery call expressions.

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.

Differential Revision: https://reviews.llvm.org/D89946

Added: 
    

Modified: 
    clang/lib/Sema/SemaOverload.cpp
    clang/test/AST/ast-dump-recovery.cpp
    clang/test/SemaCXX/typo-correction-delayed.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index d7b985a7b329..ae106ff4557a 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -12735,8 +12735,6 @@ class BuildRecoveryCallExprRAII {
 }
 
 /// Attempts to recover from a call where no functions were found.
-///
-/// Returns true if new candidates were found.
 static ExprResult
 BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
                       UnresolvedLookupExpr *ULE,
@@ -12793,7 +12791,7 @@ BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
     return ExprError();
   }
 
-  // Build an implicit member call if appropriate.  Just drop the
+  // Build an implicit member access expression if appropriate. Just drop the
   // casts and such from the call, we don't really care.
   ExprResult NewFn = ExprError();
   if ((*R.begin())->isCXXClassMember())
@@ -12808,12 +12806,19 @@ BuildRecoveryCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
   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.
-  return SemaRef.BuildCallExpr(/*Scope*/ nullptr, NewFn.get(), LParenLoc,
-                               MultiExprArg(Args.data(), Args.size()),
-                               RParenLoc);
+  auto CallE =
+      SemaRef.BuildCallExpr(/*Scope*/ nullptr, NewFn.get(), LParenLoc,
+                            MultiExprArg(Args.data(), Args.size()), RParenLoc);
+  if (CallE.isInvalid())
+    return ExprError();
+  // We now have recovered a callee. However, building a real call may lead to
+  // incorrect secondary diagnostics if our recovery wasn't correct.
+  // We keep the recovery behavior but suppress all following diagnostics by
+  // using RecoveryExpr. We deliberately drop the return type of the recovery
+  // function, and rely on clang's dependent mechanism to suppress following
+  // diagnostics.
+  return SemaRef.CreateRecoveryExpr(CallE.get()->getBeginLoc(),
+                                    CallE.get()->getEndLoc(), {CallE.get()});
 }
 
 /// Constructs and populates an OverloadedCandidateSet from

diff  --git a/clang/test/AST/ast-dump-recovery.cpp b/clang/test/AST/ast-dump-recovery.cpp
index 69d5f80427cb..366b3bfd9e07 100644
--- a/clang/test/AST/ast-dump-recovery.cpp
+++ b/clang/test/AST/ast-dump-recovery.cpp
@@ -271,3 +271,14 @@ void InvalidCondition() {
   // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2
   invalid() ? 1 : 2;
 }
+
+void abcc();
+void TypoCorrection() {
+  // RecoveryExpr is always dependent-type in this case in order to suppress
+  // following diagnostics.
+  // CHECK:      RecoveryExpr {{.*}} '<dependent type>'
+  // CHECK-NEXT: `-CallExpr {{.*}} 'void'
+  // CHECK-NEXT:   `-ImplicitCastExpr
+  // CHECK-NEXT:     `-DeclRefExpr {{.*}} 'abcc'
+  abc();
+}

diff  --git a/clang/test/SemaCXX/typo-correction-delayed.cpp b/clang/test/SemaCXX/typo-correction-delayed.cpp
index 66d19daf66fd..aa136a08be4f 100644
--- a/clang/test/SemaCXX/typo-correction-delayed.cpp
+++ b/clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -209,6 +209,15 @@ int z = 1 ? N : ;  // expected-error {{expected expression}}
 // expected-error-re at -1 {{use of undeclared identifier 'N'{{$}}}}
 }
 
+namespace noSecondaryDiags {
+void abcc(); // expected-note {{'abcc' declared here}}
+
+void test() {
+  // Verify the secondary diagnostic ".. convertible 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 at +2 {{use of undeclared identifier 'uintmax_t'{{$}}}}