[PATCH] D40381: Parse concept definition

saar.raz updated this revision to Diff 209068. saar.raz added a comment. Final committed diff. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40381/new/ https://reviews.llvm.org/D40381 Files: include/clang/AST/ASTNodeTraverser.h include/clang/AST/DeclTemplate.h

[PATCH] D40381: Parse concept definition

saar.raz added a comment. @rsmith? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40381/new/ https://reviews.llvm.org/D40381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40381: Parse concept definition

saar.raz added a comment. Awesome! I do not have commit permissions though, so can you do the actual commit? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40381/new/ https://reviews.llvm.org/D40381 ___ cfe-commits

[PATCH] D40381: Parse concept definition

saar.raz updated this revision to Diff 198200. saar.raz marked 4 inline comments as done. saar.raz added a comment. - Address final CR comments by rsmith Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40381/new/ https://reviews.llvm.org/D40381 Files:

[PATCH] D40381: Parse concept definition

rsmith accepted this revision. rsmith added a comment. LGTM with a few mechanical updates. Comment at: include/clang/Basic/DiagnosticParseKinds.td:1262 + "name defined in concept definition must be an identifier">; +def err_concept_legacy_bool_keyword : ExtWarn< + "ISO C++2a

[PATCH] D40381: Parse concept definition

saar.raz added a comment. @rsmith are we done with CR on this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40381/new/ https://reviews.llvm.org/D40381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40381: Parse concept definition

saar.raz updated this revision to Diff 196009. saar.raz added a comment. Herald added a reviewer: martong. Address CR comments by rsmith Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40381/new/ https://reviews.llvm.org/D40381 Files:

[PATCH] D40381: Parse concept definition

