[clang] 70d9dc8 - [AST][RecoveryExpr] Support dependent binary operator in C for error recovery.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-10-06T08:53:31+02:00
New Revision: 70d9dc867417ac63fe280ab145776f75a9487f0f

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

LOG: [AST][RecoveryExpr] Support dependent binary operator in C for error recovery.

see the whole context in:  https://reviews.llvm.org/D85025

Reviewed By: sammccall

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

Added: 
    clang/test/Sema/error-dependence.c

Modified: 
    clang/include/clang/AST/ASTContext.h
    clang/lib/Sema/SemaExpr.cpp
    clang/test/AST/ast-dump-recovery.c

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index d30cf045f104..e261e29036e9 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -665,6 +665,14 @@ class ASTContext : public RefCountedBase<ASTContext> {
 
   const LangOptions& getLangOpts() const { return LangOpts; }
 
+  // If this condition is false, typo correction must be performed eagerly
+  // rather than delayed in many places, as it makes use of dependent types.
+  // the condition is false for clang's C-only codepath, as it doesn't support
+  // dependent types yet.
+  bool isDependenceAllowed() const {
+    return LangOpts.CPlusPlus || LangOpts.RecoveryAST;
+  }
+
   const SanitizerBlacklist &getSanitizerBlacklist() const {
     return *SanitizerBL;
   }

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e51b27626184..ee41d5f5b37d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
@@ -13683,7 +13684,7 @@ static std::pair<ExprResult, ExprResult>
 CorrectDelayedTyposInBinOp(Sema &S, BinaryOperatorKind Opc, Expr *LHSExpr,
                            Expr *RHSExpr) {
   ExprResult LHS = LHSExpr, RHS = RHSExpr;
-  if (!S.getLangOpts().CPlusPlus) {
+  if (!S.Context.isDependenceAllowed()) {
     // C cannot handle TypoExpr nodes on either side of a binop because it
     // doesn't handle dependent types properly, so make sure any TypoExprs have
     // been dealt with before checking the operands.
@@ -14364,6 +14365,47 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
       return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
   }
 
+  if (getLangOpts().RecoveryAST &&
+      (LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent())) {
+    assert(!getLangOpts().CPlusPlus);
+    assert((LHSExpr->containsErrors() || RHSExpr->containsErrors()) &&
+           "Should only occur in error-recovery path.");
+    if (BinaryOperator::isCompoundAssignmentOp(Opc))
+      // C [6.15.16] p3:
+      // An assignment expression has the value of the left operand after the
+      // assignment, but is not an lvalue.
+      return CompoundAssignOperator::Create(
+          Context, LHSExpr, RHSExpr, Opc,
+          LHSExpr->getType().getUnqualifiedType(), VK_RValue, OK_Ordinary,
+          OpLoc, CurFPFeatureOverrides());
+    QualType ResultType;
+    switch (Opc) {
+    case BO_Assign:
+      ResultType = LHSExpr->getType().getUnqualifiedType();
+      break;
+    case BO_LT:
+    case BO_GT:
+    case BO_LE:
+    case BO_GE:
+    case BO_EQ:
+    case BO_NE:
+    case BO_LAnd:
+    case BO_LOr:
+      // These operators have a fixed result type regardless of operands.
+      ResultType = Context.IntTy;
+      break;
+    case BO_Comma:
+      ResultType = RHSExpr->getType();
+      break;
+    default:
+      ResultType = Context.DependentTy;
+      break;
+    }
+    return BinaryOperator::Create(Context, LHSExpr, RHSExpr, Opc, ResultType,
+                                  VK_RValue, OK_Ordinary, OpLoc,
+                                  CurFPFeatureOverrides());
+  }
+
   // Build a built-in binary operation.
   return CreateBuiltinBinOp(OpLoc, Opc, LHSExpr, RHSExpr);
 }

diff  --git a/clang/test/AST/ast-dump-recovery.c b/clang/test/AST/ast-dump-recovery.c
index f3a33fdac49b..66830e072a2a 100644
--- a/clang/test/AST/ast-dump-recovery.c
+++ b/clang/test/AST/ast-dump-recovery.c
@@ -42,11 +42,33 @@ void test1() {
 
 void test2() {
   int* ptr;
-  // FIXME: the top-level expr should be a binary operator.
-  // CHECK:      ImplicitCastExpr {{.*}} contains-errors <LValueToRValue>
-  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors lvalue
-  // CHECK-NEXT:   |-DeclRefExpr {{.*}} 'ptr' 'int *'
-  // CHECK-NEXT:   `-RecoveryExpr {{.*}}
-  // CHECK-NEXT:     `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK:     BinaryOperator {{.*}} 'int *' contains-errors '='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'ptr' 'int *'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}}
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
   ptr = some_func(); // should not crash
+
+  int compoundOp;
+  // CHECK:     CompoundAssignOperator {{.*}} 'int' contains-errors '+='
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'compoundOp'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}} contains-errors
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
+  compoundOp += some_func();
+
+  // CHECK:     BinaryOperator {{.*}} 'int' contains-errors '||'
+  // CHECK-NEXT: |-RecoveryExpr {{.*}}
+  // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  some_func() || 1;
+
+  // CHECK:     BinaryOperator {{.*}} '<dependent type>' contains-errors ','
+  // CHECK-NEXT: |-IntegerLiteral {{.*}} 'int' 1
+  // CHECK-NEXT: `-RecoveryExpr {{.*}}
+  // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
+  1, some_func();
+  // CHECK:     BinaryOperator {{.*}} 'int' contains-errors ','
+  // CHECK-NEXT: |-RecoveryExpr {{.*}} '<dependent type>'
+  // CHECK-NEXT: | `-DeclRefExpr {{.*}} 'some_func'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  some_func(), 1;
 }

diff  --git a/clang/test/Sema/error-dependence.c b/clang/test/Sema/error-dependence.c
new file mode 100644
index 000000000000..a98b021094de
--- /dev/null
+++ b/clang/test/Sema/error-dependence.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -frecovery-ast -fno-recovery-ast-type %s
+
+int call(int); // expected-note {{'call' declared here}}
+
+void test1(int s) {
+  // verify "assigning to 'int' from incompatible type '<dependent type>'" is
+  // not emitted.
+  s = call(); // expected-error {{too few arguments to function call}}
+}