[PATCH] D11027: [CONCEPTS] Creating Diagnostics for ill-formed function concept declaration
Nathan Wilson
nwilson20 at gmail.com
Fri Jul 10 08:01:07 PDT 2015
On Fri, Jul 10, 2015 at 9:44 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:
> On Fri, Jul 10, 2015 at 10:34 AM, Nathan Wilson <nwilson20 at gmail.com>
> wrote:
> >
> >
> > On Fri, Jul 10, 2015 at 8:52 AM, Hubert Tong
> > <hubert.reinterpretcast at gmail.com> wrote:
> >>
> >> On Fri, Jul 10, 2015 at 8:30 AM, Aaron Ballman <aaron at 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 <nwilson20 at 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.
> >
> >
> > 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, ...)
> ~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
> >>> > cfe-commits at cs.uiuc.edu
> >>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >>> >
> >>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150710/885a3725/attachment.html>
More information about the cfe-commits
mailing list