[PATCH] D40381: Parse concept definition

[changyu via Phabricator via cfe-commits]

changyu added a comment.

I moved some template param checks from `Parser::ParseTemplateDeclarationOrSpecialization` to `Parser::ParseConceptDefinition` and `Sema::ActOnConceptDefinition`.



================
Comment at: lib/Parse/ParseTemplate.cpp:181
+    TemplateParameterList *TPL = ParamLists[0];
+    if (TPL->getLAngleLoc().getLocWithOffset(1) == TPL->getRAngleLoc()) {
+      Diag(TPL->getTemplateLoc(),
----------------
faisalv wrote:
> changyu wrote:
> > There's one problem here.
> > 
> > ​I added this `if` in attempt to catch the following case (but it's wrong)
> > ```
> > 	​template<> concept D1 = true;  // expected-error {{expected template parameter}}
> > ```
> > The problem is I'm not sure how to differentiate between the above situation and the following
> > ```
> > 	​template<concept T> concept D1 = true; // expected-error {{expected template parameter}}
> > ```
> > Both have an empty template parameter list​. The latter case has diagnostic printed by `ParseNonTypeTemplateParameter` while the former has not (so we try to catch it here).
> > 
> > What should we do?
> > 
> 
> I was thinking that we would just emit a (redundant in the case of a bad template parameter) message in Sema if the template-parameters are empty that explicit specializations are not allowed here.  while it would be a little misleading in the invalid template parameter case - to fix this robustly would require some fine-tuning and correcting some of the handshaking/error-propagation between the parsing of the template parameters and the code that calls it, I think.  I would vote for not holding up this patch for that, unless you feel strongly you'd like to fix that behavior - then we can try and work on that first?
> 
> Thoughts?
> 
> 
Sure, let's fix that in another patch. I added a note for this in cxx2a-concept-declaration.cpp.


https://reviews.llvm.org/D40381