[PATCH] D154559: [clang] Fix constant evaluation about static member function

2023-08-07 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith requested changes to this revision.
rsmith added a comment.
This revision now requires changes to proceed.

Clang was correct here until fairly recently; P2280 
 
(https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2280r4.html) changed 
the language rules. It was applied as a DR, so we should make that change 
retroactively rather than only in C++23 mode. See 
https://github.com/llvm/llvm-project/issues/63139, which tracks implementation 
of that language change.

I don't think we should be applying an ad-hoc patch like this which doesn't 
follow either the old or the new language rule, and should instead implement 
the new rule from P2280 .


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154559/new/

https://reviews.llvm.org/D154559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154559: [clang] Fix constant evaluation about static member function

2023-07-18 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

@rsmith Richard, PTAL. This needs your language lawyering expertise.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154559/new/

https://reviews.llvm.org/D154559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154559: [clang] Fix constant evaluation about static member function

2023-07-18 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154559/new/

https://reviews.llvm.org/D154559

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D154559: [clang] Fix constant evaluation about static member function

2023-07-05 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra, rsmith.
Herald added a project: All.
yaxunl requested review of this revision.

Currently, clang does not allow static constexpr member
functions called through a const reference of an object
in constant expression, e.g. the following code

  class static_multimap{
  public:
  static constexpr int size() noexcept{
  return 8;
  }
  };
  
  template 
  void test_non_shmem_pair_retrieve(Map& map){
  auto constexpr cg_size = map.size();
  }
  
  int main(){
  static_multimap map;
  test_non_shmem_pair_retrieve(map);
  return 0;
  }

fails to compile with clang. (https://godbolt.org/z/T17vTWYcs)

This does not make sense since the evaluation of map.size
does not rely on map. The same code compiles with GCC.


https://reviews.llvm.org/D154559

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2379,15 +2379,19 @@
   template void f1(T t) {
 constexpr int k = t.size();
   }
-  template void f2(const T ) { // expected-note 2{{declared 
here}}
-constexpr int k = t.size(); // expected-error 2{{constant}} expected-note 
2{{function parameter 't' with unknown value cannot be used in a constant 
expression}}
+  template void f2a(const T ) {
+constexpr int k1 = t.size();
+constexpr int k2 = (t).size();
+  }
+  template void f2b(const T ) { // expected-note {{declared 
here}}
+constexpr int k = t.size(); // expected-error {{constant}} expected-note 
{{function parameter 't' with unknown value cannot be used in a constant 
expression}}
   }
   template void f3(const T ) {
 constexpr int k = T::size();
   }
   void g(array<3> a) {
 f1(a);
-f2(a); // expected-note {{instantiation of}}
+f2a(a);
 f3(a);
   }
 
@@ -2396,7 +2400,7 @@
   };
   void h(array_nonstatic<3> a) {
 f1(a);
-f2(a); // expected-note {{instantiation of}}
+f2b(a); // expected-note {{instantiation of}}
   }
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -8031,6 +8031,15 @@
 
   /// Potentially visit a MemberExpr's base expression.
   void VisitIgnoredBaseExpression(const Expr *E) {
+// If E is a reference, and if so, there is no side effect.
+if (const DeclRefExpr *DRE =
+dyn_cast(E->IgnoreParenImpCasts())) {
+  if (const VarDecl *VD = dyn_cast(DRE->getDecl())) {
+if (VD->getType()->isReferenceType()) {
+  return;
+}
+  }
+}
 // While MSVC doesn't evaluate the base expression, it does diagnose the
 // presence of side-effecting behavior.
 if (Info.getLangOpts().MSVCCompat && !E->HasSideEffects(Info.Ctx))


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2379,15 +2379,19 @@
   template void f1(T t) {
 constexpr int k = t.size();
   }
-  template void f2(const T ) { // expected-note 2{{declared here}}
-constexpr int k = t.size(); // expected-error 2{{constant}} expected-note 2{{function parameter 't' with unknown value cannot be used in a constant expression}}
+  template void f2a(const T ) {
+constexpr int k1 = t.size();
+constexpr int k2 = (t).size();
+  }
+  template void f2b(const T ) { // expected-note {{declared here}}
+constexpr int k = t.size(); // expected-error {{constant}} expected-note {{function parameter 't' with unknown value cannot be used in a constant expression}}
   }
   template void f3(const T ) {
 constexpr int k = T::size();
   }
   void g(array<3> a) {
 f1(a);
-f2(a); // expected-note {{instantiation of}}
+f2a(a);
 f3(a);
   }
 
@@ -2396,7 +2400,7 @@
   };
   void h(array_nonstatic<3> a) {
 f1(a);
-f2(a); // expected-note {{instantiation of}}
+f2b(a); // expected-note {{instantiation of}}
   }
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -8031,6 +8031,15 @@
 
   /// Potentially visit a MemberExpr's base expression.
   void VisitIgnoredBaseExpression(const Expr *E) {
+// If E is a reference, and if so, there is no side effect.
+if (const DeclRefExpr *DRE =
+dyn_cast(E->IgnoreParenImpCasts())) {
+  if (const VarDecl *VD = dyn_cast(DRE->getDecl())) {
+if (VD->getType()->isReferenceType()) {
+  return;
+}
+  }
+}
 // While MSVC doesn't evaluate the base expression, it does