[AST][RecoveryExpr] Error-dependent expression should not be treat as a nullptr pointer constant.

[hokein]

Summary

If an expression is contains-error and its type is unknown (dependent), we
don't treat it as a null pointer constant.

Fix a recovery-ast crash on C.

[hokein]

dump cast kind.

[hokein]

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

51

For references, the crash stacktrace is like below, the cause is that we run into the codepath.

can't implicitly cast lvalue to rvalue with this cast kind: NullToPointer
UNREACHABLE executed at llvm-project/clang/lib/Sema/Sema.cpp:545!
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: ./bin/clang -cc1 -fsyntax-only -ast-dump -frecovery-ast /tmp/t.c 
1.      /tmp/t.c:78:20: current parser token ';'
2.      /tmp/t.c:71:16: parsing function body 'NoCrash'
#13 0x0000000003e75e59 clang::Sema::ImpCastExprToType(clang::Expr*, clang::QualType, clang::CastKind, clang::ExprValueKind, llvm::SmallVector<clang::CXXBaseSpecifier*, 4u> const*, clang::Sema::CheckedConversionKind) llvm-project/clang/lib/Sema/Sema.cpp:582:16
#14 0x0000000004132c06 clang::Sema::CheckSingleAssignmentConstraints(clang::QualType, clang::ActionResult<clang::Expr*, true>&, bool, bool, bool) llvm-project/clang/lib/Sema/SemaExpr.cpp:9374:13
#15 0x00000000041420f6 clang::Sema::CheckAssignmentOperands(clang::Expr*, clang::ActionResult<clang::Expr*, true>&, clang::SourceLocation, clang::QualType) llvm-project/clang/lib/Sema/SemaExpr.cpp:12759:9
#16 0x000000000411e348 clang::Sema::CreateBuiltinBinOp(clang::SourceLocation, clang::BinaryOperatorKind, clang::Expr*, clang::Expr*) llvm-project/clang/lib/Sema/SemaExpr.cpp:0:16
#17 0x00000000041060ba clang::Sema::ActOnBinOp(clang::Scope*, clang::SourceLocation, clang::tok::TokenKind, clang::Expr*, clang::Expr*) llvm-project/clang/lib/Sema/SemaExpr.cpp:14170:1

[sammccall]

clang/lib/AST/Expr.cpp

3750

would it be clearer to say if (containsErrors()) return NPCK_NotNull at the top?

Only difference in behavior is not hitting llvm_unreachable if we have NPC_NeverValueDependent... do you think we actually want that?

[hokein]

address comment.

[hokein]

clang/lib/AST/Expr.cpp

3750

this is good point, I missed that. thanks!

clang/lib/AST/Expr.cpp

 /// Null pointer constants can be integer constant expressions with the
 /// value zero, casts of zero to void*, nullptr (C++0X), or __null
 /// (a GNU extension).
 Expr::NullPointerConstantKind
 Expr::isNullPointerConstant(ASTContext &Ctx,
                             NullPointerConstantValueDependence NPC) const {
   if (isValueDependent() &&
       (!Ctx.getLangOpts().CPlusPlus11 || Ctx.getLangOpts().MSVCCompat)) {
+    // Error-dependent expr should never be a null pointer.
+    if (containsErrors())
+      return NPCK_NotNull;
     switch (NPC) {
     case NPC_NeverValueDependent:
       llvm_unreachable("Unexpected value dependent expression!");
     case NPC_ValueDependentIsNull:
       if (isTypeDependent() || getType()->isIntegralType(Ctx))
         return NPCK_ZeroExpression;
       else
         return NPCK_NotNull;
 
     case NPC_ValueDependentIsNotNull:
       return NPCK_NotNull;
     }
   }

clang/lib/Sema/Sema.cpp

 ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
                                    CastKind Kind, ExprValueKind VK,
                                    const CXXCastPath *BasePath,
                                    CheckedConversionKind CCK) {
 #ifndef NDEBUG
   if (VK == VK_RValue && !E->isRValue()) {
     switch (Kind) {
     default:
-      llvm_unreachable("can't implicitly cast lvalue to rvalue with this cast "
-                       "kind");
+      llvm_unreachable(("can't implicitly cast lvalue to rvalue with this cast "
+                        "kind: " +
+                        std::string(CastExpr::getCastKindName(Kind)))
+                           .c_str());
     case CK_Dependent:
     case CK_LValueToRValue:
     case CK_ArrayToPointerDecay:
     case CK_FunctionToPointerDecay:
     case CK_ToVoid:
     case CK_NonAtomicToAtomic:
       break;
     }

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

 // this will be fixed when we support dependent mechanism and delayed typo correction for C.
 int ternary = a ? undef : a;
 
 void test1() {
   // CHECK:     `-RecoveryExpr {{.*}} contains-errors
   // CHECK-NEXT:  `-DeclRefExpr {{.*}} 'a' 'const int'
   static int foo = a++; // verify no crash on local static var decl.
 }
+
+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'
+  ptr = some_func(); // should not crash
+}