[PATCH] Fix for 18283 - Const int initialized with curly braces is not a compile time constant

[Richard Smith]

On Fri Jan 24 2014 at 3:07:55 AM, Rahul Jain <rahul1.jain at samsung.com>
wrote:

>
>
> Hi Richard,
>
>
>
> Thanks for your inputs.
>
> As per review comments, I am now checking if the element contained in the
> InitList is an ICE.
>
> Also added a test in constant-expression.cpp.
>
> Please comment for further improvements.
>
>
>
> Patch:
>
>
>
> Index: test/SemaCXX/constant-expression.cpp
> ===================================================================
> --- test/SemaCXX/constant-expression.cpp (revision 199888)
> +++ test/SemaCXX/constant-expression.cpp (working copy)
> @@ -124,3 +124,12 @@
>    struct Y { bool b; X x; }; // expected-error {{field has incomplete
> type 'test3::X'}}
>    int f() { return Y().b; }
>  }
> +
> +// PR18283
> +namespace test4 {
> +  template <int> struct A {};
> +  int const i = { 42 };
> +  // i can be used as non-type template-parameter as "const int x = { 42
> };" is
> +  // equivalent to "const int x = 42;" as per C++03 8.5/p13.
> +  typedef A<i> Ai; // ok
> +}
> Index: lib/AST/ExprConstant.cpp
> ===================================================================
> --- lib/AST/ExprConstant.cpp (revision 199888)
> +++ lib/AST/ExprConstant.cpp (working copy)
> @@ -8331,7 +8331,14 @@
>    case Expr::MaterializeTemporaryExprClass:
>    case Expr::PseudoObjectExprClass:
>    case Expr::AtomicExprClass:
> -  case Expr::InitListExprClass:
> +  case Expr::InitListExprClass: {
>

The fallthrough here isn't correct. What follows should only apply to
InitListExprClass, not to the other kinds of expression that land here.

+    // C++03 8.5/13 says that If T is a scalar type, then a declaration of
> the
> +    // form "T x = { a };" is equivalent to "T x = a;". T is a scalar as
> it is
> +    // known to be of integral or enumeration type.
> +    if (E->isRValue())
> +      if (cast<InitListExpr>(E)->getNumInits() == 1)
> +        return CheckICE(cast<InitListExpr>(E)->getInit(0), Ctx);
> +  }
>    case Expr::LambdaExprClass:
>      return ICEDiag(IK_NotICE, E->getLocStart());
>
Thanks,
>
Rahul
>
>
>
> ------- *Original Message* -------
>
> *Sender* : Richard Smith<metafoo at gmail.com>
>
> *Date* : Jan 24, 2014 04:00 (GMT+05:30)
>
> *Title* : [PATCH] Fix for 18283 - Const int initialized with curly braces
> is not a compile time constant
>
>
> On Thu Jan 23 2014 at 2:24:25 AM, Rahul Jain <rahul1.jain at samsung.com>
> wrote:
>
>
>
> Thanks Marshal and Richard for the valuable inputs.
>
>
>
> Hi Richard,
>
>
>
> I have implemented a patch as per your suggestion, adding a check to
>
> handle the case of InitListExpr, when it is used as an RValue.
>
>
>
> Please if you could review it and suggest improvements.
>
>
> The patch needs a corresponding regression test to be added to some file
> in tests/. test/SemaCXX/constant-expression.cpp seems like a good place.
>
>  Index: tools/clang/lib/AST/ExprConstant.cpp
> ===================================================================
> --- tools/clang/lib/AST/ExprConstant.cpp (revision 199874)
> +++ tools/clang/lib/AST/ExprConstant.cpp (working copy)
> @@ -8331,7 +8331,14 @@
>    case Expr::MaterializeTemporaryExprClass:
>    case Expr::PseudoObjectExprClass:
>    case Expr::AtomicExprClass:
> -  case Expr::InitListExprClass:
> +  case Expr::InitListExprClass: {
> +    // Fix for PR 18283, if InitListExpr is used as an RValue containing
> +    // only one scalar value, it is an ICE.
>
>
> Don't reference a PR number here; the code should be able to justify its
> existence without reference to bugs. Instead, you can say that C++03
> [dcl.init]p13 says that "T x = { a };" is equivalent to "T x = a;" when T
> is a scalar type (and maybe point out that T must be a scalar type because
> we know it's an integral or enumeration type). You should reference the PR
> number in the test case, instead.
>
>  +    if (E->isRValue())
> +      if (cast<InitListExpr>(E)->getNumInits() == 1)
> +        return NoDiag();
>
>
> This isn't correct -- you can't assume that any braced initialization is a
> constant expression. Instead, call CheckICE on the subexpression.
>
>  +  }
> +
>    case Expr::LambdaExprClass:
>      return ICEDiag(IK_NotICE, E->getLocStart());
>
>
>
>
> Thanks,
>
> Rahul
>
>
>
> ------- *Original Message* -------
>
> *Sender* : Richard Smith<richard at metafoo.co.uk>
>
> *Date* : Jan 23, 2014 01:12 (GMT+05:30)
>
> *Title* : Re: [cfe-dev] C++ 03 - Need clarification regarding non-type
> template argument
>
>
>  On Wed, Jan 22, 2014 at 11:22 AM, Marshall Clow <mclow.lists at gmail.com>wrote:
>
>
>  On Jan 21, 2014, at 5:59 AM, Rahul Jain <rahul1.jain at samsung.com> wrote:
>
>
>
>
> Hi all,
>
>
> Note - this is with respect to C++ 03.
>
> *Test code:*
>
>
>
> *template <int> struct A {};int const i = {42};typedef A<i> Ai;*
>
>
> Short, snarky answer - you can’t bracket-initialize a variable in c++03
>
>
> You can in a few cases: specifically, if the type is an aggregate or a
> scalar type. In the latter case,
>
>    T x = { a };
>
> is equivalent to
>
>   T x = a;
>
> (See C++03 [dcl.init](8.5)/13.)
>
>
>  int const i = {42};
>
>   For non-type template argument, standard (C++ 03) says that:
>  *14.3.2 / 1*
>
> *A template-argument for a non-type, non-template template-parameter shall
> be one of:*
>
>     - *an integral constant-expression of integral or enumeration type;
>    or ......*
>
> For the test code above, the statement '*typedef A<i> Ai*', should the
> compiler
> check the rhs (rvalue) of variable 'i' to determine if it is a constant or
> not?
>
>
> Yes. For 'i' to be an integral constant-expression, it can only mention
> "const variables [...] of integral or enumeration types initialized with
> constant expressions" (C++03 [expr.const](5.19)/1). So the compiler is
> required to check whether 'i' is indeed initialized by a constant
> expression.
>
>    Shouldnt it just check the type of i (which is declared as const)?
> Whether 'i' evaluates to constant (at compile time) or not should be the
> responsibility of compiler
> while processing statement '*int const i = {42}*' and not '*typedef A<i>
> Ai' ? Isnt it?*
>
> Another analogy:
> For arrays, the size of the array needs to be a compile time constant. So,
> the following code :
>
> *int const x = 42;*
> *int z[x];*
>
> For the statement *'int z[x]'*, will the compiler evaluate the rhs of
> *'x'* to check if it evaluates to constant at runtime
> or will it just see that '*x*' is of 'integer const' type and hence there
> is no problem with the array declaration?
> Whether '*x*' evaluates to constant should be the responsibility of
> compiler while processing statement '*int const x=42*'
> and not while processing '*int z[x]*' ?
>
> Should this analogy be applicable to non-type template argument?
>
>
> According to the C++ standard, the two cases are exactly the same.
> However, to support code relying on GCC extensions, we allow any
> constant-foldable expression as an array bound, whereas we require a
> constant-expression as a template argument. Use -pedantic-errors to enable
> Clang's strictly-conforming mode, where we (incorrectly) reject both cases.
>
> The bug is that CheckICE in lib/AST/ExprConstant.cpp (or
> maybe VarDecl::checkInitIsICE) doesn't handle the case of an InitListExpr
> being used to initialize a scalar.
>
>     Please throw some light on this, am I missing any particular case?
>
>
> My understanding (which may be flawed) is that the compiler needs to see
> both:
> * That the compiler needs to be able to determine that the value it needs
> is const
> * That the compiler can see what the value actually is, so that it can
> allocate space for the array, or instantiate the template.
>
>
> So, this would not work:
>
> extern const int i;
> template <int> struct A {};
> typedef A<i> Ai;
>
>
> Right.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140124/4d72a68e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 201401241637260_BEI0XT4N.gif
Type: image/gif
Size: 14036 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140124/4d72a68e/attachment.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 201401241637275_2LL5XOK0.gif
Type: image/gif
Size: 14036 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140124/4d72a68e/attachment-0001.gif>