[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG924acb624f58: [clang] Prevent folding of non-const compound 
expr (authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124038

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


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}} expected-note 1+{{not valid in a constant expression}} 
expected-note 1+{{declared here}}
+// expected-error@-1 1+{{must be initialized by a constant expression}}
+
 template int A::n = *(int[N]){0};
 
 template  void f() {
@@ -230,9 +232,9 @@
 };
 
 decltype(A<-3>::k) d1 = 0; // ok
-decltype(char{A<-4>::k}) d2 = 0; // expected-note {{instantiation of }} 
expected-error {{narrow}} expected-note {{cast}}
-decltype(char{A<1>::k}) d3 = 0; // ok
-decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}
+decltype(char{A<-4>::k}) d2 = 0;// expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1>::k}) d3 = 0; // expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}  expected-note {{instantiation of}}
   }
 }
 
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1596,8 +1596,13 @@
   // Matching GCC, file-scope array compound literals initialized by constants
   // are lifetime-extended.
   constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}}
-  static_assert(*p == 3, "");
+  static_assert(*p == 3, "");   // expected-error {{static_assert 
expression is not an integral constant expression}}
+// expected-note@-1 {{subexpression 
not valid}}
+// expected-note@-3 {{declared here}}
   static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}}
+  // expected-error@-1 {{static_assert expression is not an integral constant 
expression}}
+  // expected-note@-2 {{subexpression not valid}}
+  // expected-note@-3 {{declared here}}
 
   // Other kinds are not.
   struct X { int a[2]; };
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4259,9 +4259,33 @@
 Info.FFDiag(Conv);
 return false;
   }
+
   APValue Lit;
   if (!Evaluate(Lit, Info, CLE->getInitializer()))
 return false;
+
+  // According to GCC info page:
+  //
+  // 6.28 Compound Literals
+  //
+  // As an optimization, G++ sometimes gives array compound literals longer
+  // lifetimes: when the array either appears outside a function or has a
+  // const-qualified type. If foo and its initializer had elements of type
+  // char *const rather than char *, or if foo were a global variable, the
+  // array would have static storage duration. But it is probably safest
+  // just to avoid the use of array compound literals in C++ code.
+  //
+  // Obey that rule by checking constness for converted array types.
+
+  QualType CLETy = CLE->getType();
+  if (CLETy->isArrayType() && !Type->isArrayType()) {
+if (!CLETy.isConstant(Info.Ctx)) {
+  Info.FFDiag(Conv);
+  Info.Note(CLE->getExprLoc(), diag::note_declared_at);
+  return false;
+}
+  }
+
   CompleteObject LitObj(LVal.Base, &Lit, Base->getType());
   return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal, AK);
 } else if (isa(Base) || isa(Base)) {


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-e

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 429156.
serge-sans-paille added a comment.

Update messed up format


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

https://reviews.llvm.org/D124038

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


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}} expected-note 1+{{not valid in a constant expression}} 
expected-note 1+{{declared here}}
+// expected-error@-1 1+{{must be initialized by a constant expression}}
+
 template int A::n = *(int[N]){0};
 
 template  void f() {
@@ -230,9 +232,9 @@
 };
 
 decltype(A<-3>::k) d1 = 0; // ok
-decltype(char{A<-4>::k}) d2 = 0; // expected-note {{instantiation of }} 
expected-error {{narrow}} expected-note {{cast}}
-decltype(char{A<1>::k}) d3 = 0; // ok
-decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}
+decltype(char{A<-4>::k}) d2 = 0;// expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1>::k}) d3 = 0; // expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}  expected-note {{instantiation of}}
   }
 }
 
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1596,8 +1596,13 @@
   // Matching GCC, file-scope array compound literals initialized by constants
   // are lifetime-extended.
   constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}}
-  static_assert(*p == 3, "");
+  static_assert(*p == 3, "");   // expected-error {{static_assert 
expression is not an integral constant expression}}
+// expected-note@-1 {{subexpression 
not valid}}
+// expected-note@-3 {{declared here}}
   static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}}
