[PATCH] D32824: [libc++] Use placeholder return types to avoid hard errors during overload resolution

2017-05-03 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth added a comment.

In https://reviews.llvm.org/D32824#745392, @EricWF wrote:

> This change regresses other code. By changing `__bind_return<...>::type` into 
> `decltype(auto)` you prevent the member functions from SFINAE'ing. For 
> example:
>
>   #include 
>  
>   struct Func {
> template 
> void operator()(Args&&...) = delete;
>  
> template 
> void operator()(Args&&...) const {}
>   };
>  
>   int main() {
>   Func f;
>   std::bind(f)(); // Broken after your change.
>   }
>
>
> This patch cannot introduce regressions but changing to using 
> `decltype(auto)` does. I'm not sure the behavior you want is possible. The 
> library is required to evaluate the validity of calling the specified functor 
> in the signature of the call operator in order to correctly SFINAE.
>  I think this makes accepting the code in PR32856 impossible.


This is **intended **to fail (as I also mentioned in the motivating SO post 
). The type of the object argument 
`fd` is `FD`, where `FD `is `decay_t`, i.e. `Func`. That's it. And if we 
use a non-`const ` object argument, overload resolution prefers non-`const` 
member functions per se, hence we should get a diagnostic saying that a deleted 
function is called. Alternatively, some other construct inside `bind` should 
fail: libstdc++'s `bind` somehow requires `result_of_t`, which fails.

In short, this was never intended to work; the fact that it does is a 
consequence of the return type being denoted in an expression SFINAE manner.


https://reviews.llvm.org/D32824



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


[PATCH] D32824: [libc++] Use placeholder return types to avoid hard errors during overload resolution

2017-05-03 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth created this revision.
Herald added a reviewer: EricWF.

See https://bugs.llvm.org/show_bug.cgi?id=32856. Basically, as this code was 
written before placeholder return types for functions were a
thing, the function call operators of __bind apply decltype on the call 
expression to infer the return type. This can lead to hard errors when we pass 
template functors. Another consequence of this is that the validity or 
invalidity of these call expressions influences overload resolution (via 
SFINAE), hence the const version could be selected where the __bind object was 
non-const, which is not the prescribed behaviour. This is fixed by 
using placeholder return types.


https://reviews.llvm.org/D32824

Files:
  include/functional


Index: include/functional
===
--- include/functional
+++ include/functional
@@ -1006,7 +1006,7 @@
 {
 _Predicate __pred_;
 public:
-_LIBCPP_INLINE_VISIBILITY explicit _LIBCPP_CONSTEXPR_AFTER_CXX11 
+_LIBCPP_INLINE_VISIBILITY explicit _LIBCPP_CONSTEXPR_AFTER_CXX11
 binary_negate(const _Predicate& __pred) : __pred_(__pred) {}
 
 _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
@@ -1793,7 +1793,7 @@
 typedef __function::__func<_Fp, _Alloc, _Rp(_ArgTypes...)> _FF;
 typedef typename __rebind_alloc_helper<__alloc_traits, _FF>::type _Ap;
 _Ap __a(__a0);
-if (sizeof(_FF) <= sizeof(__buf_) && 
+if (sizeof(_FF) <= sizeof(__buf_) &&
 is_nothrow_copy_constructible<_Fp>::value && 
is_nothrow_copy_constructible<_Ap>::value)
 {
 __f_ = ::new((void*)&__buf_) _FF(_VSTD::move(__f), _Alloc(__a));
@@ -2254,16 +2254,24 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
+#ifndef __cpp_decltype_auto
 typename __bind_return<_Fd, _Td, tuple<_Args&&...> >::type
+#else
+decltype(auto)
+#endif
 operator()(_Args&& ...__args)
 {
 return __apply_functor(__f_, __bound_args_, __indices(),
   
tuple<_Args&&...>(_VSTD::forward<_Args>(__args)...));
 }
 
 template 
 _LIBCPP_INLINE_VISIBILITY
+#ifndef __cpp_decltype_auto
 typename __bind_return >::type
+#else
+decltype(auto)
+#endif
 operator()(_Args&& ...__args) const
 {
 return __apply_functor(__f_, __bound_args_, __indices(),


Index: include/functional
===
--- include/functional
+++ include/functional
@@ -1006,7 +1006,7 @@
 {
 _Predicate __pred_;
 public:
-_LIBCPP_INLINE_VISIBILITY explicit _LIBCPP_CONSTEXPR_AFTER_CXX11 
+_LIBCPP_INLINE_VISIBILITY explicit _LIBCPP_CONSTEXPR_AFTER_CXX11
 binary_negate(const _Predicate& __pred) : __pred_(__pred) {}
 
 _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
@@ -1793,7 +1793,7 @@
 typedef __function::__func<_Fp, _Alloc, _Rp(_ArgTypes...)> _FF;
 typedef typename __rebind_alloc_helper<__alloc_traits, _FF>::type _Ap;
 _Ap __a(__a0);
-if (sizeof(_FF) <= sizeof(__buf_) && 
+if (sizeof(_FF) <= sizeof(__buf_) &&
 is_nothrow_copy_constructible<_Fp>::value && is_nothrow_copy_constructible<_Ap>::value)
 {
 __f_ = ::new((void*)&__buf_) _FF(_VSTD::move(__f), _Alloc(__a));
@@ -2254,16 +2254,24 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
+#ifndef __cpp_decltype_auto
 typename __bind_return<_Fd, _Td, tuple<_Args&&...> >::type
+#else
+decltype(auto)
+#endif
 operator()(_Args&& ...__args)
 {
 return __apply_functor(__f_, __bound_args_, __indices(),
   tuple<_Args&&...>(_VSTD::forward<_Args>(__args)...));
 }
 
 template 
 _LIBCPP_INLINE_VISIBILITY
+#ifndef __cpp_decltype_auto
 typename __bind_return >::type
+#else
+decltype(auto)
+#endif
 operator()(_Args&& ...__args) const
 {
 return __apply_functor(__f_, __bound_args_, __indices(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D32372: Arrays of unknown bound in constant expressions

2017-04-28 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth updated this revision to Diff 97180.
Arcoth added a comment.

Applied the last review's suggestions.


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-array-unknown-bound.cpp

Index: test/SemaCXX/constexpr-array-unknown-bound.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-array-unknown-bound.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return p[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re@-1 {{in call to 'f({{.*}})'}}
+
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // ok
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{constant bound}}
+
+void g(int i) {
+  int arr[i];
+  constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{constant bound}}
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -148,37 +148,44 @@
   static unsigned
   findMostDerivedSubobject(ASTContext &Ctx, APValue::LValueBase Base,
ArrayRef Path,
-   uint64_t &ArraySize, QualType &Type, bool &IsArray) {
+   uint64_t &ArraySize, QualType &Type, bool &IsArray,
+   bool &IsUnsizedArray) {
 // This only accepts LValueBases from APValues, and APValues don't support
 // arrays that lack size info.
 assert(!isBaseAnAllocSizeCall(Base) &&
"Unsized arrays shouldn't appear here");
 unsigned MostDerivedLength = 0;
 Type = getType(Base);
 
 for (unsigned I = 0, N = Path.size(); I != N; ++I) {
-  if (Type->isArrayType()) {
-const ConstantArrayType *CAT =
-cast(Ctx.getAsArrayType(Type));
-Type = CAT->getElementType();
-ArraySize = CAT->getSize().getZExtValue();
+  if (auto AT = Ctx.getAsArrayType(Type)) {
 MostDerivedLength = I + 1;
 IsArray = true;
+if (auto CAT = Ctx.getAsConstantArrayType(Type))
+  ArraySize = CAT->getSize().getZExtValue();
+else {
+  ArraySize = 0;
+  IsUnsizedArray = true;
+}
+Type = AT->getElementType();
   } else if (Type->isAnyComplexType()) {
 const ComplexType *CT = Type->castAs();
 Type = CT->getElementType();
 ArraySize = 2;
 MostDerivedLength = I + 1;
 IsArray = true;
+IsUnsizedArray = false;
   } else if (const FieldDecl *FD = getAsField(Path[I])) {
 Type = FD->getType();
 ArraySize = 0;
 MostDerivedLength = I + 1;
 IsArray = false;
+IsUnsizedArray = false;
   } else {
 // Path[I] describes a base class.
 ArraySize = 0;
 IsArray = false;
+IsUnsizedArray = false;
   }
 }
 return MostDerivedLength;
@@ -200,8 +207,9 @@
 /// Is this a pointer one past the end of an object?
 unsigned IsOnePastTheEnd : 1;
 
-/// Indicator of whether the first entry is an unsized array.
-unsigned FirstEntryIsAnUnsizedArray : 1;
+/// Indicator of whether the most-derived object is an unsized array (e.g.
+/// of unknown bound).
+unsigned MostDerivedIsAnUnsizedArray : 1;
 
 /// Indicator of whether the most-derived object is an array element.
 unsigned MostDerivedIsArrayElement : 1;
@@ -231,25 +239,28 @@
 
 explicit SubobjectDesignator(QualType T)
 : Invalid(false), IsOnePastTheEnd(false),
-  FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
+  MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
   MostDerivedPathLength(0), MostDerivedArraySize(0),
   MostDerivedType(T) {}
 
 SubobjectDesignator(ASTContext &Ctx, const APValue &V)
 : Invalid(!V.isLValue() || !V.hasLValuePath()), IsOnePastTheEnd(false),
-  FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
+  MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
   MostDerivedPathLength(0), MostDerivedArraySize(0) {
   assert(V.isLValue() && "Non-LValue used to make an LValue designator?");
   if (!Invalid) {
 IsOnePastTheEnd = V.isLValueOnePastTheEnd();
 ArrayRef VEntries = V.getLValuePath();
 Entries.insert(Entries.end(), VEntries.begin(), VEntries.end());
-if (V.getLValueBase()) {
-  bool IsArray = false;
+if (auto Base = V.getLValueBase()) {
+  if (auto Decl = Base.dyn_cast())

[PATCH] D32372: Arrays of unknown bound in constant expressions

2017-04-28 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth updated this revision to Diff 97116.
Arcoth added a comment.

(Just fixed a slip after typing with the wrong window in focus...)


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-array-unknown-bound.cpp

Index: test/SemaCXX/constexpr-array-unknown-bound.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-array-unknown-bound.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return p[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re@-1 {{in call to 'f({{.*}})'}}
+
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // ok
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{constant bound}}
+
+void g(int i) {
+  int arr[i];
+  constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{constant bound}}
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -148,22 +148,26 @@
   static unsigned
   findMostDerivedSubobject(ASTContext &Ctx, APValue::LValueBase Base,
ArrayRef Path,
-   uint64_t &ArraySize, QualType &Type, bool &IsArray) {
+   uint64_t &ArraySize, QualType &Type, bool &IsArray,
+   bool &isUnsizedArray) {
 // This only accepts LValueBases from APValues, and APValues don't support
 // arrays that lack size info.
 assert(!isBaseAnAllocSizeCall(Base) &&
"Unsized arrays shouldn't appear here");
 unsigned MostDerivedLength = 0;
 Type = getType(Base);
 
 for (unsigned I = 0, N = Path.size(); I != N; ++I) {
-  if (Type->isArrayType()) {
-const ConstantArrayType *CAT =
-cast(Ctx.getAsArrayType(Type));
-Type = CAT->getElementType();
-ArraySize = CAT->getSize().getZExtValue();
+  if (auto AT = Ctx.getAsArrayType(Type)) {
 MostDerivedLength = I + 1;
 IsArray = true;
+if (auto CAT = Ctx.getAsConstantArrayType(Type))
+  ArraySize = CAT->getSize().getZExtValue();
+else {
+  ArraySize = 0;
+  isUnsizedArray = true;
+}
+Type = AT->getElementType();
   } else if (Type->isAnyComplexType()) {
 const ComplexType *CT = Type->castAs();
 Type = CT->getElementType();
@@ -200,8 +204,9 @@
 /// Is this a pointer one past the end of an object?
 unsigned IsOnePastTheEnd : 1;
 
-/// Indicator of whether the first entry is an unsized array.
-unsigned FirstEntryIsAnUnsizedArray : 1;
+/// Indicator of whether the most-derived object is an unsized array (e.g.
+/// of unknown bound).
+unsigned MostDerivedIsAnUnsizedArray : 1;
 
 /// Indicator of whether the most-derived object is an array element.
 unsigned MostDerivedIsArrayElement : 1;
@@ -231,25 +236,28 @@
 
 explicit SubobjectDesignator(QualType T)
 : Invalid(false), IsOnePastTheEnd(false),
-  FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
+  MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
   MostDerivedPathLength(0), MostDerivedArraySize(0),
   MostDerivedType(T) {}
 
 SubobjectDesignator(ASTContext &Ctx, const APValue &V)
 : Invalid(!V.isLValue() || !V.hasLValuePath()), IsOnePastTheEnd(false),
-  FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
+  MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
   MostDerivedPathLength(0), MostDerivedArraySize(0) {
   assert(V.isLValue() && "Non-LValue used to make an LValue designator?");
   if (!Invalid) {
 IsOnePastTheEnd = V.isLValueOnePastTheEnd();
 ArrayRef VEntries = V.getLValuePath();
 Entries.insert(Entries.end(), VEntries.begin(), VEntries.end());
-if (V.getLValueBase()) {
-  bool IsArray = false;
+if (auto Base = V.getLValueBase()) {
+  if (auto Decl = Base.dyn_cast())
+Base = cast(Decl->getMostRecentDecl());
+  bool IsArray = false, IsUnsizedArray = false;
   MostDerivedPathLength = findMostDerivedSubobject(
-  Ctx, V.getLValueBase(), V.getLValuePath(), MostDerivedArraySize,
-  MostDerivedType, IsArray);
-  MostDerivedIsArrayElement = IsArray;
+  Ctx, Base, V.getLValuePath(), MostDerivedArraySize,
+  MostDerivedType, IsArray, IsUnsizedArray);
+  MostDerivedI

[PATCH] D32372: Arrays of unknown bound in constant expressions

2017-04-28 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth updated this revision to Diff 97114.
Arcoth added a comment.

Simplified array-to-pointer conversion (simply add the array as unsized; let 
findCompleteObject and the SubobjectDesignator ctor do the work).


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-array-unknown-bound.cpp

Index: test/SemaCXX/constexpr-array-unknown-bound.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-array-unknown-bound.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return p[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re@-1 {{in call to 'f({{.*}})'}}
+
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // ok
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{constant bound}}
+
+void g(int i) {
+  int arr[i];
+  constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{constant bound}}
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -148,22 +148,26 @@
   static unsigned
   findMostDerivedSubobject(ASTContext &Ctx, APValue::LValueBase Base,
ArrayRef Path,
-   uint64_t &ArraySize, QualType &Type, bool &IsArray) {
+   uint64_t &ArraySize, QualType &Type, bool &IsArray,
+   bool &isUnsizedArray) {
 // This only accepts LValueBases from APValues, and APValues don't support
 // arrays that lack size info.
 assert(!isBaseAnAllocSizeCall(Base) &&
"Unsized arrays shouldn't appear here");
 unsigned MostDerivedLength = 0;
 Type = getType(Base);
 
 for (unsigned I = 0, N = Path.size(); I != N; ++I) {
-  if (Type->isArrayType()) {
-const ConstantArrayType *CAT =
-cast(Ctx.getAsArrayType(Type));
-Type = CAT->getElementType();
-ArraySize = CAT->getSize().getZExtValue();
+  if (auto AT = Ctx.getAsArrayType(Type)) {
 MostDerivedLength = I + 1;
 IsArray = true;
+if (auto CAT = Ctx.getAsConstantArrayType(Type))
+  ArraySize = CAT->getSize().getZExtValue();
+else {
+  ArraySize = 0;
+  isUnsizedArray = true;
+}
+Type = AT->getElementType();
   } else if (Type->isAnyComplexType()) {
 const ComplexType *CT = Type->castAs();
 Type = CT->getElementType();
@@ -200,8 +204,9 @@
 /// Is this a pointer one past the end of an object?
 unsigned IsOnePastTheEnd : 1;
 
-/// Indicator of whether the first entry is an unsized array.
-unsigned FirstEntryIsAnUnsizedArray : 1;
+/// Indicator of whether the most-derived object is an unsized array (e.g.
+/// of unknown bound).
+unsigned MostDerivedIsAnUnsizedArray : 1;
 
 /// Indicator of whether the most-derived object is an array element.
 unsigned MostDerivedIsArrayElement : 1;
@@ -231,25 +236,28 @@
 
 explicit SubobjectDesignator(QualType T)
 : Invalid(false), IsOnePastTheEnd(false),
-  FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
+  MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
   MostDerivedPathLength(0), MostDerivedArraySize(0),
   MostDerivedType(T) {}
 
 SubobjectDesignator(ASTContext &Ctx, const APValue &V)
 : Invalid(!V.isLValue() || !V.hasLValuePath()), IsOnePastTheEnd(false),
-  FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
+  MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
   MostDerivedPathLength(0), MostDerivedArraySize(0) {
   assert(V.isLValue() && "Non-LValue used to make an LValue designator?");
   if (!Invalid) {
 IsOnePastTheEnd = V.isLValueOnePastTheEnd();
 ArrayRef VEntries = V.getLValuePath();
 Entries.insert(Entries.end(), VEntries.begin(), VEntries.end());
-if (V.getLValueBase()) {
-  bool IsArray = false;
+if (auto Base = V.getLValueBase()) {
+  if (auto Decl = Base.dyn_cast())
+Base = cast(Decl->getMostRecentDecl());
+  bool IsArray = false, IsUnsizedArray = false;
   MostDerivedPathLength = findMostDerivedSubobject(
-  Ctx, V.getLValueBase(), V.getLValuePath(), MostDerivedArraySize,
-  MostDerivedType, IsArray);
-  MostDerivedIsArrayElement = IsArray;
+  Ctx, Base, V.getLValuePath(), MostDerivedArraySize,
+   

[PATCH] D32372: Arrays of unknown bound in constant expressions

2017-04-28 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth updated this revision to Diff 97105.
Arcoth added a comment.

Updated `SubobjectDesignator` to hold `MostDerivedIsAnUnsizedArray` instead of 
`FirstEntryIsAnUnsizedArray`. Consequently, we cannot know whether the first 
element is an unsized array; please see line 7352 in particular, where the 
check for `isMostDerivedAnUnsizedArray`had to be removed (is this fine? All 
regression tests pass.)


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-array-unknown-bound.cpp

Index: test/SemaCXX/constexpr-array-unknown-bound.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-array-unknown-bound.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return p[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re@-1 {{in call to 'f({{.*}})'}}
+
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // ok
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{constant bound}}
+
+void g(int i) {
+  int arr[i];
+  constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{constant bound}}
+}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -148,22 +148,26 @@
   static unsigned
   findMostDerivedSubobject(ASTContext &Ctx, APValue::LValueBase Base,
ArrayRef Path,
-   uint64_t &ArraySize, QualType &Type, bool &IsArray) {
+   uint64_t &ArraySize, QualType &Type, bool &IsArray,
+   bool &isUnsizedArray) {
 // This only accepts LValueBases from APValues, and APValues don't support
 // arrays that lack size info.
 assert(!isBaseAnAllocSizeCall(Base) &&
"Unsized arrays shouldn't appear here");
 unsigned MostDerivedLength = 0;
 Type = getType(Base);
 
 for (unsigned I = 0, N = Path.size(); I != N; ++I) {
-  if (Type->isArrayType()) {
-const ConstantArrayType *CAT =
-cast(Ctx.getAsArrayType(Type));
-Type = CAT->getElementType();
-ArraySize = CAT->getSize().getZExtValue();
+  if (auto AT = Ctx.getAsArrayType(Type)) {
 MostDerivedLength = I + 1;
 IsArray = true;
+if (auto CAT = Ctx.getAsConstantArrayType(Type))
+  ArraySize = CAT->getSize().getZExtValue();
+else {
+  ArraySize = 0;
+  isUnsizedArray = true;
+}
+Type = AT->getElementType();
   } else if (Type->isAnyComplexType()) {
 const ComplexType *CT = Type->castAs();
 Type = CT->getElementType();
@@ -200,8 +204,9 @@
 /// Is this a pointer one past the end of an object?
 unsigned IsOnePastTheEnd : 1;
 
-/// Indicator of whether the first entry is an unsized array.
-unsigned FirstEntryIsAnUnsizedArray : 1;
+/// Indicator of whether the most-derived object is an unsized array (e.g.
+/// of unknown bound).
+unsigned MostDerivedIsAnUnsizedArray : 1;
 
 /// Indicator of whether the most-derived object is an array element.
 unsigned MostDerivedIsArrayElement : 1;
@@ -231,25 +236,28 @@
 
 explicit SubobjectDesignator(QualType T)
 : Invalid(false), IsOnePastTheEnd(false),
-  FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
+  MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
   MostDerivedPathLength(0), MostDerivedArraySize(0),
   MostDerivedType(T) {}
 
 SubobjectDesignator(ASTContext &Ctx, const APValue &V)
 : Invalid(!V.isLValue() || !V.hasLValuePath()), IsOnePastTheEnd(false),
-  FirstEntryIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
+  MostDerivedIsAnUnsizedArray(false), MostDerivedIsArrayElement(false),
   MostDerivedPathLength(0), MostDerivedArraySize(0) {
   assert(V.isLValue() && "Non-LValue used to make an LValue designator?");
   if (!Invalid) {
 IsOnePastTheEnd = V.isLValueOnePastTheEnd();
 ArrayRef VEntries = V.getLValuePath();
 Entries.insert(Entries.end(), VEntries.begin(), VEntries.end());
-if (V.getLValueBase()) {
-  bool IsArray = false;
+if (auto Base = V.getLValueBase()) {
+  if (auto Decl = Base.dyn_cast())
+Base = cast(Decl->getMostRecentDecl());
+  bool IsArray = false, IsUnsizedArray = false;
   MostDerivedPathLength = findMostDerivedSubobject(
-  Ctx, V.getLValueBase(), V.getLValuePath

[PATCH] D32372: Arrays of unknown bound in constant expressions

2017-04-25 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth updated this revision to Diff 96603.
Arcoth added a comment.

Small fix.


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-array-unknown-bound.cpp

Index: test/SemaCXX/constexpr-array-unknown-bound.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-array-unknown-bound.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return arr[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re@-1 {{in call to 'f({{.*}})'}}
+
+void g(int i) {
+int arr[i];
+constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{runtime bound}}
+}
+
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // ok
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{unknown bound}}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -495,7 +495,7 @@
 // FIXME: Force the precision of the source value down so we don't
 // print digits which are usually useless (we don't really care here if
 // we truncate a digit by accident in edge cases).  Ideally,
-// APFloat::toString would automatically print the shortest 
+// APFloat::toString would automatically print the shortest
 // representation which rounds to the correct value, but it's a bit
 // tricky to implement.
 unsigned precision =
@@ -720,7 +720,7 @@
   private:
 OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId,
 unsigned ExtraNotes, bool IsCCEDiag) {
-
+
   if (EvalStatus.Diag) {
 // If we have a prior diagnostic, it will be noting that the expression
 // isn't a constant expression. This diagnostic is more important,
@@ -773,7 +773,7 @@
   unsigned ExtraNotes = 0) {
   return Diag(Loc, DiagId, ExtraNotes, false);
 }
-
+
 OptionalDiagnostic FFDiag(const Expr *E, diag::kind DiagId
   = diag::note_invalid_subexpr_in_const_expr,
 unsigned ExtraNotes = 0) {
@@ -2946,6 +2946,17 @@
   return CompleteObject();
 }
 
+// The complete object can be an array of unknown bound, in which case we
+// have to find the most recent declaration and adjust the type accordingly.
+if (Info.Ctx.getAsIncompleteArrayType(BaseType)) {
+  QualType MostRecentType =
+ cast(D->getMostRecentDecl())->getType();
+  if (auto CAT = Info.Ctx.getAsConstantArrayType(MostRecentType))
+BaseType = MostRecentType;
+  else
+return CompleteObject();
+}
+
 // Accesses of volatile-qualified objects are not allowed.
 if (BaseType.isVolatileQualified()) {
   if (Info.getLangOpts().CPlusPlus) {
@@ -4098,13 +4109,13 @@
 
   if (Info.getLangOpts().CPlusPlus11) {
 const FunctionDecl *DiagDecl = Definition ? Definition : Declaration;
-
+
 // If this function is not constexpr because it is an inherited
 // non-constexpr constructor, diagnose that directly.
 auto *CD = dyn_cast(DiagDecl);
 if (CD && CD->isInheritingConstructor()) {
   auto *Inherited = CD->getInheritedConstructor().getConstructor();
-  if (!Inherited->isConstexpr()) 
+  if (!Inherited->isConstexpr())
 DiagDecl = CD = Inherited;
 }
 
@@ -4635,7 +4646,7 @@
   return false;
 This = &ThisVal;
 Args = Args.slice(1);
-  } else if (MD && MD->isLambdaStaticInvoker()) {   
+  } else if (MD && MD->isLambdaStaticInvoker()) {
 // Map the static invoker for the lambda back to the call operator.
 // Conveniently, we don't have to slice out the 'this' argument (as is
 // being done for the non-static case), since a static member function
@@ -4670,7 +4681,7 @@
   FD = LambdaCallOp;
   }
 
-  
+
 } else
   return Error(E);
 
@@ -5501,7 +5512,7 @@
   // Update 'Result' to refer to the data member/field of the closure object
   // that represents the '*this' capture.
   if (!HandleLValueMember(Info, E, Result,
- Info.CurrentCall->LambdaThisCaptureField)) 
+ Info.CurrentCall->LambdaThisCaptureField))
 return false;
   // If we captured '*this' by reference, replace the field with its referent.
   if (Info.CurrentCall->LambdaThisCaptureField->getType()
@@ -5545,6 +5556,16 @@
   if (!EvaluateInteger(IExp, Offset, Info) || !EvalPtrOK)
 return false;
 

[PATCH] D32372: Arrays of unknown bound in constant expressions

2017-04-25 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth updated this revision to Diff 96594.
Arcoth marked 6 inline comments as done.
Arcoth added a comment.

Adjusted.

Implemented the changes suggested by Richard. The array-to-pointer decay 
is now well-formed; the check was instead moved to the pointer 
arithmetic facility.


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp
  test/SemaCXX/constexpr-array-unknown-bound.cpp

Index: test/SemaCXX/constexpr-array-unknown-bound.cpp
===
--- /dev/null
+++ test/SemaCXX/constexpr-array-unknown-bound.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -Wno-uninitialized -std=c++1z -fsyntax-only -verify
+
+const extern int arr[];
+constexpr auto p = arr; // ok
+constexpr int f(int i) {return arr[i];} // expected-note {{read of dereferenced one-past-the-end pointer}}
+
+constexpr int arr[] {1, 2, 3};
+constexpr auto p2 = arr + 2; // ok
+constexpr int x = f(2); // ok
+constexpr int y = f(3); // expected-error {{constant expression}}
+// expected-note-re@-1 {{in call to 'f({{.*}})'}}
+
+void g(int i) {
+int arr[i];
+constexpr auto p = arr + 2; // expected-error {{constant expression}} expected-note {{runtime bound}}
+}
+
+struct A {int m[];} a;
+constexpr auto p3 = a.m; // ok
+constexpr auto p4 = a.m + 1; // expected-error {{constant expression}} expected-note {{unknown bound}}
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -495,7 +495,7 @@
 // FIXME: Force the precision of the source value down so we don't
 // print digits which are usually useless (we don't really care here if
 // we truncate a digit by accident in edge cases).  Ideally,
-// APFloat::toString would automatically print the shortest 
+// APFloat::toString would automatically print the shortest
 // representation which rounds to the correct value, but it's a bit
 // tricky to implement.
 unsigned precision =
@@ -720,7 +720,7 @@
   private:
 OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId,
 unsigned ExtraNotes, bool IsCCEDiag) {
-
+
   if (EvalStatus.Diag) {
 // If we have a prior diagnostic, it will be noting that the expression
 // isn't a constant expression. This diagnostic is more important,
@@ -773,7 +773,7 @@
   unsigned ExtraNotes = 0) {
   return Diag(Loc, DiagId, ExtraNotes, false);
 }
-
+
 OptionalDiagnostic FFDiag(const Expr *E, diag::kind DiagId
   = diag::note_invalid_subexpr_in_const_expr,
 unsigned ExtraNotes = 0) {
@@ -2946,6 +2946,17 @@
   return CompleteObject();
 }
 
+// The complete object can be an array of unknown bound, in which case we
+// have to find the most recent declaration and adjust the type accordingly.
+if (Info.Ctx.getAsIncompleteArrayType(BaseType)) {
+  QualType MostRecentType =
+ cast(D->getMostRecentDecl())->getType();
+  if (auto CAT = Info.Ctx.getAsConstantArrayType(MostRecentType))
+BaseType = MostRecentType;
+  else
+return CompleteObject();
+}
+
 // Accesses of volatile-qualified objects are not allowed.
 if (BaseType.isVolatileQualified()) {
   if (Info.getLangOpts().CPlusPlus) {
@@ -4098,13 +4109,13 @@
 
   if (Info.getLangOpts().CPlusPlus11) {
 const FunctionDecl *DiagDecl = Definition ? Definition : Declaration;
-
+
 // If this function is not constexpr because it is an inherited
 // non-constexpr constructor, diagnose that directly.
 auto *CD = dyn_cast(DiagDecl);
 if (CD && CD->isInheritingConstructor()) {
   auto *Inherited = CD->getInheritedConstructor().getConstructor();
-  if (!Inherited->isConstexpr()) 
+  if (!Inherited->isConstexpr())
 DiagDecl = CD = Inherited;
 }
 
@@ -4635,7 +4646,7 @@
   return false;
 This = &ThisVal;
 Args = Args.slice(1);
-  } else if (MD && MD->isLambdaStaticInvoker()) {   
+  } else if (MD && MD->isLambdaStaticInvoker()) {
 // Map the static invoker for the lambda back to the call operator.
 // Conveniently, we don't have to slice out the 'this' argument (as is
 // being done for the non-static case), since a static member function
@@ -4670,7 +4681,7 @@
   FD = LambdaCallOp;
   }
 
-  
+
 } else
   return Error(E);
 
@@ -5501,7 +5512,7 @@
   // Update 'Result' to refer to the data member/field of the closure object
   // that represents the '*this' capture.
   if (!HandleLValueMember(Info, E, Result,
- Info.CurrentCall->LambdaThisCaptureField)) 
+ Info.CurrentCall->LambdaThisCaptureField))
 return false;
   // If we captured '*this' by reference, re

[PATCH] D32372: Arrays of unknown bound in constant expressions

2017-04-21 Thread Robert Haberlach via Phabricator via cfe-commits
Arcoth created this revision.

Arrays of unknown bound are subject to some bugs in constant expressions.

const extern int arr[];
constexpr int f(int i) {return arr[i];}
constexpr int arr[] {1, 2, 3};
int main() {constexpr int x = f(2);}

This is spuriously rejected. On the other hand,

extern const int arr[];
constexpr int const* p = arr + 2;

compiles. The standard will presumably incorporate a rule that forbids 
array-to-pointer conversions on arrays of unknown bound (unless they are 
completed at the point of evaluation, as in the first
example). The proposed changes reflect this idea.


https://reviews.llvm.org/D32372

Files:
  include/clang/Basic/DiagnosticASTKinds.td
  lib/AST/ExprConstant.cpp

Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -495,7 +495,7 @@
 // FIXME: Force the precision of the source value down so we don't
 // print digits which are usually useless (we don't really care here if
 // we truncate a digit by accident in edge cases).  Ideally,
-// APFloat::toString would automatically print the shortest 
+// APFloat::toString would automatically print the shortest
 // representation which rounds to the correct value, but it's a bit
 // tricky to implement.
 unsigned precision =
@@ -720,7 +720,7 @@
   private:
 OptionalDiagnostic Diag(SourceLocation Loc, diag::kind DiagId,
 unsigned ExtraNotes, bool IsCCEDiag) {
-
+
   if (EvalStatus.Diag) {
 // If we have a prior diagnostic, it will be noting that the expression
 // isn't a constant expression. This diagnostic is more important,
@@ -773,7 +773,7 @@
   unsigned ExtraNotes = 0) {
   return Diag(Loc, DiagId, ExtraNotes, false);
 }
-
+
 OptionalDiagnostic FFDiag(const Expr *E, diag::kind DiagId
   = diag::note_invalid_subexpr_in_const_expr,
 unsigned ExtraNotes = 0) {
@@ -2619,10 +2619,23 @@
 LastField = nullptr;
 if (ObjType->isArrayType()) {
   // Next subobject is an array element.
-  const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(ObjType);
-  assert(CAT && "vla in literal type?");
+  uint64_t actualIndex;
+  const ArrayType *AT = Info.Ctx.getAsArrayType(ObjType); // non-null by assumption
+  if (I == 0) {
+/* we're dealing with the complete array object, which may have been declared
+   without a bound */
+actualIndex = Sub.MostDerivedArraySize;
+assert(Sub.MostDerivedIsArrayElement && "Complete object is an array, but isn't?");
+  }
+  else {
+/* ... the array must have been given a bound
+([dcl.array]/3 for both subarrays and members). */
+auto CAT = dyn_cast(AT);
+assert(CAT && "vla in literal type?");
+actualIndex = CAT->getSize().getZExtValue();
+  }
   uint64_t Index = Sub.Entries[I].ArrayIndex;
-  if (CAT->getSize().ule(Index)) {
+  if (actualIndex <= Index) {
 // Note, it should not be possible to form a pointer with a valid
 // designator which points more than one past the end of the array.
 if (Info.getLangOpts().CPlusPlus11)
@@ -2633,7 +2646,7 @@
 return handler.failed();
   }
 
-  ObjType = CAT->getElementType();
+  ObjType = AT->getElementType();
 
   // An array object is represented as either an Array APValue or as an
   // LValue which refers to a string literal.
@@ -4098,13 +4111,13 @@
 
   if (Info.getLangOpts().CPlusPlus11) {
 const FunctionDecl *DiagDecl = Definition ? Definition : Declaration;
-
+
 // If this function is not constexpr because it is an inherited
 // non-constexpr constructor, diagnose that directly.
 auto *CD = dyn_cast(DiagDecl);
 if (CD && CD->isInheritingConstructor()) {
   auto *Inherited = CD->getInheritedConstructor().getConstructor();
-  if (!Inherited->isConstexpr()) 
+  if (!Inherited->isConstexpr())
 DiagDecl = CD = Inherited;
 }
 
@@ -4635,7 +4648,7 @@
   return false;
 This = &ThisVal;
 Args = Args.slice(1);
-  } else if (MD && MD->isLambdaStaticInvoker()) {   
+  } else if (MD && MD->isLambdaStaticInvoker()) {
 // Map the static invoker for the lambda back to the call operator.
 // Conveniently, we don't have to slice out the 'this' argument (as is
 // being done for the non-static case), since a static member function
@@ -4670,7 +4683,7 @@
   FD = LambdaCallOp;
   }
 
-  
+
 } else
   return Error(E);
 
@@ -5501,7 +5514,7 @@
   // Update 'Result' to refer to the data member/field of the closure object
   // that represents the '*this' capture.
   if (!HandleLValueMember(Info, E, Result,
- Info.CurrentCall->Lambda