[clang][ConstExprEmitter] handle NullToPointer ImplicitCastExpr

[nickdesaulniers]

Summary

Consider the following statement:

void* foo = ((void *)0);

For the sub-AST:

| `-ImplicitCastExpr 'const void *' <NullToPointer>
|   `-CStyleCastExpr 'void *' <NullToPointer>
|     `-IntegerLiteral 'int' 0

If the subexpression of the cast is itself the NULL constant, then
ImplicitCastExpr should emit the NULL pointer constant.

[nickdesaulniers]

clang/lib/CodeGen/CGExprConstant.cpp

1131–1132

FWIW, I would have thought these might be unnecessary, but we do trip 2 tests without these guards IIRC.

[efriedma]

clang/lib/CodeGen/CGExprConstant.cpp

1131–1132

The fact that the Visit can fail doesn't surprise me; you can write arbitrary expressions of type nullptr_t (for example, nullptr_t f(); void* g = f();. I can't think of any reason you'd need to check C->isNullValue(), though; do you have a testcase for that?

[nickdesaulniers]

clang/lib/CodeGen/CGExprConstant.cpp

1131–1132

Failed Tests (2):

Clang :: CodeGenCXX/const-init-cxx11.cpp
Clang :: CodeGenCXX/cxx11-thread-local-instantiated.cpp

Looking at the first:

decltype(nullptr) null();
int *p = null();

and the corresponding AST:

|-FunctionDecl 0x5650ebbb44a8 <x.cpp:2:3, col:26> col:21 used null 'decltype(nullptr) ()'
`-VarDecl 0x5650ebbb45e0 <line:3:3, col:17> col:8 p 'int *' cinit
  `-ImplicitCastExpr 0x5650ebbb4740 <col:12, col:17> 'int *' <NullToPointer>
    `-CallExpr 0x5650ebbb4720 <col:12, col:17> 'decltype(nullptr)':'std::nullptr_t'
      `-ImplicitCastExpr 0x5650ebbb4708 <col:12> 'decltype(nullptr) (*)()' <FunctionToPointerDecay>
        `-DeclRefExpr 0x5650ebbb4690 <col:12> 'decltype(nullptr) ()' lvalue Function 0x5650ebbb44a8 'null' 'decltype(nullptr) ()'

we change the existing test case from having a __cxx_global_var_init routine, i.e.:

@p = dso_local global ptr null, align 8
@llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I_x.cpp, ptr null }]

; Function Attrs: noinline uwtable
define internal void @__cxx_global_var_init() #0 section ".text.startup" {
entry:
  %call = call ptr @_Z4nullv()
  store ptr null, ptr @p, align 8
  ret void
}

declare ptr @_Z4nullv() #1

; Function Attrs: noinline uwtable
define internal void @_GLOBAL__sub_I_x.cpp() #0 section ".text.startup" {
entry:
  call void @__cxx_global_var_init()
  ret void
}

to simply:

@p = dso_local global ptr null, align 8

So it seems nice to skip having the static constructors, but then @_Z4nullv is never invoked. That seems problematic? (I expect the fast path to fail once it recurses down to the DeclRefExpr, but that requires the subexpr of the ImplicitCastExpr be visited).

---

So I think the code as written is good to go. WDYT?

[efriedma]

clang/lib/CodeGen/CGExprConstant.cpp

1131–1132

I think you need to check that Visit() succeeds (returns a non-null Constant*), but like I said before, I can't see why you need to check C->isNullValue()

[nickdesaulniers]

• remove the check for isNullValue as per @efriedma

clang/lib/CodeGen/CGExprConstant.cpp

1131–1132

oh, sorry, my reading comprehension skills are sliding off a cliff.

Yeah, that's not necessary for test cases in tree. Removed.

[nickdesaulniers]

• rebase

[efriedma]

LGTM

clang/lib/CodeGen/CGExprConstant.cpp

llvm::Constant *VisitCastExpr(CastExpr *E, QualType destType) {
 
     case CK_AtomicToNonAtomic:
     case CK_NonAtomicToAtomic:
     case CK_NoOp:
     case CK_ConstructorConversion:
       return Visit(subExpr, destType);
 
     case CK_ArrayToPointerDecay:
       if (const auto *S = dyn_cast<StringLiteral>(subExpr))
         return CGM.GetAddrOfConstantStringFromLiteral(S).getPointer();
       return nullptr;
+    case CK_NullToPointer:
+      if (llvm::Constant *C = Visit(subExpr, destType))
+        return CGM.EmitNullConstant(destType);
+      return nullptr;
 
     case CK_IntToOCLSampler:
       llvm_unreachable("global sampler variables are not generated");
 
     case CK_Dependent: llvm_unreachable("saw dependent cast!");
 
     case CK_BuiltinFnToFnPtr:
       llvm_unreachable("builtin functions are handled elsewhere");
llvm::Constant *VisitCastExpr(CastExpr *E, QualType destType) {
     case CK_FloatingComplexToIntegralComplex:
     case CK_IntegralRealToComplex:
     case CK_IntegralComplexToReal:
     case CK_IntegralComplexToBoolean:
     case CK_IntegralComplexCast:
     case CK_IntegralComplexToFloatingComplex:
     case CK_PointerToIntegral:
     case CK_PointerToBoolean:
-    case CK_NullToPointer:
     case CK_IntegralCast:
     case CK_BooleanToSignedIntegral:
     case CK_IntegralToPointer:
     case CK_IntegralToBoolean:
     case CK_IntegralToFloating:
     case CK_FloatingToIntegral:
     case CK_FloatingToBoolean:
     case CK_FloatingCast: