On Fri, Jul 10, 2015 at 11:01 AM, Nathan Wilson <[email protected]> wrote: > > > On Fri, Jul 10, 2015 at 9:44 AM, Aaron Ballman <[email protected]> > wrote: >> >> On Fri, Jul 10, 2015 at 10:34 AM, Nathan Wilson <[email protected]> >> wrote: >> > >> > >> > On Fri, Jul 10, 2015 at 8:52 AM, Hubert Tong >> > <[email protected]> wrote: >> >> >> >> On Fri, Jul 10, 2015 at 8:30 AM, Aaron Ballman <[email protected]> >> >> 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 <[email protected]> >> >>> 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. >> > >> > >> > Yeah, I was attempting to be explicit, but I guess the code does that. I >> > can >> > make the change. >> >> >> >> >> >>> >> >>> >> >>> > + 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. >> > >> > >> > Okay, that works. Do you guys have any issue with the name: >> > err_function_concept_not_defined >> > >> > Or, would something like this be more appropriate?: >> > err_invalid_concept_function_decl >> >> I prefer err_invalid_concept_function_decl. > > > Hmm, well, I'm leaning to what Hubert said about it being a bit too generic > since there are other places where a name like this would be applicable > (declared returning type not being bool, non empty parameter list, ...)
I can swing around to that viewpoint. :-) ~Aaron > >> >> ~Aaron >> >> > >> >> >> >> >> >>> >> >>> >> >>> > + >> >>> > // 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 >> >>> > [email protected] >> >>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >>> > >> >> >> >> >> > > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