+  // expected-error@-1 {{static_assert expression is not an integral constant 
expression}}
+  // expected-note@-2 {{subexpression not valid}}
+  // expected-note@-3 {{declared here}}
 
   // Other kinds are not.
   struct X { int a[2]; };
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4259,9 +4259,33 @@
 Info.FFDiag(Conv);
 return false;
   }
+
   APValue Lit;
   if (!Evaluate(Lit, Info, CLE->getInitializer()))
 return false;
+
+  // According to GCC info page:
+  //
+  // 6.28 Compound Literals
+  //
+  // As an optimization, G++ sometimes gives array compound literals longer
+  // lifetimes: when the array either appears outside a function or has a
+  // const-qualified type. If foo and its initializer had elements of type
+  // char *const rather than char *, or if foo were a global variable, the
+  // array would have static storage duration. But it is probably safest
+  // just to avoid the use of array compound literals in C++ code.
+  //
+  // Obey that rule by checking constness for converted array types.
+
+  QualType CLETy = CLE->getType();
+  if (CLETy->isArrayType() && !Type->isArrayType()) {
+if (!CLETy.isConstant(Info.Ctx)) {
+  Info.FFDiag(Conv);
+  Info.Note(CLE->getExprLoc(), diag::note_declared_at);
+  return false;
+}
+  }
+
   CompleteObject LitObj(LVal.Base, &Lit, Base->getType());
   return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal, AK);
 } else if (isa(Base) || isa(Base)) {


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}} expected-note 1+{{not valid in a constant expression}} expected-note 1+{{declared here}}
+

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Updated messed up formatting?

Otherwise, I guess this is fine.


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 428562.
serge-sans-paille added a comment.

Update GCC manual quote


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

https://reviews.llvm.org/D124038

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


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}} expected-note 1+{{not valid in a constant expression}} 
expected-note 1+{{declared here}}
+// expected-error@-1 1+{{must be initialized by a constant expression}}
+
 template int A::n = *(int[N]){0};
 
 template  void f() {
@@ -230,9 +232,9 @@
 };
 
 decltype(A<-3>::k) d1 = 0; // ok
-decltype(char{A<-4>::k}) d2 = 0; // expected-note {{instantiation of }} 
expected-error {{narrow}} expected-note {{cast}}
-decltype(char{A<1>::k}) d3 = 0; // ok
-decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}
+decltype(char{A<-4>::k}) d2 = 0;// expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1>::k}) d3 = 0; // expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}  expected-note {{instantiation of}}
   }
 }
 
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1596,8 +1596,13 @@
   // Matching GCC, file-scope array compound literals initialized by constants
   // are lifetime-extended.
   constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}}
-  static_assert(*p == 3, "");
+  static_assert(*p == 3, "");   // expected-error {{static_assert 
expression is not an integral constant expression}}
+// expected-note@-1 {{subexpression 
not valid}}
+// expected-note@-3 {{declared here}}
   static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}}
+  // expected-error@-1 {{static_assert expression is not an integral constant 
expression}}
+  // expected-note@-2 {{subexpression not valid}}
+  // expected-note@-3 {{declared here}}
 
   // Other kinds are not.
   struct X { int a[2]; };
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4259,9 +4259,28 @@
 Info.FFDiag(Conv);
 return false;
   }
+
   APValue Lit;
   if (!Evaluate(Lit, Info, CLE->getInitializer()))
 return false;
+
+  // According to GCC info page:
+  //
+  // 6.28 Compound Literals
+  //
+  // As an optimization, G++ sometimes gives array compound literals longer
+  // lifetimes: when the array either appears outside a function or has a
+  // const-qualified type. If foo and its initializer had elements of type
+  // char *const rather than char *, or if foo were a global variable, the
+  // array would have static storage duration. But it is probably safest
+  // just to avoid the use of array compound literals in C++ code.
+  //
+  // Obey that rule by checking constness for converted array types.
+  QualType CLETy = CLE->getType(); if (CLETy->isArrayType() &&
+  !Type->isArrayType()) { if (!CLETy.isConstant(Info.Ctx)) {
+Info.FFDiag(Conv); Info.Note(CLE->getExprLoc(), 
diag::note_declared_at);
+return false; } }
+
   CompleteObject LitObj(LVal.Base, &Lit, Base->getType());
   return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal, AK);
 } else if (isa(Base) || isa(Base)) {


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}} expected-note 1+{{not valid in a constant expression}} expected-note 1+{{declared here}}
+// expected-error@-1 1+{{must be init

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I think so, yes.


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D124038#3504371 , @efriedma wrote:

> I think you're looking at old documentation?  Here's what the current page 
> (https://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html) has to say:

Indeed! I was looking at my local Info Page. thanks for the extra pointer.

>> As an optimization, G++ sometimes gives array compound literals longer 
>> lifetimes: when the array either appears outside a function or has a 
>> const-qualified type. If foo and its initializer had elements of type char 
>> *const rather than char *, or if foo were a global variable, the array would 
>> have static storage duration. But it is probably safest just to avoid the 
>> use of array compound literals in C++ code.

I can quote that part instead. I don't think this invalidates the actual code, 
right? At least with that commit the observable behavior gets closer to GCC, 
and it fixes https://github.com/llvm/llvm-project/issues/39324


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I think you're looking at old documentation?  Here's what the current page 
(https://gcc.gnu.org/onlinedocs/gcc/Compound-Literals.html) has to say:

  In C, a compound literal designates an unnamed object with static or 
automatic storage duration. In C++, a compound literal designates a temporary 
object that only lives until the end of its full-expression. As a result, 
well-defined C code that takes the address of a subobject of a compound literal 
can be undefined in C++, so G++ rejects the conversion of a temporary array to 
a pointer. For instance, if the array compound literal example above appeared 
inside a function, any subsequent use of foo in C++ would have undefined 
behavior because the lifetime of the array ends after the declaration of foo.
  
  As an optimization, G++ sometimes gives array compound literals longer 
lifetimes: when the array either appears outside a function or has a 
const-qualified type. If foo and its initializer had elements of type char 
*const rather than char *, or if foo were a global variable, the array would 
have static storage duration. But it is probably safest just to avoid the use 
of array compound literals in C++ code.


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:1609
   struct X { int a[2]; };
   constexpr int *n = (X){1, 2}.a; // expected-warning {{C99}} expected-warning 
{{temporary}}
   // expected-error@-1 {{constant expression}}

@efriedma this is the test you asked for. it's already implemented, and my 
patch doesn't change its behavior, which matches the GCC behavior.


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 428317.
serge-sans-paille edited the summary of this revision.
serge-sans-paille added a comment.

Added reg to GCC info page to explain current behavior, and make the test more 
explicit with respect to that quote.


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

https://reviews.llvm.org/D124038

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


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}} expected-note 1+{{not valid in a constant expression}} 
expected-note 1+{{declared here}}
+// expected-error@-1 1+{{must be initialized by a constant expression}}
+
 template int A::n = *(int[N]){0};
 
 template  void f() {
@@ -230,9 +232,9 @@
 };
 
 decltype(A<-3>::k) d1 = 0; // ok
-decltype(char{A<-4>::k}) d2 = 0; // expected-note {{instantiation of }} 
expected-error {{narrow}} expected-note {{cast}}
-decltype(char{A<1>::k}) d3 = 0; // ok
-decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}
+decltype(char{A<-4>::k}) d2 = 0;// expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1>::k}) d3 = 0; // expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}  expected-note {{instantiation of}}
   }
 }
 
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1596,8 +1596,13 @@
   // Matching GCC, file-scope array compound literals initialized by constants
   // are lifetime-extended.
   constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}}
-  static_assert(*p == 3, "");
+  static_assert(*p == 3, "");   // expected-error {{static_assert 
expression is not an integral constant expression}}
+// expected-note@-1 {{subexpression 
not valid}}
+// expected-note@-3 {{declared here}}
   static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}}
+  // expected-error@-1 {{static_assert expression is not an integral constant 
expression}}
+  // expected-note@-2 {{subexpression not valid}}
+  // expected-note@-3 {{declared here}}
 
   // Other kinds are not.
   struct X { int a[2]; };
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4259,9 +4259,29 @@
 Info.FFDiag(Conv);
 return false;
   }
