faisalv added inline comments.

================
Comment at: lib/Sema/SemaTemplate.cpp:7735
+  ActOnDocumentableDecl(NewDecl);
+  CurContext->addDecl(NewDecl);
+  return NewDecl;
----------------
changyu wrote:
> faisalv wrote:
> > Why not use 'PushOnScopeChains' onto the enclosing scope?
> > Something along these lines:
> >    assert(S->isTemplateParamScope() && S->getParent() && 
> >     !S->getParent()->isTemplateParamScope() && "...");
> >    PushOnScopeChains(NewDecl, S->getParent(),/*AddToContext*/true);
> The condition
> ```
> S->isTemplateParamScope()
> ```
> fails in this case
> ```
> template<concept T> concept D1 = true; // expected-error {{expected template 
> parameter}}
> ```
> 
> `ParseTemplateDeclarationOrSpecialization` calls `ParseTemplateParameters` 
> which eventually calls `ParseNonTypeTemplateParameter`. 
> `ParseNonTypeTemplateParameter` prints the diag and fails but does not cause 
> `ParseTemplateParameters` to fail (I'm not sure why).  
> `ParseTemplateDeclarationOrSpecialization` proceeds to parse the concept 
> definition, and we get this
> 
> ```
> /home/changyu/test.cpp:1:10: error: expected template parameter
> template<concept T> concept D1 = true; // expected-error {{expected template 
> parameter}}
>          ^
> clang: /home/changyu/git/llvm/tools/clang/lib/Sema/SemaTemplate.cpp:7747: 
> clang::Decl* clang::Sema::ActOnConceptDefinition(clang::Scope*, 
> clang::MultiTemplateParamsArg, clang::IdentifierInfo*, clang::SourceLocation, 
> clang::Expr*): Assertion `S->isTemplateParamScope() && "Not in template param 
> scope?"' failed.
> ```
> 
> What should we do?
I think this occurs because our template parameter list is incorrectly 
perceived as empty - i.e. once the error occurs - to continue processing errors 
clang assumes this case is an explicit-specialization and replaces the template 
parameter scope with the outer scope.  I think the thing to do here - which 
might also address the case where folks actually write 'template<> concept' is 
to actually check if template-parameter-list is empty - and emit a diagnostic 
about concepts can not be explicit specializations or some such ...

On a somewhat related note - i think the logic for signaling, handling and 
propagating failures of parsing the template parameter list might be a little 
broken (fixing which, might avoid triggering the assertion in template<concept> 
but not template<>).  I might submit a patch to fix that error handling-issue 
separately - but either way I think we should handle the 
explicit-specialization error?

Thoughts?


https://reviews.llvm.org/D40381



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to