[AST][RecoveryExpr] Part 2: Build dependent callexpr in C for error-recovery.

[sammccall]

clang/lib/Sema/SemaExpr.cpp

6378

why is CDependence a condition and not an assertion inside the if?
(If it's for performance, then we're going to lose this performance when flipping the flag, so I'd consider just flipping it now)

If you're using it as a short-hand for "is this plain C and also has recovery enabled" then I'd avoid that - it's not clear that this is a language check.

6385

Is this really the right place vs in BuildResolvedCallExpr?

6385

why DependentTy? shouldn't it be the return type of the function, if available?

clang/test/AST/ast-dump-recovery.c

102

i'm not sure what the int (int) is about - can you remove it from the assertion?

[hokein]

address comments.

[hokein]

fix format.

clang/lib/Sema/SemaExpr.cpp

6385

Is this really the right place vs in BuildResolvedCallExpr?

BuildResolvedCallExpr does some semantic analysis/check, it might emit diagnostics, which we want to avoid. And putting the logic here also aligns with what C++ code path does.

why DependentTy? shouldn't it be the return type of the function, if available?

oh, yeah, I missed this. Added.

[hokein]

rebase

clang/lib/Sema/SemaExpr.cpp

if (CallingNDeclIndirectly && !checkAddressOfFunctionIsAvailable(
       return ExprError();
 
     if (getLangOpts().OpenCL && checkOpenCLDisabledDecl(*FD, *Fn))
       return ExprError();
 
     checkDirectCallValidity(*this, Fn, FD, ArgExprs);
   }
 
+  if (Context.isDependenceAllowed() &&
+      (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs))) {
+    assert(!getLangOpts().CPlusPlus);
+    assert(Fn->containsErrors() ||
+           llvm::any_of(ArgExprs,
+                        [](clang::Expr *E) { return E->containsErrors(); }) &&
+               "should only occur in error-recovery path.");
+    QualType ReturnType =
+        llvm::isa_and_nonnull<FunctionDecl>(NDecl)
+            ? dyn_cast<FunctionDecl>(NDecl)->getCallResultType()
+            : Context.DependentTy;
+    return CallExpr::Create(Context, Fn, ArgExprs, ReturnType,
+                            Expr::getValueKindForType(ReturnType), RParenLoc,
+                            CurFPFeatureOverrides());
+  }
   return BuildResolvedCallExpr(Fn, NDecl, LParenLoc, ArgExprs, RParenLoc,
                                ExecConfig, IsExecConfig);
 }
 
 /// ActOnAsTypeExpr - create a new asType (bitcast) from the arguments.
 ///
 /// __builtin_astype( value, dst type )
 ///
TheCall = CUDAKernelCallExpr::Create(Context, Fn, cast<CallExpr>(Config),
                                          Args, ResultTy, VK_RValue, RParenLoc,
                                          CurFPFeatureOverrides(), NumParams);
   } else {
     TheCall =
         CallExpr::Create(Context, Fn, Args, ResultTy, VK_RValue, RParenLoc,
                          CurFPFeatureOverrides(), NumParams, UsesADL);
   }
 
-  if (!getLangOpts().CPlusPlus) {
+  if (!Context.isDependenceAllowed()) {
     // Forget about the nulled arguments since typo correction
     // do not handle them well.
     TheCall->shrinkNumArgs(Args.size());
     // C cannot always handle TypoExpr nodes in builtin calls and direct
     // function calls as their argument checking don't necessarily handle
     // dependent types properly, so make sure any TypoExprs have been
     // dealt with.
     ExprResult Result = CorrectDelayedTyposInExpr(TheCall);
static ExprResult diagnoseUnknownAnyExpr(Sema &S, Expr *E) {
 
   // Never recoverable.
   return ExprError();
 }
 
 /// Check for operands with placeholder types and complain if found.
 /// Returns ExprError() if there was an error and no recovery was possible.
 ExprResult Sema::CheckPlaceholderExpr(Expr *E) {
-  if (!getLangOpts().CPlusPlus) {
+  if (!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.
     ExprResult Result = CorrectDelayedTyposInExpr(E);
     if (!Result.isUsable()) return ExprError();
     E = Result.get();
   }
 

clang/test/AST/ast-dump-recovery.c

void test2() {
   // CHECK-NEXT: `-DeclRefExpr {{.*}} 'float' lvalue
   (ptr > f ? ptr : f);
 
   // CHECK:     CStyleCastExpr {{.*}} 'float' contains-errors <Dependent>
   // CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>'
   // CHECK-NEXT:   `-DeclRefExpr {{.*}} 'some_func'
   (float)some_func();
 }
+
+void test3() {
+  // CHECK:     CallExpr {{.*}} '<dependent type>' contains-errors
+  // CHECK-NEXT: |-ParenExpr {{.*}} contains-errors lvalue
+  // CHECK-NEXT: | `-RecoveryExpr {{.*}} contains-errors
+  // CHECK-NEXT: |   `-DeclRefExpr {{.*}} '__builtin_classify_type'
+  // CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
+  (*__builtin_classify_type)(1);
+
+  extern void ext();
+  // CHECK:     CallExpr {{.*}} 'void' contains-errors
+  // CHECK-NEXT: |-DeclRefExpr {{.*}} 'ext'
+  // CHECK-NEXT: `-RecoveryExpr {{.*}} '<dependent type>'
+  ext(undef_var);
+}

clang/test/Sema/error-dependence.c

 // RUN: %clang_cc1 -fsyntax-only -verify -frecovery-ast -fno-recovery-ast-type %s
 
 int call(int); // expected-note3 {{'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}}
 
   // verify diagnostic "operand of type '<dependent type>' where arithmetic or
   // pointer type is required" is not emitted.
   (float)call(); // expected-error {{too few arguments to function call}}
+  // verify disgnostic "called object type '<dependent type>' is not a function
+  // or function pointer" is not emitted.
+  (*__builtin_classify_type)(1); // expected-error {{builtin functions must be directly called}}
 }
 
 void test2(int* ptr, float f) {
   // verify diagnostic "used type '<dependent type>' where arithmetic or pointer
   // type is required" is not emitted.
   (call() ? ptr : f); // expected-error {{too few arguments to function call}}
 }