[Concepts] Class template associated constraints

[hubert.reinterpretcast]

Summary

This adds associated constraints as a property of class templates.
An error is produced if redeclarations are not similarly constrained.

[hubert.reinterpretcast]

test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp

12

I should probably add some template-dependent cases.

[hubert.reinterpretcast]

Friendly ping...

[rsmith]

include/clang/AST/DeclTemplate.h

412–417

Rather than tail-allocating the associated constraints (which will be an inconvenience for every class that derives from TemplateDecl), can you store this as a PointerUnion of a TemplateParameterList* and a pointer to a struct wrapping a TemplateParameterList* and the associated constraints expression? I'm not really concerned about paying one extra pointer per associated constraints expression we store (and I suspect we may want to store this as something richer than an expression at some point).

lib/Sema/SemaTemplate.cpp

1158

I'm not a big fan of continue as a proxy for a goto. It looks like you could rearrange the logic here to avoid this:

bool ConstraintMismatch = false;
if ((bool)CurAC != (bool)PrevAC)
  ConstraintMismatch = true;
else if (CurAC) {
  // profile and set ConstraintMismatch
}
if (ConstraintMismatch) Diag(...)

... or factor out a lambda to do the mismatch checking.

test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp

1

I like the idea of separate subdirectories of test/CXX for TSes. Thanks :)

[hubert.reinterpretcast]

Address review comments; update to revision 292996

Fix possibly ill-formed NDR case

Test template-dependent cases for class redeclaration

Address review comment: use lambda instead of do while(0)

Address review comment: use PointerUnion w/struct

[hubert.reinterpretcast]

include/clang/AST/DeclTemplate.h

355

Removed this constructor as a drive-by.

412–417

As discussed in person, I am using PointerUnion in the base class. I am still allocating the proposed struct alongside the TemplateDecl instance where it is not too much trouble.

test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp

9

Status of this line is unclear. A conservative user of the language should probably avoid this as ill-formed NDR (functionally equivalent but not equivalent). This line as been replaced.

12

Propagation of associated constraints to instantiated specializations will need to be added later.

[hubert.reinterpretcast]

Ping!

[hubert.reinterpretcast]

Ping 2?

[rsmith]

include/clang/AST/DeclTemplate.h

373–391

This mechanism seems unnecessary to me; allocating the ConstrainedTemplateDeclInfo separately seems a lot simpler. Forcing this and the template into a single allocation is unlikely to help anything since we use a slab allocator (which is going to lay the objects out the same way this template trick does, unless we hit the end of a slab).

lib/Sema/SemaTemplate.cpp

1158

[nit] Comments should be full sentences: capitalized and ending in a period.

1268–1269

Is there a reason you don't want to store the associated constraints that were specified on a redeclaration? I'd expect that to hurt tools that want source fidelity (for instance, a renaming tool will want to be able to find all the references to a particular name, even in a requires-clause on a redeclaration of a template).

[hubert.reinterpretcast]

include/clang/AST/DeclTemplate.h

373–391

Okay; I'll probably allocate separately in the Create function.

lib/Sema/SemaTemplate.cpp

1158

Okay.

1268–1269

Associated constraints are not part of the source fidelity: the requires-clause on the template-parameter-list is (and later, if constraints are introduced by abbreviated function call syntax, etc., the function parameter list).

[hubert.reinterpretcast]

Address review comments; update to revision 294580

Allocate ConstrainedTemplateDeclInfo separately

Update comments to be sentences; NFC

[rsmith]

lib/Sema/SemaTemplate.cpp

1269

Add period to this sentence.