+
   APValue Lit;
   if (!Evaluate(Lit, Info, CLE->getInitializer()))
 return false;
+
+  // According to GCC info page, in C++
+  //
+  // If all the elements of the compound literal are (made
+  // up of) simple constant expressions suitable for use in initializers of
+  // objects of static storage duration, then the compound literal can be
+  // coerced to a pointer to its first element and used in such an
+  // initializer.
+  //
+  // Obey that rule by checking constness for converted array types.
+  QualType CLETy = CLE->getType();
+  if(CLETy->isArrayType() && !Type->isArrayType()) {
+if (!CLETy.isConstant(Info.Ctx)) {
+  Info.FFDiag(Conv);
+  Info.Note(CLE->getExprLoc(), diag::note_declared_at);
+  return false;
+}
+  }
+
   CompleteObject LitObj(LVal.Base, &Lit, Base->getType());
   return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal, AK);
 } else if (isa(Base) || isa(Base)) {


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}} expected-note 1+{{not valid in a constant expression}} expected-note 1+{{declared here}}
+// expected-error@-1 1+{{must be initializ

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4267
+  bool IsConstant = CLETy.isConstant(Info.Ctx);
+  if (!IsConstant && CLETy->isArrayType()) {
+Info.FFDiag(Conv);

serge-sans-paille wrote:
> efriedma wrote:
> > Is the "isArrayType()" check here necessary?
> Yeah, otherwise we have an issue with
> 
> ```
> typedef __attribute__(( ext_vector_type(4) )) float float4;
> float4 foo = (float4){ 1.0, 2.0, 3.0, 4.0 };
> ```
> 
> error: cannot compile this static initializer yet
Hmm, I see... in C++, the compound literal rules are weird.  C compound 
literals are lvalues, but C++ ones are not.  So normally, we don't allow taking 
the address of a compound literal or any of its members.  But if it's an array, 
somehow different rules (what rules?) apply.

If you can add a comment explaining what's going on here, maybe it's okay?

I'm tempted to say we should reject the testcase, though.  For example, in the 
following, it doesn't really make sense to reject the first variable, but 
accept the second.

```
typedef int arr[2];
constexpr int *x = arr{1};
constexpr int *y = (arr){1};
```

--

Can you add a testcase involving an array of structs with a "mutable" member?


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4267
+  bool IsConstant = CLETy.isConstant(Info.Ctx);
+  if (!IsConstant && CLETy->isArrayType()) {
+Info.FFDiag(Conv);

efriedma wrote:
> Is the "isArrayType()" check here necessary?
Yeah, otherwise we have an issue with

```
typedef __attribute__(( ext_vector_type(4) )) float float4;
float4 foo = (float4){ 1.0, 2.0, 3.0, 4.0 };
```

error: cannot compile this static initializer yet


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:4267
+  bool IsConstant = CLETy.isConstant(Info.Ctx);
+  if (!IsConstant && CLETy->isArrayType()) {
+Info.FFDiag(Conv);

Is the "isArrayType()" check here necessary?


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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 428042.
serge-sans-paille added a comment.

Match GCC behavior here: some test case were previously accepted while having 
the opposite behavior. This pacth both fixes the original issue and adopt gcc 
behavior.


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

https://reviews.llvm.org/D124038

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


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 
1+{{negative}} expected-note 1+{{not valid in a constant expression}} 
expected-note 1+{{declared here}}
+// expected-error@-1 1+{{must be initialized by a constant expression}}
+
 template int A::n = *(int[N]){0};
 
 template  void f() {
@@ -230,9 +232,9 @@
 };
 
 decltype(A<-3>::k) d1 = 0; // ok
-decltype(char{A<-4>::k}) d2 = 0; // expected-note {{instantiation of }} 
expected-error {{narrow}} expected-note {{cast}}
-decltype(char{A<1>::k}) d3 = 0; // ok
-decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}
+decltype(char{A<-4>::k}) d2 = 0;// expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1>::k}) d3 = 0; // expected-note 
1+{{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
+decltype(char{A<1 + (unsigned char)-1>::k}) d4 = 0; // expected-error 
{{narrow}} expected-note {{cast}}  expected-note {{instantiation of}}
   }
 }
 
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1596,8 +1596,13 @@
   // Matching GCC, file-scope array compound literals initialized by constants
   // are lifetime-extended.
   constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}}
