On Fri, Jul 10, 2015 at 8:30 AM, Aaron Ballman <aa...@aaronballman.com> wrote:
> Tiny nits, otherwise LGTM, but please wait for Richard to also sign off. > Same from my end. > > On Fri, Jul 10, 2015 at 12:21 AM, Nathan Wilson <nwilso...@gmail.com> > wrote: > > nwilson updated this revision to Diff 29426. > > nwilson added a comment. > > > > Update phrasing for concept declaration scope diagnostic > > Fix indentation issue > > Update comments for function declaration being a definition > > Adding acceptance tests for concepts in namespace scope and adding > diagnostic test for variable concept not being in namespace scope > > > > > > http://reviews.llvm.org/D11027 > > > > Files: > > include/clang/Basic/DiagnosticSemaKinds.td > > lib/Sema/SemaDecl.cpp > > test/SemaCXX/cxx-concept-declaration.cpp > > > > Index: test/SemaCXX/cxx-concept-declaration.cpp > > =================================================================== > > --- /dev/null > > +++ test/SemaCXX/cxx-concept-declaration.cpp > > @@ -0,0 +1,17 @@ > > +// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s > > + > > +namespace A { > > + template<typename T> concept bool C1() { return true; } > > + > > + template<typename T> concept bool C2 = true; > > +} > > + > > +template<typename T> concept bool D1(); // expected-error {{can only > declare a function concept with its definition}} > > + > > +struct B { > > + template<typename T> concept bool D2() { return true; } // > expected-error {{concept declarations may only appear in namespace scope}} > > +}; > > + > > +struct C { > > + template<typename T> static concept bool D3 = true; // expected-error > {{concept declarations may only appear in namespace scope}} > > +}; > > Index: lib/Sema/SemaDecl.cpp > > =================================================================== > > --- lib/Sema/SemaDecl.cpp > > +++ lib/Sema/SemaDecl.cpp > > @@ -4878,6 +4878,17 @@ > > if (getLangOpts().CPlusPlus) > > CheckExtraCXXDefaultArguments(D); > > > > + if (D.getDeclSpec().isConceptSpecified()) { > > + // C++ Concepts TS [dcl.spec.concept]p1: The concept specifier > shall be > > + // applied only to the definition of a function template or variable > > + // template, declared in namespace scope > > Missing a period. > > > + if (!DC->getRedeclContext()->isFileContext()) { > > + Diag(D.getIdentifierLoc(), > > + diag::err_concept_decls_may_only_appear_in_namespace_scope); > > + return nullptr; > > + } > > + } > > + > > NamedDecl *New; > > > > bool AddToScope = true; > > @@ -7223,6 +7234,7 @@ > > bool isVirtual = D.getDeclSpec().isVirtualSpecified(); > > bool isExplicit = D.getDeclSpec().isExplicitSpecified(); > > bool isConstexpr = D.getDeclSpec().isConstexprSpecified(); > > + bool isConcept = D.getDeclSpec().isConceptSpecified(); > > isFriend = D.getDeclSpec().isFriendSpecified(); > > if (isFriend && !isInline && D.isFunctionDefinition()) { > > // C++ [class.friend]p5 > > @@ -7439,6 +7451,21 @@ > > Diag(D.getDeclSpec().getConstexprSpecLoc(), > diag::err_constexpr_dtor); > > } > > > > + if (isConcept) { > > + // C++ Concepts TS [dcl.spec.concept]p1: The concept specifier > shall be > > + // applied only to the definition of a function template > > + // (we are checking that the declaration is indeed a definition) > > Missing a period. > I think ellipsis is appropriate. Also, the parenthesized part is redundant now. > > > + if (!D.isFunctionDefinition()) { > > + Diag(D.getDeclSpec().getConceptSpecLoc(), > > + diag::err_function_concept_not_defined); > > + NewFD->setInvalidDecl(); > > + } > > + > > + // C++ Concepts TS [dcl.spec.concept]p2: Every concept definition > is > > + // implicity defined to be a constexpr declaration (implicitly > inline) > > + NewFD->setImplicitlyInline(); > > + } > > + > > // If __module_private__ was specified, mark the function > accordingly. > > if (D.getDeclSpec().isModulePrivateSpecified()) { > > if (isFunctionTemplateSpecialization) { > > Index: include/clang/Basic/DiagnosticSemaKinds.td > > =================================================================== > > --- include/clang/Basic/DiagnosticSemaKinds.td > > +++ include/clang/Basic/DiagnosticSemaKinds.td > > @@ -1959,6 +1959,12 @@ > > def note_private_extern : Note< > > "use __attribute__((visibility(\"hidden\"))) attribute instead">; > > > > +// C++ Concepts TS > > +def err_concept_decls_may_only_appear_in_namespace_scope : Error< > > + "concept declarations may only appear in namespace scope">; > > +def err_function_concept_not_defined : Error< > > + "can only declare a function concept with its definition">; > > This reads slightly strange to me. We have another diagnostic that we > could steal wording from: > > def err_invalid_constexpr_var_decl : Error< > "constexpr variable declaration must be a definition">; > > So perhaps "function concept declaration must be a definition"? > That sounds good. > > > + > > // C++11 char16_t/char32_t > > def warn_cxx98_compat_unicode_type : Warning< > > "'%0' type specifier is incompatible with C++98">, > > ~Aaron > > > > > > > > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
