[clang][AST] Improve diagnostic for `nullptr` constexpr function pointer call

[hazohelet]

Summary

This patch improves diagnostic for clang constexpr evaluator by adding a check for nullptr in function pointer call evaluations.
This fixes https://github.com/llvm/llvm-project/issues/59872

ex.

constexpr int foo(int (*bla)(void)) {
  return bla();
}

static_assert(foo(nullptr) == 1);

BEFORE this patch, clang generates the following diagnostic for the code above

<source>:5:15: error: static assertion expression is not an integral constant expression
static_assert(foo(nullptr) == 1);
              ^~~~~~~~~~~~~~~~~
<source>:2:10: note: subexpression not valid in a constant expression
  return bla();
         ^
<source>:5:15: note: in call to 'foo(nullptr)'
static_assert(foo(nullptr) == 1);
              ^
1 error generated.

AFTER this patch, subexpression not valid in a constant expression note is replaced with 'bla' evaluates to a null function pointer.

[tbaeder]

clang/lib/AST/ExprConstant.cpp

7673

Is the const_cast really necessary?

[hazohelet]

clang/lib/AST/ExprConstant.cpp

7673

Is the const_cast really necessary?

Without const_cast, it did not compile.
I searched the existing codebase to find this line https://github.com/llvm/llvm-project/blob/151d3b607e1e3256ed901e02b48b92e79a77021d/clang/lib/Sema/SemaConcept.cpp#L300 and I did the same here.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticASTKinds.td

131

Small rewording.

[shafik]

clang/lib/AST/ExprConstant.cpp

7673

Yeah, we do seem to const_cast on const Expr* all over and in some places it is obviously harmless but others it is far from clear, at least on a cursory examination.

[cjdb]

LGTM pending other reviewers' commentary. Thanks for working on this!

[hazohelet]

Address comment from @aaron.ballman

• Rewording the diagnostic

[aaron.ballman]

LGTM, though please add a release note for the fix. If you need someone to land on your behalf, let us know what name and email address you'd like used for patch attribution.

clang/lib/AST/ExprConstant.cpp

7673

Clang has historically had very poor const-correctness, and trying to retrofit it is always an exercise in avoiding viral changes. So new interfaces tend to be more likely to be const-correct, while older ones (like the diagnostics engine) tend to not be.

The const_cast<> looks necessary to me, but if someone wanted to explore fixing up the interface so we can remove the const_casts from multiple places (as a follow-up patch, not as part of this work), that could be fruitful.

[hazohelet]

Added a release note

[hazohelet]

In D145793#4189252, @aaron.ballman wrote:

LGTM, though please add a release note for the fix. If you need someone to land on your behalf, let us know what name and email address you'd like used for patch attribution.

Yeah, I don't have commit access, so I need someone to commit for me.
Please use "Takuya Shimizu <[e-mail]>" for the patch attribution. Thank you!

clang/include/clang/Basic/DiagnosticASTKinds.td

def note_constexpr_past_end_subobject : Note<
   "pointer past the end of object">;
 def note_non_null_attribute_failed : Note<
   "null passed to a callee that requires a non-null argument">;
 def note_constexpr_null_subobject : Note<
   "cannot %select{access base class of|access derived class of|access field of|"
   "access array element of|perform pointer arithmetic on|"
   "access real component of|"
   "access imaginary component of}0 null pointer">;
+def note_constexpr_null_callee : Note<
+  "function pointer '%0' evaluates to a null pointer">;
 def note_constexpr_function_param_value_unknown : Note<
   "function parameter %0 with unknown value cannot be used in a constant "
   "expression">;
 def note_constexpr_var_init_unknown : Note<
   "initializer of %0 is unknown">;
 def note_constexpr_var_init_non_constant : Note<
   "initializer of %0 is not a constant expression">;
 def note_constexpr_var_init_weak : Note<

clang/lib/AST/ExprConstant.cpp

if (CalleeType->isSpecificBuiltinType(BuiltinType::BoundMember)) {
       FD = Member;
     } else if (CalleeType->isFunctionPointerType()) {
       LValue CalleeLV;
       if (!EvaluatePointer(Callee, CalleeLV, Info))
         return false;
 
       if (!CalleeLV.getLValueOffset().isZero())
         return Error(Callee);
+      if (CalleeLV.isNullPointer()) {
+        Info.FFDiag(Callee, diag::note_constexpr_null_callee)
+            << const_cast<Expr *>(Callee);
+        return false;
+      }
       FD = dyn_cast_or_null<FunctionDecl>(
           CalleeLV.getLValueBase().dyn_cast<const ValueDecl *>());
       if (!FD)
         return Error(Callee);
       // Don't call function pointers which have been cast to some other type.
       // Per DR (no number yet), the caller and callee can differ in noexcept.
       if (!Info.Ctx.hasSameFunctionTypeIgnoringExceptionSpec(
         CalleeType->getPointeeType(), FD->getType())) {

clang/test/SemaCXX/constant-expression-cxx11.cpp

namespace FunctionPointers {
 
   constexpr int Double(int n) { return 2 * n; }
   constexpr int Triple(int n) { return 3 * n; }
   constexpr int Twice(int (*F)(int), int n) { return F(F(n)); }
   constexpr int Quadruple(int n) { return Twice(Double, n); }
   constexpr auto Select(int n) -> int (*)(int) {
     return n == 2 ? &Double : n == 3 ? &Triple : n == 4 ? &Quadruple : 0;
   }
-  constexpr int Apply(int (*F)(int), int n) { return F(n); } // expected-note {{subexpression}}
+  constexpr int Apply(int (*F)(int), int n) { return F(n); } // expected-note {{function pointer 'F' evaluates to a null pointer}}
 
   static_assert(1 + Apply(Select(4), 5) + Apply(Select(3), 7) == 42, "");
 
   constexpr int Invalid = Apply(Select(0), 0); // expected-error {{must be initialized by a constant expression}} expected-note {{in call to 'Apply(nullptr, 0)'}}
 
 }
 
 namespace PointerComparison {