Page MenuHomePhabricator

[clang] Allow comparing pointers to string literals
Needs ReviewPublic

Authored by tbaeder on Nov 11 2022, 3:46 AM.

Diff Detail

Unit TestsFailed

TimeTest
100 msx64 debian > Clang.AST/Interp::cxx20.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/16/include -nostdsysteminc -fexperimental-new-constant-interpreter -std=c++20 -verify /var/lib/buildkite-agent/builds/llvm-project/clang/test/AST/Interp/cxx20.cpp

Event Timeline

tbaeder created this revision.Nov 11 2022, 3:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2022, 3:46 AM
tbaeder requested review of this revision.Nov 11 2022, 3:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2022, 3:46 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tahonermann added inline comments.
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 added inline comments.Nov 17 2022, 12:07 AM
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 added inline comments.Nov 18 2022, 4:17 PM
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 added inline comments.Nov 18 2022, 11:12 PM
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 added inline comments.Dec 7 2022, 2:53 PM
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