[cfe-commits] r148348 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/Sema.cpp lib/Sema/SemaDeclCXX.cpp

[Sebastian Redl]

On 18.01.2012, at 00:34, Richard Smith wrote:

> On Tue, January 17, 2012 22:49, Sebastian Redl wrote:
>> Author: cornedbee
>> Date: Tue Jan 17 16:49:33 2012
>> New Revision: 148348
>> 
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=148348&view=rev
>> Log:
>> Add Sema::isStdInitializerList, which will be necessary for the upcoming
>> operations.
>> 
>> Modified:
>> cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/Sema.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> 
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=14
>> 8348&r1=148347&r2=148348&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Jan 17 16:49:33 2012
>> @@ -5767,6 +5767,59 @@
>> return getStdNamespace(); }
>> 
>> 
>> +bool Sema::isStdInitializerList(QualType Ty, QualType *Element) {
>> +  assert(getLangOptions().CPlusPlus &&
>> +         "Looking for std::initializer_list outside of C++.");
>> +
>> +  // We're looking for implicit instantiations of
>> +  // template <typename E> class std::initializer_list.
>> +
>> +  if (!StdNamespace) // If we haven't seen namespace std yet, this can't be
>> it. +    return false;
>> +
>> +  const RecordType *RT = Ty->getAs<RecordType>();
>> +  if (!RT)
>> +    return false;
>> +
>> +  ClassTemplateSpecializationDecl *Specialization =
>> +      dyn_cast<ClassTemplateSpecializationDecl>(RT->getDecl());
>> +  if (!Specialization)
>> +    return false;
>> +
>> +  if (Specialization->getSpecializationKind() != TSK_ImplicitInstantiation)
>> +    return false;
>> +
>> +  ClassTemplateDecl *Template = Specialization->getSpecializedTemplate();
> 
> It's not clear how we should behave if this isn't a specialization of the
> primary template -- [namespace.std]p1 seems to allow user-specialization of
> std::initializer_list, though that seems unimplementable in general.

I don't think we can do anything reasonable in such a case, so I haven't made any decision, just wrote the code that was easiest to write.
Option: disallow specialization. That would probably be the most reasonable approach, all things considered.
Option: allow specialization, but don't give special treatment to specializations of std::initializer_list (i.e. treat them as normal types). That's unintuitive, though.
Option: allow specialization and still give special treatment. As you said, unimplementable.

I'm still somewhat worried about the lack of specification on initializer_list in general. As far as I can tell, the common C++ ABI doesn't specify the layout or at least the constructor signature of initializer_list. This is a problem for codegen. Do we use a constructor of initializer_list? If so, which one? (E*, size_t), as in libstdc++ and libc++? (E*, E*) as in MSVC's stdlib? template <size_t N> (E &[N]), as it would be perfectly valid to have in some stdlib? Do we just look for these constructors and use the first we find? Or do we just initialize the initializer_list object in-place? Again, what's the layout? Pointer+size or pointer+pointer?

Also, I believe the specification of the lifetime of the backing array to be highly defective. [dcl.init.list] says:
"Otherwise, if T is a specialization of std::initializer_list<E>, an initializer_list object is constructed as described below and used to initialize the object according to the rules for initialization of an object from a class of the same type."
Note that it says that an object is constructed and then basically copied to the actual object. Below, it says:
"The lifetime of the array is the same as that of the initializer_list object."
So if a temporary is constructed, wouldn't the lifetime of the array be that of the temporary?
The example contradicts this, of course, since that would be stupid, but if the lifetime of the array is the same as that of the actual initializer_list object, then how do you implement this?
auto listptr = new std::initializer_list<int>{1, 2, 3, 4, 5};

Or for that matter, since it's equivalent:
auto listptr = new auto{1, 2, 3, 4, 5};

> 
>> +
>> +  if (!StdInitializerList) {
>> +    // Haven't recognized std::initializer_list yet, maybe this is it.
>> +    CXXRecordDecl *TemplateClass = Template->getTemplatedDecl();
>> +    if (TemplateClass->getIdentifier() !=
>> +            &PP.getIdentifierTable().get("initializer_list") ||
>> +        !TemplateClass->getDeclContext()->Equals(getStdNamespace()))
> 
> initializer_list might be inside an inline namespace within std (and I believe
> it is, in libc++). getStdNamespace()->InEnclosingNamespaceSetOf(your
> namespace) should handle this, IIRC.

Done. Although IMO initializer_list should be part of the ABI library and thus unversioned.

> 
>> +      return false;
>> +    // This is a template called std::initializer_list, but is it the right
>> +    // template?
>> +    TemplateParameterList *Params = Template->getTemplateParameters();
>> +    if (Params->size() != 1)
>> +      return false;
> 
> Minor nit: a standard library implementation is permitted to give
> initializer_list additional defaulted template parameters.

Done.

And the comment typo from the other commit as well. Thanks.

Sebastian