rsmith added a comment. Thanks! Please revert the (presumably unintended) mode changes to the `ptxas` executables. Comment at: include/clang/AST/DeclTemplate.h:3035 + SourceRange getSourceRange() const override LLVM_READONLY { +return SourceRange(getLocation(),

[PATCH] D40381: Parse concept definition

saar.raz updated this revision to Diff 194417. saar.raz added a comment. Herald added a subscriber: jfb. Herald added a project: clang. Address most of rsmith's CR comments, rebase onto trunk Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40381/new/

[PATCH] D40381: Parse concept definition

saar.raz marked 21 inline comments as done. saar.raz added a comment. Only the TemplatedDecl issue remains, all other comments have been addressed and we're rebased onto master. @rsmith please reply to the last comment, and lets finally start merging these :) Repository: rC Clang CHANGES

[PATCH] D40381: Parse concept definition

saar.raz added inline comments. Herald added a subscriber: arphaman. Comment at: include/clang/AST/DeclTemplate.h:3035 + SourceRange getSourceRange() const override LLVM_READONLY { +return SourceRange(getLocation(), getLocation()); + } rsmith wrote: >

[PATCH] D40381: Parse concept definition

rsmith added inline comments. Comment at: include/clang/AST/DeclTemplate.h:3007 +/// \brief Definition of concept, not just declaration actually. +class ConceptDecl : public TemplateDecl { This comment isn't appropriate. Please just describe what the node is.

[PATCH] D40381: Parse concept definition

faisalv added a comment. I discussed this briefly w Hubert - and i'm planning on modifying this patch slightly so that it flows through ParseDeclSpecifier and handles attributes and other invalid decl-specifiers such as static etc. more gracefully on a concept decl. I have this partially

[PATCH] D40381: Parse concept definition

saar.raz added inline comments. Comment at: lib/Parse/ParseTemplate.cpp:383 + + if (!Tok.is(tok::identifier)) { +Diag(Tok.getLocation(), diag::err_expected) << tok::identifier; We could accept 'bool' here to be nice to people coming in from the old

[PATCH] D40381: Parse concept definition

saar.raz added inline comments. Comment at: lib/Parse/ParseTemplate.cpp:161 // Parse the actual template declaration. - return ParseSingleDeclarationAfterTemplate(Context, - ParsedTemplateInfo(, -

[PATCH] D40381: Parse concept definition

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 =

[PATCH] D40381: Parse concept definition

changyu updated this revision to Diff 128123. changyu marked 2 inline comments as done. https://reviews.llvm.org/D40381 Files: include/clang/AST/DeclTemplate.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/DeclNodes.td include/clang/Basic/DiagnosticParseKinds.td

[PATCH] D40381: Parse concept definition

faisalv added inline comments. Comment at: lib/Parse/ParseTemplate.cpp:181 +TemplateParameterList *TPL = ParamLists[0]; +if (TPL->getLAngleLoc().getLocWithOffset(1) == TPL->getRAngleLoc()) { + Diag(TPL->getTemplateLoc(), changyu wrote: > There's one

[PATCH] D40381: Parse concept definition

changyu added a comment. I don't have commit privilege. And also there's one more problem we might want to address first. Thank you. Comment at: lib/Parse/ParseTemplate.cpp:181 +TemplateParameterList *TPL = ParamLists[0]; +if (TPL->getLAngleLoc().getLocWithOffset(1)

[PATCH] D40381: Parse concept definition

faisalv added a comment. I think this looks good enough to commit - do you have commit privileges - or do you need one of us to commit it for you? thank you! https://reviews.llvm.org/D40381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40381: Parse concept definition

changyu updated this revision to Diff 128094. changyu marked an inline comment as done. https://reviews.llvm.org/D40381 Files: include/clang/AST/DeclTemplate.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/DeclNodes.td include/clang/Basic/DiagnosticParseKinds.td

[PATCH] D40381: Parse concept definition

changyu marked 27 inline comments as done. changyu added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:7713 + + if (!ConstraintExpr->isTypeDependent() && + ConstraintExpr->getType() != Context.BoolTy) { faisalv wrote: > saar.raz wrote: > > faisalv

[PATCH] D40381: Parse concept definition

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? > >

[PATCH] D40381: Parse concept definition

changyu marked 6 inline comments as done. changyu added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:7735 + ActOnDocumentableDecl(NewDecl); + CurContext->addDecl(NewDecl); + return NewDecl; faisalv wrote: > Why not use 'PushOnScopeChains' onto the

[PATCH] D40381: Parse concept definition

faisalv requested changes to this revision. faisalv added inline comments. This revision now requires changes to proceed. Comment at: lib/Sema/SemaTemplate.cpp:3899 + // constraint expressions right now. + return Template->getConstraintExpr(); +} saar.raz

[PATCH] D40381: Parse concept definition

saar.raz added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:3899 + // constraint expressions right now. + return Template->getConstraintExpr(); +} faisalv wrote: > I don't think we want to just return the constraint expr? I think we would > need to

[PATCH] D40381: Parse concept definition

faisalv added a comment. Thanks for working on this! :) Comment at: include/clang/AST/DeclTemplate.h:3035 + SourceRange getSourceRange() const override LLVM_READONLY { +return SourceRange(getLocation(), getLocation()); + } why not just fix it now?

[PATCH] D40381: Parse concept definition

saar.raz requested changes to this revision. saar.raz added a comment. This revision now requires changes to proceed. Do the requested clang-formatting as shown by hubert, other than that looks good to me :) https://reviews.llvm.org/D40381 ___

[PATCH] D40381: Parse concept definition

Rakete added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:7693 +Decl *Sema::ActOnConceptDefinition(Scope *S, + MultiTemplateParamsArg TemplateParameterLists, + IdentifierInfo *Name, SourceLocation L,

[PATCH] D40381: Parse concept definition

hubert.reinterpretcast added inline comments. Comment at: include/clang/Sema/Sema.h:6194 +SourceLocation TemplateLoc, +const TemplateArgumentListInfo *TemplateArgs); + changyu wrote: >

[PATCH] D40381: Parse concept definition

changyu added inline comments. Comment at: include/clang/Sema/Sema.h:6194 +SourceLocation TemplateLoc, +const TemplateArgumentListInfo *TemplateArgs); + hubert.reinterpretcast wrote: > Indentation

[PATCH] D40381: Parse concept definition

changyu planned changes to this revision. changyu marked 22 inline comments as done. changyu added inline comments. Comment at: lib/Parse/ParseTemplate.cpp:374 + + ExprResult ConstraintExpr = ParseConstraintExpression(); + saar.raz wrote: > Add a check to

[PATCH] D40381: Parse concept definition

hubert.reinterpretcast added inline comments. Comment at: include/clang/Sema/Sema.h:6194 +SourceLocation TemplateLoc, +const TemplateArgumentListInfo *TemplateArgs); + Indentation issue here too.

[PATCH] D40381: Parse concept definition

saar.raz added inline comments. Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:1 +// RUN: %clang_cc1 -std=c++1z -fconcepts-ts -fcxx-exceptions -x c++ -verify %s +// expected-no-diagnostics hubert.reinterpretcast wrote: > saar.raz wrote: > >

[PATCH] D40381: Parse concept definition

hubert.reinterpretcast added inline comments. Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:1 +// RUN: %clang_cc1 -std=c++1z -fconcepts-ts -fcxx-exceptions -x c++ -verify %s +// expected-no-diagnostics saar.raz wrote: > Rakete wrote: >

[PATCH] D40381: Parse concept definition

saar.raz added inline comments. Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:1 +// RUN: %clang_cc1 -std=c++1z -fconcepts-ts -fcxx-exceptions -x c++ -verify %s +// expected-no-diagnostics Rakete wrote: > There is no `-fconcepts-ts`

[PATCH] D40381: Parse concept definition

Rakete added inline comments. Comment at: lib/Parse/ParseTemplate.cpp:161 // Parse the actual template declaration. - return ParseSingleDeclarationAfterTemplate(Context, - ParsedTemplateInfo(, -

[PATCH] D40381: Parse concept definition

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

[PATCH] D40381: Parse concept definition

changyu added a comment. I should add that this depends on https://reviews.llvm.org/D40380 https://reviews.llvm.org/D40381 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40381: Parse concept definition

changyu created this revision. Per p0734r0. https://reviews.llvm.org/D40381 Files: include/clang/AST/DeclTemplate.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/DeclNodes.td include/clang/Basic/TemplateKinds.h include/clang/Parse/Parser.h include/clang/Sema/Sema.h