[cfe-commits] r147026 - in /cfe/trunk: lib/AST/Expr.cpp test/Sema/static-init.c

[Richard Smith]

On Thu, December 22, 2011 07:15, Eli Friedman wrote:
> On Wed, Dec 21, 2011 at 9:27 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> On Thu, December 22, 2011 04:48, Eli Friedman wrote:
>>> On Wed, Dec 21, 2011 at 2:51 PM, Eli Friedman <eli.friedman at gmail.com>
>>> wrote:
>>>> On Tue, Dec 20, 2011 at 7:42 PM, Richard Smith <richard at metafoo.co.uk>
>>>> wrote:
>>>>>> Err, great.  That will be a bit annoying to write.  It would make
>>>>>> all the constant stuff a bit less complicated if Evaluate could
>>>>>> handle all scalar expressions...
>>>>>
>>>>> I agree, and we're very close. The cases we can't evaluate are:
>>>>>
>>>>>  - difference of two address labels (would need a new APValue kind)
>>>>>  - calls to trivial constructors (which are not constexpr)
>>>>>  - vector bitcasts
>>>>>  - cast to union
>>>>
>>>> I think there might be a few other cases you're missing (one or two
>>>> involving compound literals, some cases involving string literals, maybe
>>>> a couple others), but yes, it's close.
>>>
>>> Here's a more-or-less complete list of what is missing on trunk to get
>>> through the clang regression tests with isConstantInitializer forwarding
>>> to Evaluate:
>>> - StringLiteral and ObjCEncodeExpr outside of an InitListExpr
>>> - InitListExprs of scalar type (int, float, etc.)
>>> - CXXTemporaryObjectExpr
>>
>> I'd be very interested in any specific examples where these don't work --
>> those would probably be holes in our constexpr support. Note that
>> InitListExpr
>> for scalars is supported by ExprEvaluatorBase, CXXTemporaryObjectExpr
>> derives from CXXConstructExpr so is supported by the code which handles
>> that, and ObjCEncodeExpr is supported by LValueExprEvaluator.
>
> For CXXTemporaryObjectExpr: we give an error for the following. I
> can't think of any obvious reason why it isn't legal:
>
> struct A { int x; }; constexpr A x = A();

That's ill-formed because a constexpr constructor is required to initialize
all members. We could do with a better diagnostic for this (and a bunch of
other cases). I think there's a FIXME for that one :)

> For InitLists, it looks like there's some code in
> ExprEvaluatorBase::VisitInitListExpr that we only enable in C++11 mode
> which isn't obviously C++11-specific; am I missing something?

I'm not sure how that check got in there :) That code isn't C++11-specific.

> Also, clang supports an extension involving _Complex that it would be nice
> to have constexpr support for:
>
> class B { _Complex float x = {1,2}; }; constexpr B y = B();

Agreed. Presumably there's an isConstantInitializer/Evaluate incompatibility
here for simple cases like:

 _Complex float x = {1,2};

> For StringLiterals, just trying to run 'char a[2] = "a";' through
> Evaluate causes issues, but it's hard to come up with a testcase that
> doesn't involve a patch.

isEvaluatable will try to evaluate as an rvalue, but the initializer is an
lvalue, and we don't support lvalue-to-rvalue conversions on StringLiterals as
a whole yet (just on character subobjects). This is partially deliberate: I've
tried to avoid creating APValues representing the contents of string literals
(since APValue is horribly inefficient at representing arrays of char), and
with care we can avoid doing so here.

I've been working towards a different constant value representation which
eschews this problem (and is generally much more space-efficient); I'd love
some comments if you have time to look over it:

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20111128/049672.html

> On a side note, I'm not sure if it's really related,
> but for StringLiterals, the following crashes:
>
> class C { char a[4] = "foo"; }; constexpr C a = C(); constexpr int b = a.a[0];

In this case, we really do need the string copy. This should (hopefully) be a
simple generalization of r147120.

> For ObjCEncodeExpr, the following breaks; it looks like there's a FIXME,
> though.
>
> constexpr char a[2] = @encode(int); cosntexpr char b = a[0];

Yes, string literals other than StringLiteral aren't supported in such cases
yet, though there's no isConstantInitializer / Evaluate incompatibility here
at least.

Thanks for digging into this.
Richard