[clang][Interp] Emit diagnostic when comparing function pointers

[tbaeder]

Summary

Function pointers can be compared for (in)equality but, but LE, GE, LT,
and GT opcodes should emit an error and abort.

[aaron.ballman]

clang/lib/AST/Interp/Interp.h

589–590

Can we pass in the result of getType() instead of doing this string conversion dance?

[tbaeder]

clang/lib/AST/Interp/Interp.h

589–590

Well the diagnostic doesn't print the result of the LHS/RHS:

./array.cpp:202:18: error: constexpr variable 'u13' must be initialized by a constant expression
  202 |   constexpr bool u13 = pf < pg; // ref-warning {{ordered comparison of function pointers}}
      |                  ^     ~~~~~~~
./array.cpp:202:27: note: comparison between '&f' and '&g' has unspecified value
  202 |   constexpr bool u13 = pf < pg; // ref-warning {{ordered comparison of function pointers}}
      |                           ^

I'm not exactly a fan of how the code looks though. I might add a helper function for this later.

[aaron.ballman]

LGTM

clang/lib/AST/Interp/Interp.h

589–590

Ah of course, good point. And yeah, a helper function for this would probably not be a bad idea.

[tbaeder]

clang/lib/AST/Interp/Interp.h

589–590

Do you like the toDiagnosticString() from https://reviews.llvm.org/D149172 better?

[aaron.ballman]

clang/lib/AST/Interp/Interp.h

589–590

I do, that's a nice helper. Feel free to either land those changes and use the function here, or land these changes and do a later NFC commit to the helper method.

clang/lib/AST/Interp/FunctionPointer.h

 
 
 #ifndef LLVM_CLANG_AST_INTERP_FUNCTION_POINTER_H
 #define LLVM_CLANG_AST_INTERP_FUNCTION_POINTER_H
 
 #include "Function.h"
 #include "Primitives.h"
 #include "clang/AST/APValue.h"
 
 namespace clang {
+class ASTContext;
 namespace interp {
 
 class FunctionPointer final {
 private:
   const Function *Func;
 
 public:
   FunctionPointer() : Func(nullptr) {}
void print(llvm::raw_ostream &OS) const {
     OS << "FnPtr(";
     if (Func)
       OS << Func->getName();
     else
       OS << "nullptr";
     OS << ")";
   }
 
+  std::string toDiagnosticString(const ASTContext &Ctx) const {
+    if (!Func)
+      return "nullptr";
+
+    return toAPValue().getAsString(Ctx, Func->getDecl()->getType());
+  }
+
   ComparisonCategoryResult compare(const FunctionPointer &RHS) const {
     if (Func == RHS.Func)
       return ComparisonCategoryResult::Equal;
     return ComparisonCategoryResult::Unordered;
   }
 };
 
 inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,

clang/lib/AST/Interp/Interp.h

bool CmpHelper(InterpState &S, CodePtr OpPC, CompareFn Fn) {
   return true;
 }
 
 template <typename T>
 bool CmpHelperEQ(InterpState &S, CodePtr OpPC, CompareFn Fn) {
   return CmpHelper<T>(S, OpPC, Fn);
 }
 
+/// Function pointers cannot be compared in an ordered way.
+template <>
+inline bool CmpHelper<FunctionPointer>(InterpState &S, CodePtr OpPC,
+                                       CompareFn Fn) {
+  const auto &RHS = S.Stk.pop<FunctionPointer>();
+  const auto &LHS = S.Stk.pop<FunctionPointer>();
+
+  const SourceInfo &Loc = S.Current->getSource(OpPC);
+  S.FFDiag(Loc, diag::note_constexpr_pointer_comparison_unspecified)
+      << LHS.toDiagnosticString(S.getCtx())
+      << RHS.toDiagnosticString(S.getCtx());
+  return false;
+}
+
+template <>
+inline bool CmpHelperEQ<FunctionPointer>(InterpState &S, CodePtr OpPC,
+                                         CompareFn Fn) {
+  const auto &RHS = S.Stk.pop<FunctionPointer>();
+  const auto &LHS = S.Stk.pop<FunctionPointer>();
+  S.Stk.push<Boolean>(Boolean::from(Fn(LHS.compare(RHS))));
+  return true;
+}
+
 template <>
 inline bool CmpHelper<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
   using BoolT = PrimConv<PT_Bool>::T;
   const Pointer &RHS = S.Stk.pop<Pointer>();
   const Pointer &LHS = S.Stk.pop<Pointer>();
 
   if (!Pointer::hasSameBase(LHS, RHS)) {
     const SourceInfo &Loc = S.Current->getSource(OpPC);

clang/lib/AST/Interp/Opcodes.td

def NonPtrTypeClass : TypeClass {
   let Types = !listconcat(IntegerTypeClass.Types, [Bool], [Float]);
 }
 
 def AllTypeClass : TypeClass {
   let Types = !listconcat(AluTypeClass.Types, PtrTypeClass.Types, FloatTypeClass.Types);
 }
 
 def ComparableTypeClass : TypeClass {
-  let Types = !listconcat(AluTypeClass.Types, [Ptr], [Float]);
+  let Types = !listconcat(AluTypeClass.Types, [Ptr], [Float], [FnPtr]);
 }
 
 class SingletonTypeClass<Type Ty> : TypeClass {
   let Types = [Ty];
 }
 
 //===----------------------------------------------------------------------===//
 // Record describing all opcodes.

clang/test/AST/Interp/functions.cpp

struct S {
     int i;
     void (*fp)();
   };
 
   constexpr S s{ 12 };
   static_assert(s.fp == nullptr, ""); // zero-initialized function pointer.
 }
 
+namespace Comparison {
+  void f(), g();
+  constexpr void (*pf)() = &f, (*pg)() = &g;
+
+  constexpr bool u13 = pf < pg; // ref-warning {{ordered comparison of function pointers}} \
+                                // ref-error {{must be initialized by a constant expression}} \
+                                // ref-note {{comparison between '&f' and '&g' has unspecified value}} \
+                                // expected-warning {{ordered comparison of function pointers}} \
+                                // expected-error {{must be initialized by a constant expression}} \
+                                // expected-note {{comparison between '&f' and '&g' has unspecified value}}
+
+  constexpr bool u14 = pf < (void(*)())nullptr; // ref-warning {{ordered comparison of function pointers}} \
+                                                // ref-error {{must be initialized by a constant expression}} \
+                                                // ref-note {{comparison between '&f' and 'nullptr' has unspecified value}} \
+                                                // expected-warning {{ordered comparison of function pointers}} \
+                                                // expected-error {{must be initialized by a constant expression}} \
+                                                // expected-note {{comparison between '&f' and 'nullptr' has unspecified value}}
+
+
+
+  static_assert(pf != pg, "");
+  static_assert(pf == &f, "");
+  static_assert(pg == &g, "");
+}
+
 }
 
 struct F {
   constexpr bool ok() const {
     return okRecurse();
   }
   constexpr bool okRecurse() const {
     return true;