[Clang] Always constant-evaluate operands of comparisons to nullptr

[cor3ntin]

Summary

even if we know what the result is going to be.
There may be side effects we ought not to ignore,

Fixes #64923

[tbaeder]

clang/lib/AST/ExprConstant.cpp

13316

I kinda expected the fix for this bug to simply remove a special case... does just falling through to the DoAfter() call not work?

[cor3ntin]

clang/lib/AST/ExprConstant.cpp

13316

Nope, EvaluateComparisonBinaryOperator does compute the result of the comparison. DoAfter only adjust the result.
Removing that block of codes lead to a whole bunch of test failures

[tbaeder]

clang/lib/AST/ExprConstant.cpp

13316

Shame, but ok.

[ChuanqiXu]

LGTM. Thanks.

[cor3ntin]

@ChuanqiXu Thanks. Do you think this needs to be backported for coroutine support?

[ChuanqiXu]

In D158601#4612908, @cor3ntin wrote:

@ChuanqiXu Thanks. Do you think this needs to be backported for coroutine support?

For coroutine support, I don't think it is needed. It is a pretty corner case and Tobias had mentioned there are too many back ports now (https://discourse.llvm.org/t/llvm-17-0-0-rc3-released/72946). Although he doesn't throw any requirement to us, I think it may better to lift our bar for backporting patches now.

[cor3ntin]

Gotcha. Given there are other stuffs I need to backport, this will have to wait clang 18! I don't want to give undue work to tobias and testers.

[cor3ntin]

Rebase

[shafik]

It might have been nice to add a test where nullptr_t comparison succeeds as well.

clang/docs/ReleaseNotes.rst

 
 - Clang emits an error on substitution failure within lambda body inside a
   requires-expression. This fixes:
   (`#64138 <https://github.com/llvm/llvm-project/issues/64138>`_).
 
 - Update ``FunctionDeclBitfields.NumFunctionDeclBits``. This fixes:
   (`#64171 <https://github.com/llvm/llvm-project/issues/64171>`_).
 
+- Expressions producing ``nullptr`` are correctly evaluated
+  by the constant interpreter when appearing as the operand
+  of a binary comparision.
+  (`#64923 <https://github.com/llvm/llvm-project/issues/64923>_``)
+
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed an import failure of recursive friend class template.
   `Issue 64169 <https://github.com/llvm/llvm-project/issues/64169>`_
 - Remove unnecessary RecordLayout computation when importing UnaryOperator. The
   computed RecordLayout is incorrect if fields are not completely imported and
   should not be cached.
   `Issue 64170 <https://github.com/llvm/llvm-project/issues/64170>`_

clang/lib/AST/ExprConstant.cpp

EvaluateComparisonBinaryOperator(EvalInfo &Info, const BinaryOperator *E,
   }
 
   if (LHSTy->isNullPtrType()) {
     assert(E->isComparisonOp() && "unexpected nullptr operation");
     assert(RHSTy->isNullPtrType() && "missing pointer conversion");
     // C++11 [expr.rel]p4, [expr.eq]p3: If two operands of type std::nullptr_t
     // are compared, the result is true of the operator is <=, >= or ==, and
     // false otherwise.
+    LValue Res;
+    if (!EvaluatePointer(E->getLHS(), Res, Info) ||
+        !EvaluatePointer(E->getRHS(), Res, Info))
+      return false;
     return Success(CmpResult::Equal, E);
   }
 
   return DoAfter();
 }
 
 bool RecordExprEvaluator::VisitBinCmp(const BinaryOperator *E) {
   if (!CheckLiteralType(Info, E))

clang/test/SemaCXX/compare-cxx2a.cpp

namespace PR52537 {
   const Y<int[5]> y5;
   template<typename T> bool f5() { return y5 < 0; } // expected-note {{instantiation of}}
   void g5() { f5<int>(); } // expected-note {{instantiation of}}
 
   const Y<int[6]> y6;
   template<typename T> bool f6() { return 0 < y6; } // expected-note {{instantiation of}}
   void g6() { f6<int>(); } // expected-note {{instantiation of}}
 }
+
+
+namespace GH64923 {
+using nullptr_t = decltype(nullptr);
+struct MyTask{};
+constexpr MyTask DoAnotherThing() {
+    return {};
+}
+
+constexpr nullptr_t operator++(MyTask &&T); // expected-note 2{{declared here}}
+void DoSomething() {
+  if constexpr (++DoAnotherThing() != nullptr) {} // expected-error {{constexpr if condition is not a constant expression}} \
+                                                  // expected-note  {{undefined function 'operator++' cannot be used in a constant expression}}
+
+  if constexpr (nullptr == ++DoAnotherThing()) {} // expected-error {{constexpr if condition is not a constant expression}} \
+                                                  // expected-note  {{undefined function 'operator++' cannot be used in a constant expression}}
+}
+}