Concepts: Create space for requires-clause in TemplateParameterList; NFC

[hubert.reinterpretcast]

Summary

Space for storing the constraint-expression of the requires-clause associated with a TemplateParameterList is arranged by taking a bit out of the NumParams field for the purpose of determining whether there is a requires-clause or not, and by adding to the trailing objects tied to the TemplateParameterList. An accessor is provided.

An appropriate argument is supplied to TemplateParameterList::Create at the various call sites.

Serialization changes will addressed as the Concepts implementation becomes more solid.

Drive-by fix:
This change also replaces the custom FixedSizeTemplateParameterListStorage implementation with one that follows the interface provided by llvm::TrailingObjects.

[hubert.reinterpretcast]

Store the RequiresClause expression into the created storage;
add accessor;
add unnecessary parens to silence warning

[hubert.reinterpretcast]

Actually add parens this time

[faisalv]

include/clang/AST/DeclTemplate.h

174

Yuk - this entire guy (FizedSizeTemplateParameterListStorage) seems quite fragile (dependent on object layout) - are the gains (in the single use below during auto-type deduction) in preformance really worth the introduction of this fragility/ugliness?
Unless there is a clear win from this strategy, I think i'd favor (perhaps in a later patch) - either just removing this structure and using TPL for the use-case in auto-type below, or using placement new and creating the stack TPL on a stack unsigned char array?
Thoughts?

[hubert.reinterpretcast]

include/clang/AST/DeclTemplate.h

174

I don't like the class here either; however, I would not like to introduce heap allocation just to remove it. placement-new is certainly part of the solution. I think that TrailingObjects should have an alias for a type that can be used as aligned storage (templated in the style of totalSizeToAlloc). I would like to keep that endeavour to a separate patch though.

[hubert.reinterpretcast]

Address Faisal's comment; supercedes D19771

Replaces the custom FixedSizeTemplateParameterListStorage implementation with one that follows the interface provided by llvm::TrailingObjects.

[hubert.reinterpretcast]

@rsmith; I've addressed Faisal's comment. Please let me know if this patch (and D19770) is good to go. If it isn't ready yet, I'd like your opinion on D19770+D19771.

[hubert.reinterpretcast]

Ping.

[hubert.reinterpretcast]

Set requires-clause when creating TemplateParameterLists; NFC

Removes the default argument for the requires-clause constraint expression in TemplateParameterList::Create.

An appropriate argument is supplied at the various call sites. Serialization changes will be left until the feature is otherwise complete so that the version number does not need to be bumped multiple times.

[hubert.reinterpretcast]

Ping (#2).

[hubert.reinterpretcast]

Ping.

[rsmith]

lib/AST/ASTImporter.cpp

2253–2254

else on same line as } please.

include/clang/AST/DeclTemplate.h

 
 /// \brief Stores a template parameter of any kind.
 typedef llvm::PointerUnion3<TemplateTypeParmDecl*, NonTypeTemplateParmDecl*,
                             TemplateTemplateParmDecl*> TemplateParameter;
 
 /// \brief Stores a list of template parameters for a TemplateDecl and its
 /// derived classes.
 class TemplateParameterList final
