[clang] Allow comparing pointers to string literals

[tbaeder]

Summary

This fixes https://github.com/llvm/llvm-project/issues/58754

[tahonermann]

clang/lib/AST/ExprConstant.cpp

12951–12954

A comment to explain this change would be helpful. It isn't intuitive (for me anyway) why Objective-C's @encode would require special handling here. Was this needed to avoid a test failure?

[tbaeder]

clang/lib/AST/ExprConstant.cpp

12951–12954

In ../clang/test/CodeGenObjC/encode-test-4.m:

int a(void) {
  return @encode(int) == @encode(int) &&
    @encode(Color) == @encode(long) &&
    @encode(PrintColor) == @encode(int);
}

The comparisons need to be rejected here and are folded to a 1 later on, it seems. Letting the comparison happen will lead to a 0.

[tahonermann]

clang/lib/AST/ExprConstant.cpp

12951–12954

Apologies, I meant it would be helpful to add a comment to the code :)

I think @encode(...) expressions should be treated the same as string literals; the former is lowered to the latter; both contribute to the string table.

[tbaeder]

clang/lib/AST/ExprConstant.cpp

12951–12954

You mean the special case for them here should go away?

I've tracked this down to CodeGenFunction::ConstantFoldsToSimpleInteger(). Without this special case for endcode expressions, we will return true here and fold the comparison to false, so we will just emit that.

This is a funny case though, which also exists in C++. With this patch, "a" == "a" evaluates to true, as one would expect.
However, "a" == "a" && "b" == "b" will evaluate to false.

That's pretty wrong so I think the patch needs more work.

[tahonermann]

clang/lib/AST/ExprConstant.cpp

12951–12954

Should "a" == "a" evaluate to true? The standard seems clear that the behavior is unspecified ((lex.string)p9). Is there a special case for constexpr context somewhere? I see that gcc, icc, and MSVC all compare them equally in constexpr context, but that doesn't mean that Clang needs to.

Gcc appears to treat Objective-C @encode() expressions equivalently to string literals in constexpr context. https://godbolt.org/z/Kxz3E8xKa

clang/lib/AST/ExprConstant.cpp

if (!HasSameBase(LHSValue, RHSValue)) {
         return false;
       }
       // A constant address may compare equal to the address of a symbol.
       // The one exception is that address of an object cannot compare equal
       // to a null pointer constant.
       if ((!LHSValue.Base && !LHSValue.Offset.isZero()) ||
           (!RHSValue.Base && !RHSValue.Offset.isZero()))
         return Error(E);
-      // It's implementation-defined whether distinct literals will have
-      // distinct addresses. In clang, the result of such a comparison is
-      // unspecified, so it is not a constant expression. However, we do know
-      // that the address of a literal will be non-null.
-      if ((IsLiteralLValue(LHSValue) || IsLiteralLValue(RHSValue)) &&
-          LHSValue.Base && RHSValue.Base)
+
+      // ObjC's @encode()
+      if (isa<ObjCEncodeExpr>(E->getLHS()->IgnoreParenImpCasts()) ||
+          isa<ObjCEncodeExpr>(E->getRHS()->IgnoreParenImpCasts()))
         return Error(E);
+
       // We can't tell whether weak symbols will end up pointing to the same
       // object.
       if (IsWeakLValue(LHSValue) || IsWeakLValue(RHSValue))
         return Error(E);
       // We can't compare the address of the start of one object with the
       // past-the-end address of another object, per C++ DR1652.
       if ((LHSValue.Base && LHSValue.Offset.isZero() &&
            isOnePastTheEndOfCompleteObject(Info.Ctx, RHSValue)) ||

clang/test/SemaCXX/builtins.cpp

namespace addressof {
 
   S *ptmp = __builtin_addressof(S{}); // expected-error {{taking the address of a temporary}}
 }
 
 namespace function_start {
 void a(void) {}
 int n;
 void *p = __builtin_function_start(n);               // expected-error {{argument must be a function}}
-static_assert(__builtin_function_start(a) == a, ""); // expected-error {{static assertion expression is not an integral constant expression}}
+static_assert(__builtin_function_start(a) == a, ""); // expected-error {{static assertion failed}}
 } // namespace function_start
 
 void no_ms_builtins() {
   __assume(1); // expected-error {{use of undeclared}}
   __noop(1); // expected-error {{use of undeclared}}
   __debugbreak(); // expected-error {{use of undeclared}}
 }
 

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

int f : &(U&)(*sptr) == &(U&)(*sptr); // \
     expected-note {{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
   int g : (S*)(void*)(sptr) == sptr; // \
     expected-warning {{not an integral constant expression}} \
     expected-note {{cast from 'void *' is not allowed in a constant expression}}
 };
 
 extern char externalvar[];
 constexpr bool constaddress = (void *)externalvar == (void *)0x4000UL; // expected-error {{must be initialized by a constant expression}} expected-note {{reinterpret_cast}}
-constexpr bool litaddress = "foo" == "foo"; // expected-error {{must be initialized by a constant expression}}
-// cxx20_2b-warning@-1 {{comparison between two arrays is deprecated}}
+constexpr bool litaddress = "foo" == "foo"; // cxx20_2b-warning {{comparison between two arrays is deprecated}}
 static_assert(0 != "foo", "");
 
 }
 
+constexpr const char *foo(const char *p) { return p; }
+constexpr const char *p1 = "test1";
+constexpr const char *p2 = "test2";
+
+constexpr bool b1 = foo(p1) == foo(p1);
+static_assert(b1, "");
+
+constexpr bool b2 = foo(p1) == foo(p2);
+static_assert(!b2, "");
+
+constexpr const char *name1() { return "name1"; }
+constexpr const char *name2() { return "name2"; }
+
+constexpr auto b3 = name1() == name1();
+static_assert(b3, "");
+constexpr auto b4 = name1() == name2();
+static_assert(!b4, "");
+
 namespace MaterializeTemporary {
 
 constexpr int f(const int &r) { return r; }
 constexpr int n = f(1);
 
 constexpr bool same(const int &a, const int &b) { return &a == &b; }
 constexpr bool sameTemporary(const int &n) { return same(n, n); }
 

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

constexpr DoBadThings dbt = { // expected-error {{constant expression}}
     (dbt.wp = get_bad_pointer(), 0)
   };
 
   constexpr DoBadThings dbt2 = { // ok
     get_bad_pointer(),
     (dbt2.wp = nullptr, 0)
   };
 }
+
+
+constexpr auto name1() { return "name1"; }
+constexpr auto name2() { return "name2"; }
+
+constexpr auto b3 = name1() == name1();
+static_assert(b3, "");
+constexpr auto b4 = name1() == name2();
+static_assert(!b4, "");