[PATCH] Fix for 18283 - Const int initialized with curly braces is not a compile time constant
Richard Smith
richard at metafoo.co.uk
Sat Jan 25 12:56:11 PST 2014
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/20140125/40c5133a/attachment.html>
-------------- 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/20140125/40c5133a/attachment.gif>
-------------- 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/20140125/40c5133a/attachment-0001.gif>
More information about the cfe-commits
mailing list