-    : private llvm::TrailingObjects<TemplateParameterList, NamedDecl *> {
+    : private llvm::TrailingObjects<TemplateParameterList, NamedDecl *,
+                                    Expr *> {
 
   /// The location of the 'template' keyword.
   SourceLocation TemplateLoc;
 
   /// The locations of the '<' and '>' angle brackets.
   SourceLocation LAngleLoc, RAngleLoc;
 
   /// The number of template parameters in this template
   /// parameter list.
-  unsigned NumParams : 31;
+  unsigned NumParams : 30;
 
   /// Whether this template parameter list contains an unexpanded parameter
   /// pack.
   unsigned ContainsUnexpandedParameterPack : 1;
 
+  /// Whether this template parameter list has an associated requires-clause
+  unsigned HasRequiresClause : 1;
+
 protected:
   size_t numTrailingObjects(OverloadToken<NamedDecl *>) const {
     return NumParams;
   }
 
+  size_t numTrailingObjects(OverloadToken<Expr *>) const {
+    return HasRequiresClause;
+  }
+
   TemplateParameterList(SourceLocation TemplateLoc, SourceLocation LAngleLoc,
-                        ArrayRef<NamedDecl *> Params, SourceLocation RAngleLoc);
+                        ArrayRef<NamedDecl *> Params, SourceLocation RAngleLoc,
+                        Expr *RequiresClause);
 
 public:
   static TemplateParameterList *Create(const ASTContext &C,
                                        SourceLocation TemplateLoc,
                                        SourceLocation LAngleLoc,
                                        ArrayRef<NamedDecl *> Params,
-                                       SourceLocation RAngleLoc);
+                                       SourceLocation RAngleLoc,
+                                       Expr *RequiresClause);
 
   /// \brief Iterates through the template parameters in this list.
   typedef NamedDecl** iterator;
 
   /// \brief Iterates through the template parameters in this list.
   typedef NamedDecl* const* const_iterator;
 
   iterator begin() { return getTrailingObjects<NamedDecl *>(); }
public:
   unsigned getDepth() const;
 
   /// \brief Determine whether this template parameter list contains an
   /// unexpanded parameter pack.
   bool containsUnexpandedParameterPack() const {
     return ContainsUnexpandedParameterPack;
   }
 
+  /// \brief The constraint-expression of the associated requires-clause.
+  Expr *getRequiresClause() {
+    return HasRequiresClause ? *getTrailingObjects<Expr *>() : nullptr;
+  }
+
+  /// \brief The constraint-expression of the associated requires-clause.
+  const Expr *getRequiresClause() const {
+    return HasRequiresClause ? *getTrailingObjects<Expr *>() : nullptr;
+  }
+
   SourceLocation getTemplateLoc() const { return TemplateLoc; }
   SourceLocation getLAngleLoc() const { return LAngleLoc; }
   SourceLocation getRAngleLoc() const { return RAngleLoc; }
 
   SourceRange getSourceRange() const LLVM_READONLY {
     return SourceRange(TemplateLoc, RAngleLoc);
   }
 
   friend TrailingObjects;
-  template <size_t N> friend class FixedSizeTemplateParameterListStorage;
+
+  template <size_t N, bool HasRequiresClause>
+  friend class FixedSizeTemplateParameterListStorage;
 };
 
-/// \brief Stores a list of template parameters for a TemplateDecl and its
-/// derived classes. Suitable for creating on the stack.
-template <size_t N> class FixedSizeTemplateParameterListStorage {
-  // This is kinda ugly: TemplateParameterList usually gets allocated
-  // in a block of memory with NamedDecls appended to it. Here, to get
-  // it stack allocated, we include the params as a separate
-  // variable. After allocation, the TemplateParameterList object
-  // treats them as part of itself.
-  TemplateParameterList List;
-  NamedDecl *Params[N];
+/// \brief Stores a list of template parameters and the associated
+/// requires-clause (if any) for a TemplateDecl and its derived classes.
+/// Suitable for creating on the stack.
+template <size_t N, bool HasRequiresClause>
+class FixedSizeTemplateParameterListStorage
+    : public TemplateParameterList::FixedSizeStorageOwner {
+  typename TemplateParameterList::FixedSizeStorage<
+      NamedDecl *, Expr *>::with_counts<
+      N, HasRequiresClause ? 1u : 0u
+      >::type storage;
 
 public:
   FixedSizeTemplateParameterListStorage(SourceLocation TemplateLoc,
                                         SourceLocation LAngleLoc,
                                         ArrayRef<NamedDecl *> Params,
-                                        SourceLocation RAngleLoc)
-      : List(TemplateLoc, LAngleLoc, Params, RAngleLoc) {
-    // Because we're doing an evil layout hack above, have some
-    // asserts, just to double-check everything is laid out like
-    // expected.
-    assert(sizeof(*this) ==
-               TemplateParameterList::totalSizeToAlloc<NamedDecl *>(N) &&
-           "Object layout not as expected");
-    assert(this->Params == List.getTrailingObjects<NamedDecl *>() &&
-           "Object layout not as expected");
-  }
-  TemplateParameterList *get() { return &List; }
+                                        SourceLocation RAngleLoc,
+                                        Expr *RequiresClause)
+      : FixedSizeStorageOwner(
+            (assert(N == Params.size()),
+             assert(HasRequiresClause == static_cast<bool>(RequiresClause)),
+             new (static_cast<void *>(&storage)) TemplateParameterList(
+                 TemplateLoc, LAngleLoc, Params, RAngleLoc, RequiresClause))) {}
 };
 
 /// \brief A template argument list.
 class TemplateArgumentList final
     : private llvm::TrailingObjects<TemplateArgumentList, TemplateArgument> {
   /// \brief The template argument list.
   const TemplateArgument *Arguments;
 
TemplateDecl(Kind DK, DeclContext *DC, SourceLocation L, DeclarationName Name,
         TemplateParams(Params) {}
 
 public:
   /// Get the list of template parameters
   TemplateParameterList *getTemplateParameters() const {
     return TemplateParams;
   }
 
+  /// Get the constraint-expression from the associated requires-clause (if any)
+  const Expr *getRequiresClause() const {
+    return TemplateParams ? TemplateParams->getRequiresClause() : nullptr;
+  }
+
   /// Get the underlying, templated declaration.
   NamedDecl *getTemplatedDecl() const { return TemplatedDecl.getPointer(); }
 
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) {
     return K >= firstTemplate && K <= lastTemplate;
   }

lib/AST/ASTContext.cpp

else if (NonTypeTemplateParmDecl *NTTP
       }
       CanonParams.push_back(Param);
 
     } else
       CanonParams.push_back(getCanonicalTemplateTemplateParmDecl(
                                            cast<TemplateTemplateParmDecl>(*P)));
   }
 
