[PATCH] D40381: Parse concept definition

[Saar Raz via Phabricator via cfe-commits]

saar.raz requested changes to this revision.
saar.raz added a comment.
This revision now requires changes to proceed.

Also add:
In ASTDumper

  +    void VisitConceptTemplateDecl(const ConceptTemplateDecl *D);

Implementation:

  void ASTDumper::VisitConceptTemplateDecl(const ConceptTemplateDecl *D) {
    dumpName(D);
    dumpTemplateParameters(D->getTemplateParameters());
    VisitExpr(D->getConstraintExpression());
  }

Address the other issues I've mentioned in the inline comments and we're good! Great job :)



================
Comment at: include/clang/AST/DeclTemplate.h:3023
+                             Expr *ConstraintExpr);
+
+  Expr *getConstraintExpr() const {
----------------
Add CreateDeserialized.


================
Comment at: include/clang/AST/DeclTemplate.h:3027
+  }
+
+  // TODO: Should do source range properly.
----------------
Add setConstraintExpr (for use when calling CreateDeserialized, see ASTDeclReader)


================
Comment at: include/clang/Parse/Parser.h:2787
                                    AccessSpecifier AS = AS_none);
+  // C++ 17: Template, concept definition [temp]
+  Decl *
----------------
C++2a


================
Comment at: lib/Parse/ParseTemplate.cpp:374
+
+  ExprResult ConstraintExpr = ParseConstraintExpression();
+
----------------
Add a check to ParseConstraintExpression that the type is either dependent or bool, and add an apropriate diagnostic.

```
    if (!ConstraintExpr->isTypeDependent() && ConstraintExpr->getType() != Context.BoolTy) {
      Diag(Init->getSourceRange().getBegin(),
           diag::err_concept_initialized_with_non_bool_type) << Init->getType();
      Concept->setInvalidDecl();
      return;
    }
```


================
Comment at: lib/Sema/SemaTemplate.cpp:2935
   if (!Template || isa<FunctionTemplateDecl>(Template) ||
       isa<VarTemplateDecl>(Template)) {
     // We might have a substituted template template parameter pack. If so,
----------------
Add

```
 || isa<ConceptDecl>

```


================
Comment at: lib/Sema/SemaTemplate.cpp:3936
 
+  if (R.getAsSingle<ConceptDecl>()) {
+    return CheckConceptTemplateId(SS, R.getLookupNameInfo(),
----------------
We're gonna want to check
```
!TemplateSpecializationType::anyDependentTemplateArguments(*TemplateArgs, InstantiationDependent)
``` 
here as well - so that we can instantiate a ConceptSpecializationExpr later when we have it (we're not gonna want to instantiate a ConceptSpecializationExpr with dependent arguments.


================
Comment at: lib/Sema/SemaTemplate.cpp:7698
+
+  // TODO: Maybe we should check TemplateParameterLists for nullptr?
+  Decl *NewDecl = ConceptDecl::Create(Context, DC, L, Name,
----------------
Check that !DC->isFileContext() (Concepts may only be defined at namespace scope)


================
Comment at: lib/Sema/SemaTemplate.cpp:7700
+  Decl *NewDecl = ConceptDecl::Create(Context, DC, L, Name,
+                                      TemplateParameterLists[0],
+                                      ConstraintExpr);
----------------
I think it would be better to just check that TemplateParameterLists.size() != 1 - it can't happen in a valid situation anyway, you'd want a diagnostic there.


================
Comment at: lib/Sema/SemaTemplate.cpp:7702
+                                      ConstraintExpr);
+  ActOnDocumentableDecl(NewDecl);
+  CurContext->addDecl(NewDecl);
----------------
Check that c->getAssociatedConstraints() == nullptr ([temp.concept]p4 A concept shall not have associated constraints).
Add a TODO to make a test for that once we have actual associated constraints.


================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3099
+Decl *TemplateDeclInstantiator::VisitConceptDecl(ConceptDecl *D) {
+  // TODO: Do something here?
+  return nullptr;
----------------
Yes - 

```
llvm_unreachable("Concept definitions cannot reside inside a template");
```


================
Comment at: lib/Serialization/ASTReaderDecl.cpp:1975
+void ASTDeclReader::VisitConceptDecl(ConceptDecl *D) {
+  VisitTemplateDecl(D);
+}
----------------
Add here:

```
  D->setConstraintExpression(Record.readExpr());
```
And in ASTDeclWriter, a method VisitConceptDecl:
```
void ASTDeclWriter::VisitConceptTemplateDecl(ConceptTemplateDecl *D) {
  VisitTemplateDecl(D);
  Record.AddStmt(D->getConstraintExpression());
  Code = serialization::DECL_CONCEPT;
}
```
And call it in the appropriate places.


================
Comment at: lib/Serialization/ASTReaderDecl.cpp:3503
     break;
   case DECL_CLASS_SCOPE_FUNCTION_SPECIALIZATION:
     D = ClassScopeFunctionSpecializationDecl::CreateDeserialized(Context, ID);
----------------
Add a case: DECL_CONCEPT, also create that enum member.


https://reviews.llvm.org/D40381