include/clang/AST/DeclTemplate.h

 /// \brief The base class of all kinds of template declarations (e.g.,
 /// class, function, etc.).
 ///
 /// The TemplateDecl class stores the list of template parameters and a
 /// reference to the templated scoped declaration: the underlying AST node.
 class TemplateDecl : public NamedDecl {
   void anchor() override;
 protected:
   // This is probably never used.
   TemplateDecl(Kind DK, DeclContext *DC, SourceLocation L, DeclarationName Name)
       : NamedDecl(DK, DC, L, Name), TemplatedDecl(nullptr, false),
-        TemplateParams(nullptr) {}
+        TemplateParams(nullptr, false) {}
 
   // Construct a template decl with the given name and parameters.
   // Used when there is not templated element (tt-params).
   TemplateDecl(Kind DK, DeclContext *DC, SourceLocation L, DeclarationName Name,
                TemplateParameterList *Params)
       : NamedDecl(DK, DC, L, Name), TemplatedDecl(nullptr, false),
-        TemplateParams(Params) {}
+        TemplateParams(Params, false) {}
 
   // Construct a template decl with name, parameters, and templated element.
   TemplateDecl(Kind DK, DeclContext *DC, SourceLocation L, DeclarationName Name,
                TemplateParameterList *Params, NamedDecl *Decl)
       : NamedDecl(DK, DC, L, Name), TemplatedDecl(Decl, false),
-        TemplateParams(Params) {}
+        TemplateParams(Params, false) {}
 
 public:
   /// Get the list of template parameters
   TemplateParameterList *getTemplateParameters() const {
-    return TemplateParams;
+    return TemplateParams.getPointer();
   }
 
   /// Get the constraint-expression from the associated requires-clause (if any)
   const Expr *getRequiresClause() const {
-    return TemplateParams ? TemplateParams->getRequiresClause() : nullptr;
+    return TemplateParams.getPointer()
+               ? TemplateParams.getPointer()->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;
   }
 
   SourceRange getSourceRange() const override LLVM_READONLY {
-    return SourceRange(TemplateParams->getTemplateLoc(),
+    return SourceRange(TemplateParams.getPointer()->getTemplateLoc(),
                        TemplatedDecl.getPointer()->getSourceRange().getEnd());
   }
 
   /// Whether this is a (C++ Concepts TS) function or variable concept.
   bool isConcept() const { return TemplatedDecl.getInt(); }
   void setConcept() { TemplatedDecl.setInt(true); }
 
 protected:
   /// \brief The named declaration from which this template was instantiated.
   /// (or null).
   ///
   /// The boolean value will be true to indicate that this template
   /// (function or variable) is a concept.
   llvm::PointerIntPair<NamedDecl *, 1, bool> TemplatedDecl;
 
-  TemplateParameterList* TemplateParams;
+  /// \brief The template parameter list and optional requires-clause
+  /// associated with this declaration.
+  ///
+  /// The boolean value indicates whether this particular declaration has an
+  /// attached \c Expr representing the associated constraints of the template.
+  llvm::PointerIntPair<TemplateParameterList *, 1, bool> TemplateParams;
+
+  bool storesAssociatedConstraints() const { return TemplateParams.getInt(); }
+  void setStoresAssociatedConstraints() { TemplateParams.setInt(true); }
 
 public:
   /// \brief Initialize the underlying templated declaration and
   /// template parameters.
   void init(NamedDecl *templatedDecl, TemplateParameterList* templateParams) {
     assert(!TemplatedDecl.getPointer() && "TemplatedDecl already set!");
-    assert(!TemplateParams && "TemplateParams already set!");
+    assert(!TemplateParams.getPointer() && "TemplateParams already set!");
     TemplatedDecl.setPointer(templatedDecl);
-    TemplateParams = templateParams;
+    TemplateParams.setPointer(templateParams);
   }
 };
 
 /// \brief Provides information about a function template specialization,
 /// which is a FunctionDecl that has been explicitly specialization or
 /// instantiated from a function template.
 class FunctionTemplateSpecializationInfo : public llvm::FoldingSetNode {
   FunctionTemplateSpecializationInfo(FunctionDecl *FD,
static bool classofKind(Kind K) {
     return K == ClassTemplatePartialSpecialization;
   }
 
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
 };
 
 /// Declaration of a class template.
-class ClassTemplateDecl : public RedeclarableTemplateDecl {
+class ClassTemplateDecl final
+    : public RedeclarableTemplateDecl,
+      private llvm::TrailingObjects<ClassTemplateDecl, Expr *> {
+  friend TrailingObjects;
   static void DeallocateCommon(void *Ptr);
 
 protected:
   /// \brief Data that is common to all of the declarations of a given
   /// class template.
   struct Common : CommonBase {
     Common() : LazySpecializations() { }
 
ClassTemplateDecl(ASTContext &C, DeclContext *DC, SourceLocation L,
       : RedeclarableTemplateDecl(ClassTemplate, C, DC, L, Name, Params, Decl) {}
 
   CommonBase *newCommon(ASTContext &C) const override;
 
   Common *getCommonPtr() const {
     return static_cast<Common *>(RedeclarableTemplateDecl::getCommonPtr());
   }
 
+  Expr *&storedAssociatedConstraintsField() {
+    assert(storesAssociatedConstraints() && "Accessing non-existent field?");
+    return *getTrailingObjects<Expr *>();
+  }
+
+  Expr *storedAssociatedConstraintsField() const {
+    assert(storesAssociatedConstraints() && "Accessing non-existent field?");
+    return *getTrailingObjects<Expr *>();
+  }
+
 public:
   /// \brief Load any lazily-loaded specializations from the external source.
   void LoadLazySpecializations() const;
 
   /// \brief Get the underlying class declarations of the template.
   CXXRecordDecl *getTemplatedDecl() const {
     return static_cast<CXXRecordDecl *>(TemplatedDecl.getPointer());
   }
 
   /// \brief Returns whether this template declaration defines the primary
   /// class pattern.
   bool isThisDeclarationADefinition() const {
     return getTemplatedDecl()->isThisDeclarationADefinition();
   }
 
+  /// \brief Returns the associated constraints for this template (if any).
+  Expr *getAssociatedConstraints() const {
+    const ClassTemplateDecl *const C = ClassTemplateDecl::getCanonicalDecl();
+    return C->storesAssociatedConstraints()
+               ? C->storedAssociatedConstraintsField()
+               : nullptr;
+  }
+
+  // FIXME: remove default argument for AssociatedConstraints
   /// \brief Create a class template node.
   static ClassTemplateDecl *Create(ASTContext &C, DeclContext *DC,
                                    SourceLocation L,
                                    DeclarationName Name,
                                    TemplateParameterList *Params,
                                    NamedDecl *Decl,
-                                   ClassTemplateDecl *PrevDecl);
+                                   ClassTemplateDecl *PrevDecl,
+                                   Expr *AssociatedConstraints = nullptr);
 
   /// \brief Create an empty class template node.
   static ClassTemplateDecl *CreateDeserialized(ASTContext &C, unsigned ID);
 
   /// \brief Return the specialization with the provided arguments if it exists,
   /// otherwise return the insertion point.
   ClassTemplateSpecializationDecl *
   findSpecialization(ArrayRef<TemplateArgument> Args, void *&InsertPos);

include/clang/Basic/DiagnosticSemaKinds.td

def err_variable_concept_bool_decl : Error<
   "declared type of variable concept must be 'bool'">;
 def err_concept_specified_specialization : Error<
   "'concept' cannot be applied on an "
   "%select{explicit instantiation|explicit specialization|partial specialization}0">;
 def err_concept_specialized : Error<
   "%select{function|variable}0 concept cannot be "
   "%select{explicitly instantiated|explicitly specialized|partially specialized}1">;
 
+def err_template_different_associated_constraints : Error<
+  "associated constraints differ in template redeclaration">;
+
 // C++11 char16_t/char32_t
 def warn_cxx98_compat_unicode_type : Warning<
   "'%0' type specifier is incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
 
 // __make_integer_seq
 def err_integer_sequence_negative_length : Error<
   "integer sequences must have non-negative sequence length">;

lib/AST/DeclTemplate.cpp

 }
 
 ClassTemplateDecl *ClassTemplateDecl::Create(ASTContext &C,
                                              DeclContext *DC,
                                              SourceLocation L,
                                              DeclarationName Name,
                                              TemplateParameterList *Params,
                                              NamedDecl *Decl,
-                                             ClassTemplateDecl *PrevDecl) {
+                                             ClassTemplateDecl *PrevDecl,
+                                             Expr *AssociatedConstraints) {
   AdoptTemplateParameterList(Params, cast<DeclContext>(Decl));
-  ClassTemplateDecl *New = new (C, DC) ClassTemplateDecl(C, DC, L, Name,
-                                                         Params, Decl);
+  assert(!(AssociatedConstraints && PrevDecl) &&
+         "Attaching associated constraints to later (not first) declaration");
+  ClassTemplateDecl *New =
+      new (C, DC, ClassTemplateDecl::additionalSizeToAlloc<Expr *>(
+                      AssociatedConstraints ? 1u : 0u))
+          ClassTemplateDecl(C, DC, L, Name, Params, Decl);
   New->setPreviousDecl(PrevDecl);
+  if (AssociatedConstraints) {
+    New->setStoresAssociatedConstraints();
+    New->storedAssociatedConstraintsField() = AssociatedConstraints;
+  }
   return New;
 }
 
 ClassTemplateDecl *ClassTemplateDecl::CreateDeserialized(ASTContext &C,
                                                          unsigned ID) {
   return new (C, ID) ClassTemplateDecl(C, nullptr, SourceLocation(),
                                        DeclarationName(), nullptr, nullptr);
 }

lib/Sema/SemaTemplate.cpp

 // Exported for use by Parser.
 SourceRange
 clang::getTemplateParamsRange(TemplateParameterList const * const *Ps,
                               unsigned N) {
   if (!N) return SourceRange();
   return SourceRange(Ps[0]->getTemplateLoc(), Ps[N-1]->getRAngleLoc());
 }
 
+namespace clang {
+/// \brief [temp.constr.decl]p2: A template's associated constraints are
+/// defined as a single constraint-expression derived from the introduced
+/// constraint-expressions [ ... ].
+///
+/// \param Params The template parameter list and optional requires-clause.
+///
+/// \param FD The underlying templated function declaration for a function
+/// template.
+static Expr *formAssociatedConstraints(TemplateParameterList *Params,
+                                       FunctionDecl *FD);
+}
+
+static Expr *clang::formAssociatedConstraints(TemplateParameterList *Params,
+                                              FunctionDecl *FD) {
+  // FIXME: Concepts: collect additional introduced constraint-expressions
+  assert(!FD && "Cannot collect constraints from function declaration yet.");
+  return Params->getRequiresClause();
+}
+
 /// \brief Determine whether the declaration found is acceptable as the name
 /// of a template and, if so, return that template declaration. Otherwise,
 /// returns NULL.
 static NamedDecl *isAcceptableTemplateName(ASTContext &Context,
                                            NamedDecl *Orig,
                                            bool AllowFunctionTemplates) {
   NamedDecl *D = Orig->getUnderlyingDecl();
 
if (SS.isEmpty() &&
       Diag(Shadow->getTargetDecl()->getLocation(),
            diag::note_using_decl_target);
       Diag(Shadow->getUsingDecl()->getLocation(), diag::note_using_decl) << 0;
       // Recover by ignoring the old declaration.
       PrevDecl = PrevClassTemplate = nullptr;
     }
   }
 
+  // TODO Memory management; associated constraints are not always stored.
+  Expr *const CurAC = formAssociatedConstraints(TemplateParams, nullptr);
+
   if (PrevClassTemplate) {
     // Ensure that the template parameter lists are compatible. Skip this check
     // for a friend in a dependent context: the template parameter list itself
     // could be dependent.
     if (!(TUK == TUK_Friend && CurContext->isDependentContext()) &&
         !TemplateParameterListsAreEqual(TemplateParams,
                                    PrevClassTemplate->getTemplateParameters(),
                                         /*Complain=*/true,
                                         TPL_TemplateMatch))
       return true;
 
+    // Check for matching associated constraints on redeclarations.
+    do {
+      const Expr *const PrevAC = PrevClassTemplate->getAssociatedConstraints();
+      if (!(CurAC || PrevAC))
+        continue; // nothing to check
+      if (CurAC && PrevAC) {
+        llvm::FoldingSetNodeID CurACInfo, PrevACInfo;
+        CurAC->Profile(CurACInfo, Context, /*Canonical=*/true);
+        PrevAC->Profile(PrevACInfo, Context, /*Canonical=*/true);
+        if (CurACInfo == PrevACInfo)
+          continue; // all good
+      }
+
+      Diag(CurAC ? CurAC->getLocStart() : NameLoc,
+           diag::err_template_different_associated_constraints);
+      Diag(PrevAC ? PrevAC->getLocStart() : PrevClassTemplate->getLocation(),
+           diag::note_template_prev_declaration) << /*declaration*/0;
+      return true;
+    } while (0);
+
     // C++ [temp.class]p4:
     //   In a redeclaration, partial specialization, explicit
     //   specialization or explicit instantiation of a class template,
     //   the class-key shall agree in kind with the original class
     //   template declaration (7.1.5.3).
     RecordDecl *PrevRecordDecl = PrevClassTemplate->getTemplatedDecl();
     if (!isAcceptableTagRedeclaration(PrevRecordDecl, Kind,
                                       TUK == TUK_Definition,  KWLoc, Name)) {
Sema::CheckClassTemplate(Scope *S, unsigned TagSpec, TagUseKind TUK,
 
   // Add alignment attributes if necessary; these attributes are checked when
   // the ASTContext lays out the structure.
   if (TUK == TUK_Definition) {
     AddAlignmentAttributesForRecord(NewClass);
     AddMsStructLayoutForRecord(NewClass);
   }
 
   ClassTemplateDecl *NewTemplate
     = ClassTemplateDecl::Create(Context, SemanticContext, NameLoc,
                                 DeclarationName(Name), TemplateParams,
-                                NewClass, PrevClassTemplate);
+                                NewClass, PrevClassTemplate,
+                                PrevClassTemplate ? nullptr : CurAC);
   NewClass->setDescribedClassTemplate(NewTemplate);
   
   if (ModulePrivateLoc.isValid())
     NewTemplate->setModulePrivate();
   
   // Build the type for the class template declaration now.
   QualType T = NewTemplate->getInjectedClassNameSpecialization();
   T = Context.getInjectedClassNameType(NewClass, T);

test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/class-template-decl.cpp

+// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
+
+namespace nodiag {
+
+using MyBool = bool;
+
+template <typename T> requires !!sizeof(bool)
+struct A;
+template <typename T> requires !!sizeof(MyBool)
+struct A;
+
+} // end namespace nodiag
+
+namespace diag {
+
+template <typename T> requires true // expected-note{{previous template declaration is here}}
+struct A;
+template <typename T> struct A; // expected-error{{associated constraints differ in template redeclaration}}
+
+template <typename T> struct B; // expected-note{{previous template declaration is here}}
+template <typename T> requires true // expected-error{{associated constraints differ in template redeclaration}}
+struct B;
+
+template <typename T> requires true // expected-note{{previous template declaration is here}}
+struct C;
+template <typename T> requires !0 // expected-error{{associated constraints differ in template redeclaration}}
+struct C;
+
+} // end namespace diag