+  assert(!TTP->getRequiresClause() &&
+         "Unexpected requires-clause on template template-parameter");
+  constexpr Expr *const CanonRequiresClause = nullptr;
+
   TemplateTemplateParmDecl *CanonTTP
     = TemplateTemplateParmDecl::Create(*this, getTranslationUnitDecl(), 
                                        SourceLocation(), TTP->getDepth(),
                                        TTP->getPosition(), 
                                        TTP->isParameterPack(),
                                        nullptr,
                          TemplateParameterList::Create(*this, SourceLocation(),
                                                        SourceLocation(),
                                                        CanonParams,
-                                                       SourceLocation()));
+                                                       SourceLocation(),
+                                                       CanonRequiresClause));
 
   // Get the new insert position for the node we care about.
   Canonical = CanonTemplateTemplateParms.FindNodeOrInsertPos(ID, InsertPos);
   assert(!Canonical && "Shouldn't be in the map!");
   (void)Canonical;
 
   // Create the canonical template template parameter entry.
   Canonical = new (*this) CanonicalTemplateTemplateParm(CanonTTP);

lib/AST/ASTImporter.cpp

for (TemplateParameterList::iterator P = Params->begin(),
        P != PEnd; ++P) {
     Decl *To = Importer.Import(*P);
     if (!To)
       return nullptr;
 
     ToParams.push_back(cast<NamedDecl>(To));
   }
   