-  static_assert(*p == 3, "");
+  static_assert(*p == 3, "");   // expected-error {{static_assert 
expression is not an integral constant expression}}
+// expected-note@-1 {{subexpression 
not valid}}
+// expected-note@-3 {{declared here}}
   static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}}
+  // expected-error@-1 {{static_assert expression is not an integral constant 
expression}}
+  // expected-note@-2 {{subexpression not valid}}
+  // expected-note@-3 {{declared here}}
 
   // Other kinds are not.
   struct X { int a[2]; };
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4259,9 +4259,21 @@
 Info.FFDiag(Conv);
 return false;
   }
+
+  // Dereferencing a pointer to an array of CompoundLiteralExpr requires 
the
+  // latter to be const.
+  QualType CLETy = CLE->getType();
+  bool IsConstant = CLETy.isConstant(Info.Ctx);
+  if (!IsConstant && CLETy->isArrayType()) {
+Info.FFDiag(Conv);
+Info.Note(CLE->getExprLoc(), diag::note_declared_at);
+return false;
+  }
+
   APValue Lit;
   if (!Evaluate(Lit, Info, CLE->getInitializer()))
 return false;
+
   CompleteObject LitObj(LVal.Base, &Lit, Base->getType());
   return extractSubobject(Info, Conv, LitObj, LVal.Designator, RVal, AK);
 } else if (isa(Base) || isa(Base)) {


Index: clang/test/SemaTemplate/constexpr-instantiate.cpp
===
--- clang/test/SemaTemplate/constexpr-instantiate.cpp
+++ clang/test/SemaTemplate/constexpr-instantiate.cpp
@@ -219,7 +219,9 @@
   static int n;
 };
 template struct B {};
-template constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}}
+template  constexpr int A::k = *(int[N]){N}; // expected-error 1+{{negative}} expected-note 1+{{not valid in a constant expression}} expected-note 1+{{declared here}}
+// expected-error@-1 1+{{must be initialized by a constant expression}}
+
 template int A::n = *(int[N]){0};
 
 template  void f() {
@@ -230,9 +232,9 @@
 };
 
 decltype(A<-3>::k) d1 = 0; // ok
-decltype(char{A<-4>::k}) d2 = 0; // expected-note {{instantiation of }} expected-error {{narrow}} expected-note {{cast}}
-decltype(char{A<1>::k}) d3 = 0; // ok
-decltype(char{A<1 + (uns

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The `VD->getAnyInitializer()->IgnoreCasts()` is the part that's wrong.  We want 
to allow something like the following:

  constexpr int *q = (int *)(int[1]){3};
  constexpr int *q2 = q;

To catch the bad cases specifically, we have to wait until we actually reach 
the compound literal.  See line 4262, or something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Alternatively, the following also works, but it splits the logic into 
anotherplace, while current patch is at least consistent with existing state

  diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
  index 498f0d4..233307f 100644
  --- a/clang/lib/AST/ExprConstant.cpp
  +++ b/clang/lib/AST/ExprConstant.cpp
  @@ -3352,6 +3352,17 @@ static bool evaluateVarDeclInit(EvalInfo &Info, const 
Expr *E,
   NoteLValueLocation(Info, Base);
 }
   
  +  // In the particular case of CompoundLiteralExpr initialization, check 
that it is itself
  +  // constant.
  +  if (Info.InConstantContext)
  +if (const CompoundLiteralExpr *CLE = 
dyn_cast_or_null(
  +VD->getAnyInitializer()->IgnoreCasts())) {
  +  QualType CLET = CLE->getType().getCanonicalType();
  +  if (!CLET.isConstant(Info.Ctx)) {
  +Info.FFDiag(E);
  +  }
  +}
  +


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D124038#3460731 , @efriedma wrote:

> The fix doesn't look right.
>
> A CompoundLiteralExpr is itself an lvalue; we should catch any issues when we 
> visit it, not a VarDecl that contains a pointer to it.

Well, the visitor for CompoundLiteralExpr explicitly states

>   // Defer visiting the literal until the lvalue-to-rvalue conversion. We can
>   // only see this when folding in C, so there's no standard to follow here.

Which is what I was trying to do in this patch.
We only visit CompoundLiteralExpr once, during initialization of the VarDecl, 
and that initialization is correct. We don't visit it again during checking of 
expression constness because we stop at the VarDecl level. I'm checking if we 
could peform an extra check at that point. But maybe I'm totally taking a wrong 
turn, feel free to advise :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The fix doesn't look right.

A CompoundLiteralExpr is itself an lvalue; we should catch any issues when we 
visit it, not a VarDecl that contains a pointer to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124038

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

[PATCH] D124038: [clang] Prevent folding of non-const compound expr

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: aaron.ballman.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When a non-const compound statement is used to initialize a constexpr pointer,
the pointed value is not const itself and cannot be folded at codegen time.

Fix issue #39324.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124038

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
@@ -1595,10 +1595,13 @@
 namespace CompoundLiteral {
   // Matching GCC, file-scope array compound literals initialized by constants
   // are lifetime-extended.
-  constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}}
+  constexpr int *p = (int*)(const int[1]){3}; // expected-warning {{C99}}
   static_assert(*p == 3, "");
   static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}}
 
