[PATCH] D40381: Parse concept definition

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: include/clang/Sema/Sema.h:6194
+                                SourceLocation TemplateLoc,
+                                const TemplateArgumentListInfo *TemplateArgs);
+
----------------
Indentation issue here too.


================
Comment at: lib/Parse/ParseTemplate.cpp:57
 ///
+///       template-declaration: [Concepts TS]
+///         template-head declaration
----------------
C++2a Concepts


================
Comment at: lib/Parse/ParseTemplate.cpp:374
+
+  ExprResult ConstraintExpr = ParseConstraintExpression();
+
----------------
saar.raz wrote:
> 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;
>     }
> ```
I think that would still need a TODO to instead walk the constraint expression for atomic constraints and diagnose those.
```
template <typename T>
concept C = 1 || T::value; // error
```


================
Comment at: lib/Sema/SemaTemplate.cpp:3903
+  //                                 /*FoundD=*/nullptr, TemplateArgs);
+  return Template->getConstraintExpr();
+  return true;
----------------
Add more comments here. It looks like this allows us to get id-expressions naming concepts defined with non-dependent `bool` constraint expressions to "work" for now?


================
Comment at: test/Parser/cxx-concept-declaration.cpp:5
 // RUN:  %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s
-// template<typename T> concept C1 = true;
+template<typename T> concept C1 = true;
+
----------------
Should add tests to prevent redeclaring concept names as either a "normal" (e.g., variable) or tag name.
Also the reverse for redeclaring a tag name as a concept.

Should add tests to prevent multiple definitions of the same concept.
Should eventually add tests to prevent explicit specialization, partial specialization and explicit instantiation of concepts: while the restriction is syntactic in the wording, that does not necessarily translate directly to the implementation strategy. In any case, we may discover that we want a nicer message.

Should add tests to prevent defining concepts at class scope.



https://reviews.llvm.org/D40381