+  Expr *ToRequiresClause;
+  if (Expr *const R = Params->getRequiresClause()) {
+    ToRequiresClause = Importer.Import(R);
+    if (!ToRequiresClause)
+      return nullptr;
+  }
+  else {
+    ToRequiresClause = nullptr;
+  }
+
   return TemplateParameterList::Create(Importer.getToContext(),
                                        Importer.Import(Params->getTemplateLoc()),
                                        Importer.Import(Params->getLAngleLoc()),
                                        ToParams,
-                                       Importer.Import(Params->getRAngleLoc()));
+                                       Importer.Import(Params->getRAngleLoc()),
+                                       ToRequiresClause);
 }
 
 TemplateArgument 
 ASTNodeImporter::ImportTemplateArgument(const TemplateArgument &From) {
   switch (From.getKind()) {
   case TemplateArgument::Null:
     return TemplateArgument();
      

lib/AST/DeclTemplate.cpp

 
 //===----------------------------------------------------------------------===//
 // TemplateParameterList Implementation
 //===----------------------------------------------------------------------===//
 
 TemplateParameterList::TemplateParameterList(SourceLocation TemplateLoc,
                                              SourceLocation LAngleLoc,
                                              ArrayRef<NamedDecl *> Params,
-                                             SourceLocation RAngleLoc)
+                                             SourceLocation RAngleLoc,
+                                             Expr *RequiresClause)
   : TemplateLoc(TemplateLoc), LAngleLoc(LAngleLoc), RAngleLoc(RAngleLoc),
-    NumParams(Params.size()), ContainsUnexpandedParameterPack(false) {
+    NumParams(Params.size()), ContainsUnexpandedParameterPack(false),
+    HasRequiresClause(static_cast<bool>(RequiresClause)) {
   assert(this->NumParams == NumParams && "Too many template parameters");
   for (unsigned Idx = 0; Idx < NumParams; ++Idx) {
     NamedDecl *P = Params[Idx];
     begin()[Idx] = P;
 
     if (!P->isTemplateParameterPack()) {
       if (NonTypeTemplateParmDecl *NTTP = dyn_cast<NonTypeTemplateParmDecl>(P))
         if (NTTP->getType()->containsUnexpandedParameterPack())
           ContainsUnexpandedParameterPack = true;
 
       if (TemplateTemplateParmDecl *TTP = dyn_cast<TemplateTemplateParmDecl>(P))
         if (TTP->getTemplateParameters()->containsUnexpandedParameterPack())
           ContainsUnexpandedParameterPack = true;
 
       // FIXME: If a default argument contains an unexpanded parameter pack, the
       // template parameter list does too.
     }
   }
+  if (RequiresClause) {
+    *getTrailingObjects<Expr *>() = RequiresClause;
+  }
 }
 
-TemplateParameterList *TemplateParameterList::Create(
-    const ASTContext &C, SourceLocation TemplateLoc, SourceLocation LAngleLoc,
-    ArrayRef<NamedDecl *> Params, SourceLocation RAngleLoc) {
-  void *Mem = C.Allocate(totalSizeToAlloc<NamedDecl *>(Params.size()),
+TemplateParameterList *
+TemplateParameterList::Create(const ASTContext &C, SourceLocation TemplateLoc,
+                              SourceLocation LAngleLoc,
+                              ArrayRef<NamedDecl *> Params,
+                              SourceLocation RAngleLoc, Expr *RequiresClause) {
+  void *Mem = C.Allocate(totalSizeToAlloc<NamedDecl *, Expr *>(
+                             Params.size(), RequiresClause ? 1u : 0u),
                          llvm::alignOf<TemplateParameterList>());
   return new (Mem) TemplateParameterList(TemplateLoc, LAngleLoc, Params,
-                                         RAngleLoc);
+                                         RAngleLoc, RequiresClause);
 }
 
 unsigned TemplateParameterList::getMinRequiredArguments() const {
   unsigned NumRequiredArgs = 0;
   for (iterator P = const_cast<TemplateParameterList *>(this)->begin(), 
              PEnd = const_cast<TemplateParameterList *>(this)->end(); 
        P != PEnd; ++P) {
     if ((*P)->isTemplateParameterPack()) {
createMakeIntegerSeqParameterList(const ASTContext &C, DeclContext *DC) {
   auto *N = NonTypeTemplateParmDecl::Create(
       C, DC, SourceLocation(), SourceLocation(), /*Depth=*/0, /*Position=*/1,
       /*Id=*/nullptr, TI->getType(), /*ParameterPack=*/true, TI);
   N->setImplicit(true);
 
   // <typename T, T ...Ints>
   NamedDecl *P[2] = {T, N};
   auto *TPL = TemplateParameterList::Create(
-      C, SourceLocation(), SourceLocation(), P, SourceLocation());
+      C, SourceLocation(), SourceLocation(), P, SourceLocation(), nullptr);
 
   // template <typename T, ...Ints> class IntSeq
   auto *TemplateTemplateParm = TemplateTemplateParmDecl::Create(
       C, DC, SourceLocation(), /*Depth=*/0, /*Position=*/0,
       /*ParameterPack=*/false, /*Id=*/nullptr, TPL);
   TemplateTemplateParm->setImplicit(true);
 
   // typename T
   auto *TemplateTypeParm = TemplateTypeParmDecl::Create(
       C, DC, SourceLocation(), SourceLocation(), /*Depth=*/0, /*Position=*/1,
       /*Id=*/nullptr, /*Typename=*/true, /*ParameterPack=*/false);
   TemplateTypeParm->setImplicit(true);
 
   // T N
   TypeSourceInfo *TInfo = C.getTrivialTypeSourceInfo(
       QualType(TemplateTypeParm->getTypeForDecl(), 0));
   auto *NonTypeTemplateParm = NonTypeTemplateParmDecl::Create(
       C, DC, SourceLocation(), SourceLocation(), /*Depth=*/0, /*Position=*/2,
       /*Id=*/nullptr, TInfo->getType(), /*ParameterPack=*/false, TInfo);
   NamedDecl *Params[] = {TemplateTemplateParm, TemplateTypeParm,
                          NonTypeTemplateParm};
 
   // template <template <typename T, T ...Ints> class IntSeq, typename T, T N>
   return TemplateParameterList::Create(C, SourceLocation(), SourceLocation(),
-                                       Params, SourceLocation());
+                                       Params, SourceLocation(), nullptr);
 }
 
 static TemplateParameterList *createBuiltinTemplateParameterList(
     const ASTContext &C, DeclContext *DC, BuiltinTemplateKind BTK) {
   switch (BTK) {
   case BTK__make_integer_seq:
     return createMakeIntegerSeqParameterList(C, DC);
   }

lib/Sema/SemaLambda.cpp

if (!LSI->AutoTemplateParams.empty()) {
     SourceRange IntroRange = LSI->IntroducerRange;
     SourceLocation LAngleLoc = IntroRange.getBegin();
     SourceLocation RAngleLoc = IntroRange.getEnd();
     LSI->GLTemplateParameterList = TemplateParameterList::Create(
         SemaRef.Context,
         /*Template kw loc*/ SourceLocation(), LAngleLoc,
         llvm::makeArrayRef((NamedDecl *const *)LSI->AutoTemplateParams.data(),
                            LSI->AutoTemplateParams.size()),
-        RAngleLoc);
+        RAngleLoc, nullptr);
   }
   return LSI->GLTemplateParameterList;
 }
 
 CXXRecordDecl *Sema::createLambdaClosureType(SourceRange IntroducerRange,
                                              TypeSourceInfo *Info,
                                              bool KnownDependent, 
                                              LambdaCaptureDefault CaptureDefault) {

lib/Sema/SemaTemplate.cpp

Sema::ActOnTemplateParameterList(unsigned Depth,
                                  SourceLocation TemplateLoc,
                                  SourceLocation LAngleLoc,
                                  ArrayRef<Decl *> Params,
                                  SourceLocation RAngleLoc,
                                  Expr *RequiresClause) {
   if (ExportLoc.isValid())
     Diag(ExportLoc, diag::warn_template_export_unsupported);
 
-  // FIXME: store RequiresClause
   return TemplateParameterList::Create(
       Context, TemplateLoc, LAngleLoc,
       llvm::makeArrayRef((NamedDecl *const *)Params.data(), Params.size()),
-      RAngleLoc);
+      RAngleLoc, RequiresClause);
 }
 
 static void SetNestedNameSpecifier(TagDecl *T, const CXXScopeSpec &SS) {
   if (SS.isSet())
     T->setQualifierInfo(SS.getWithLocInContext(T->getASTContext()));
 }
 
 DeclResult
if (TemplateId && !IsFriend) {
       // should.
       IsExplicitSpecialization = true;
       DiagnoseMissingExplicitSpecialization(SourceRange(TemplateId->LAngleLoc,
                                                         TemplateId->RAngleLoc));
 
       // Fabricate an empty template parameter list for the invented header.
       return TemplateParameterList::Create(Context, SourceLocation(),
                                            SourceLocation(), None,
-                                           SourceLocation());
+                                           SourceLocation(), nullptr);
     }
 
     return nullptr;
   }
 
   // If there were too many template parameter lists, complain about that now.
   if (ParamIdx < ParamLists.size() - 1) {
     bool HasAnyExplicitSpecHeader = false;

lib/Sema/SemaTemplateDeduction.cpp

Sema::DeduceAutoType(TypeLoc Type, Expr *&Init, QualType &Result) {
   LocalInstantiationScope InstScope(*this);
 
   // Build template<class TemplParam> void Func(FuncParam);
   TemplateTypeParmDecl *TemplParam =
     TemplateTypeParmDecl::Create(Context, nullptr, SourceLocation(), Loc, 0, 0,
                                  nullptr, false, false);
   QualType TemplArg = QualType(TemplParam->getTypeForDecl(), 0);
   NamedDecl *TemplParamPtr = TemplParam;
-  FixedSizeTemplateParameterListStorage<1> TemplateParamsSt(
-      Loc, Loc, TemplParamPtr, Loc);
+  FixedSizeTemplateParameterListStorage<1, false> TemplateParamsSt(
+      Loc, Loc, TemplParamPtr, Loc, nullptr);
 
   QualType FuncParam = SubstituteAutoTransform(*this, TemplArg).Apply(Type);
   assert(!FuncParam.isNull() &&
          "substituting template parameter for 'auto' failed");
 
   // Deduce type of TemplParam in Func(Init)
   SmallVector<DeducedTemplateArgument, 1> Deduced;
   Deduced.resize(1);

lib/Sema/SemaTemplateInstantiateDecl.cpp

for (auto &P : *L) {
     Params.push_back(D);
     Invalid = Invalid || !D || D->isInvalidDecl();
   }
 
   // Clean up if we had an error.
   if (Invalid)
     return nullptr;
 
+  // Note: we substitute into associated constraints later
+  Expr *const UninstantiatedRequiresClause = L->getRequiresClause();
+
   TemplateParameterList *InstL
     = TemplateParameterList::Create(SemaRef.Context, L->getTemplateLoc(),
                                     L->getLAngleLoc(), Params,
-                                    L->getRAngleLoc());
+                                    L->getRAngleLoc(),
+                                    UninstantiatedRequiresClause);
   return InstL;
 }
 
 /// \brief Instantiate the declaration of a class template partial
 /// specialization.
 ///
 /// \param ClassTemplate the (instantiated) class template that is partially
 // specialized by the instantiation of \p PartialSpec.

lib/Serialization/ASTReader.cpp

ASTReader::ReadTemplateParameterList(ModuleFile &F,
   SourceLocation RAngleLoc = ReadSourceLocation(F, Record, Idx);
 
   unsigned NumParams = Record[Idx++];
   SmallVector<NamedDecl *, 16> Params;
   Params.reserve(NumParams);
   while (NumParams--)
     Params.push_back(ReadDeclAs<NamedDecl>(F, Record, Idx));
 
+  // TODO: Concepts
   TemplateParameterList* TemplateParams =
     TemplateParameterList::Create(Context, TemplateLoc, LAngleLoc,
-                                  Params, RAngleLoc);
+                                  Params, RAngleLoc, nullptr);
   return TemplateParams;
 }
 
 void
 ASTReader::
 ReadTemplateArgumentList(SmallVectorImpl<TemplateArgument> &TemplArgs,
                          ModuleFile &F, const RecordData &Record,
                          unsigned &Idx, bool Canonicalize) {

lib/Serialization/ASTWriter.cpp

 }
 
 void ASTRecordWriter::AddTemplateParameterList(
     const TemplateParameterList *TemplateParams) {
   assert(TemplateParams && "No TemplateParams!");
   AddSourceLocation(TemplateParams->getTemplateLoc());
   AddSourceLocation(TemplateParams->getLAngleLoc());
   AddSourceLocation(TemplateParams->getRAngleLoc());
+  // TODO: Concepts
   Record->push_back(TemplateParams->size());
   for (const auto &P : *TemplateParams)
     AddDeclRef(P);
 }
 
 /// \brief Emit a template argument list.
 void ASTRecordWriter::AddTemplateArgumentList(
     const TemplateArgumentList *TemplateArgs) {