+  constexpr int *q = (int *)(int[1]){3}; // expected-warning {{C99}}
+  static_assert(*q == 3, "");// expected-error {{constant 
expression}}
+
   // Other kinds are not.
   struct X { int a[2]; };
   constexpr int *n = (X){1, 2}.a; // expected-warning {{C99}} expected-warning 
{{temporary}}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4289,6 +4289,17 @@
 }
   }
 
+  if (const VarDecl *VD = dyn_cast_or_null(
+  LVal.Base.dyn_cast())) {
+if (const CompoundLiteralExpr *CLE = dyn_cast_or_null(
+VD->getAnyInitializer()->IgnoreCasts())) {
+  QualType CLET = CLE->getType().getCanonicalType();
+  if (!CLET.isConstant(Info.Ctx)) {
+Info.FFDiag(Conv);
+  }
+}
+  }
+
   CompleteObject Obj = findCompleteObject(Info, Conv, AK, LVal, Type);
   return Obj && extractSubobject(Info, Conv, Obj, LVal.Designator, RVal, AK);
 }


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1595,10 +1595,13 @@
 namespace CompoundLiteral {
   // Matching GCC, file-scope array compound literals initialized by constants
   // are lifetime-extended.
-  constexpr int *p = (int*)(int[1]){3}; // expected-warning {{C99}}
+  constexpr int *p = (int*)(const int[1]){3}; // expected-warning {{C99}}
   static_assert(*p == 3, "");
   static_assert((int[2]){1, 2}[1] == 2, ""); // expected-warning {{C99}}
 
+  constexpr int *q = (int *)(int[1]){3}; // expected-warning {{C99}}
+  static_assert(*q == 3, "");// expected-error {{constant expression}}
+
   // Other kinds are not.
   struct X { int a[2]; };
   constexpr int *n = (X){1, 2}.a; // expected-warning {{C99}} expected-warning {{temporary}}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -4289,6 +4289,17 @@
 }
   }
 
+  if (const VarDecl *VD = dyn_cast_or_null(
+  LVal.Base.dyn_cast())) {
+if (const CompoundLiteralExpr *CLE = dyn_cast_or_null(
+VD->getAnyInitializer()->IgnoreCasts())) {
+  QualType CLET = CLE->getType().getCanonicalType();
+  if (!CLET.isConstant(Info.Ctx)) {
+Info.FFDiag(Conv);
+  }
+}
+  }
+
   CompleteObject Obj = findCompleteObject(Info, Conv, AK, LVal, Type);
   return Obj && extractSubobject(Info, Conv, Obj, LVal.Designator, RVal, AK);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits