[PATCH] Fix for 18283 - Const int initialized with curly braces is not a compile time constant
Rahul Jain
1989.rahuljain at gmail.com
Sun Jan 26 23:34:41 PST 2014
Thanks Richard! I have closed the bug.
Thanks to Suyog Sarda for working with me on this!
On Sun, Jan 26, 2014 at 2:26 AM, Richard Smith <richard at metafoo.co.uk>wrote:
> Thanks! I've committed the _2 patch as r200098. Please go ahead and close
> the bug.
>
>
> On Sat, Jan 25, 2014 at 6:48 AM, Rahul Jain <1989.rahuljain at gmail.com>wrote:
>
>> Good catch Richard! Thanks.
>>
>> Have attached a couple of workarounds for the same. Both make sure that
>> the correct statements are executed
>> for respective cases.
>>
>> Thanks,
>> Rahul
>>
>>
>>
>> On Sat, Jan 25, 2014 at 4:29 AM, Richard Smith <metafoo at gmail.com> wrote:
>>
>>> 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/20140127/69c53413/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/20140127/69c53413/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/20140127/69c53413/attachment-0001.gif>
More information about the cfe-commits
mailing list