Skip to content

[clang][ExprConst] Allow comparisons with string literals #106733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

tbaederr
Copy link
Contributor

Don't diagnose them, but literals still have distinct addresses.

Fixes #58754

@tbaederr tbaederr added clang:frontend Language frontend issues, e.g. anything involving "Sema" constexpr Anything related to constant evaluation labels Aug 30, 2024
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Aug 30, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 30, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Don't diagnose them, but literals still have distinct addresses.

Fixes #58754


Full diff: https://github.com/llvm/llvm-project/pull/106733.diff

3 Files Affected:

  • (modified) clang/lib/AST/ExprConstant.cpp (+1-1)
  • (modified) clang/test/AST/ByteCode/cxx20.cpp (+4-10)
  • (modified) clang/test/SemaCXX/constant-expression-cxx11.cpp (+2-3)
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e8a4d1d3c74102..869745db32f443 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2142,7 +2142,7 @@ static bool IsLiteralLValue(const LValue &Value) {
   if (Value.getLValueCallIndex())
     return false;
   const Expr *E = Value.Base.dyn_cast<const Expr*>();
-  return E && !isa<MaterializeTemporaryExpr>(E);
+  return E && !isa<MaterializeTemporaryExpr, StringLiteral>(E);
 }
 
 static bool IsWeakLValue(const LValue &Value) {
diff --git a/clang/test/AST/ByteCode/cxx20.cpp b/clang/test/AST/ByteCode/cxx20.cpp
index 77a967d42c4efe..13517a059f340c 100644
--- a/clang/test/AST/ByteCode/cxx20.cpp
+++ b/clang/test/AST/ByteCode/cxx20.cpp
@@ -108,22 +108,16 @@ constexpr auto p2 = "test2";
 constexpr bool b1 = foo(p1) == foo(p1);
 static_assert(b1);
 
-constexpr bool b2 = foo(p1) == foo(p2); // ref-error {{must be initialized by a constant expression}} \
-                                        // ref-note {{comparison of addresses of literals}} \
-                                        // ref-note {{declared here}}
-static_assert(!b2); // ref-error {{not an integral constant expression}} \
-                    // ref-note {{not a constant expression}}
+constexpr bool b2 = foo(p1) == foo(p2);
+static_assert(!b2);
 
 constexpr auto name1() { return "name1"; }
 constexpr auto name2() { return "name2"; }
 
 constexpr auto b3 = name1() == name1();
 static_assert(b3);
-constexpr auto b4 = name1() == name2(); // ref-error {{must be initialized by a constant expression}} \
-                                        // ref-note {{has unspecified value}} \
-                                        // ref-note {{declared here}}
-static_assert(!b4); // ref-error {{not an integral constant expression}} \
-                    // ref-note {{not a constant expression}}
+constexpr auto b4 = name1() == name2();
+static_assert(!b4);
 
 namespace UninitializedFields {
   class A {
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 44ef540f41fa8c..9997babfead09f 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -358,10 +358,9 @@ struct Str {
 
 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}}
-// expected-note@-1 {{comparison of addresses of literals has unspecified value}}
-// cxx20_23-warning@-2 {{comparison between two arrays is deprecated}}
+constexpr bool litaddress = "foo" == "foo"; // cxx20_23-warning {{comparison between two arrays is deprecated}}
 static_assert(0 != "foo", "");
+static_assert("foo" != "foo", "");
 
 }
 

@tbaederr tbaederr force-pushed the stringliteral-comparison branch from b77e5f9 to 73af678 Compare September 6, 2024 11:38
@tbaederr tbaederr force-pushed the stringliteral-comparison branch from 73af678 to 7dbb018 Compare September 6, 2024 12:04
@tbaederr
Copy link
Contributor Author

tbaederr commented Sep 6, 2024

Ping

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tbaederr tbaederr merged commit 5d1d2f0 into llvm:main Sep 13, 2024
8 checks passed
@zygoloid
Copy link
Collaborator

See #58754 (comment) -- this introduces a miscompile by producing comparison results at compile time that differ from the results at runtime. Please can this be reverted?

tbaederr added a commit that referenced this pull request Sep 17, 2024
tmsri pushed a commit to tmsri/llvm-project that referenced this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category constexpr Anything related to constant evaluation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

c++20/libstdc++: char_traits.h:213:13: note: subexpression not valid in a constant expression
4 participants