[PATCH] Fix for 18283 - Const int initialized with curly braces is not a compile time constant
Rahul Jain
1989.rahuljain at gmail.com
Sat Jan 25 06:48:38 PST 2014
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/b862dcab/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/20140125/b862dcab/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/20140125/b862dcab/attachment-0001.gif>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PR18283_1.patch
Type: application/octet-stream
Size: 1712 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140125/b862dcab/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PR18283_2.patch
Type: application/octet-stream
Size: 1672 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140125/b862dcab/attachment-0001.obj>
More information about the cfe-commits
mailing list