* [PATCH] C++: show location of problematic extern "C" specifications @ 2017-09-26 19:27 David Malcolm 2017-10-11 17:37 ` PING " David Malcolm 2017-10-11 20:50 ` Jason Merrill 0 siblings, 2 replies; 9+ messages in thread From: David Malcolm @ 2017-09-26 19:27 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm There are a few places where the C++ FE will complain when attempting to do things within an extern "C" linkage specifier. I've run into problems where it wasn't clear where the pertinent extern "C" was; for example, when failing to close an extern "C" linkage specifier in a header, leading to "template with C linkage" errors in a different source file. As of r251026 there will be a message highlighting the unclosed '{', but this may be hard to spot at the very end of the errors. This patch adds a note to the various diagnostics that complain about C linkage, showing the user where the extern "C" specification began. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. OK for trunk? gcc/cp/ChangeLog: * cp-tree.h (struct saved_scope): Add "location" field. (maybe_show_extern_c_location): New decl. * decl.c (grokfndecl): When complaining about literal operators with C linkage, issue a note giving the location of the extern "C". * parser.c (cp_parser_linkage_specification): Store the location of the "extern" token within the scope_chain. (maybe_show_extern_c_location): New function. (cp_parser_explicit_specialization): When complaining about template specializations with C linkage, issue a note giving the location of the extern "C". (cp_parser_explicit_template_declaration): Likewise for templates. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/udlit-extern-c.C: New test case. * g++.dg/diagnostic/unclosed-extern-c.C: Add example of a template erroneously covered by an unclosed extern "C". * g++.dg/template/extern-c.C: New test case. --- gcc/cp/cp-tree.h | 3 ++ gcc/cp/decl.c | 1 + gcc/cp/parser.c | 19 ++++++++++- gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C | 7 ++++ .../g++.dg/diagnostic/unclosed-extern-c.C | 11 +++++- gcc/testsuite/g++.dg/template/extern-c.C | 39 ++++++++++++++++++++++ 6 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C create mode 100644 gcc/testsuite/g++.dg/template/extern-c.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index e508598..762cc7b 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1568,6 +1568,8 @@ struct GTY(()) saved_scope { hash_map<tree, tree> *GTY((skip)) x_local_specializations; struct saved_scope *prev; + + location_t location; }; extern GTY(()) struct saved_scope *scope_chain; @@ -6352,6 +6354,7 @@ extern bool parsing_nsdmi (void); extern bool parsing_default_capturing_generic_lambda_in_template (void); extern void inject_this_parameter (tree, cp_cv_quals); extern location_t defarg_location (tree); +extern void maybe_show_extern_c_location (void); /* in pt.c */ extern bool check_template_shadow (tree); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 50fa1ba..d08ac9a 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -8724,6 +8724,7 @@ grokfndecl (tree ctype, if (DECL_LANGUAGE (decl) == lang_c) { error ("literal operator with C linkage"); + maybe_show_extern_c_location (); return NULL_TREE; } diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index d831d66..b90f40d 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -13823,7 +13823,8 @@ cp_parser_linkage_specification (cp_parser* parser) tree linkage; /* Look for the `extern' keyword. */ - cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN); + cp_token *extern_token + = cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN); /* Look for the string-literal. */ linkage = cp_parser_string_literal (parser, false, false); @@ -13843,6 +13844,7 @@ cp_parser_linkage_specification (cp_parser* parser) /* We're now using the new linkage. */ push_lang_context (linkage); + scope_chain->location = extern_token->location; /* If the next token is a `{', then we're using the first production. */ @@ -16589,6 +16591,19 @@ cp_parser_explicit_instantiation (cp_parser* parser) timevar_pop (TV_TEMPLATE_INST); } +/* Helper function for diagnostics that have complained about things + being used with 'extern "C"' linkage. + + Attempt to issue a note showing where the 'extern "C"' linkage began. */ + +void +maybe_show_extern_c_location (void) +{ + if (scope_chain->location != UNKNOWN_LOCATION) + inform (scope_chain->location, "%<extern \"C\"%> linkage started here"); +} + + /* Parse an explicit-specialization. explicit-specialization: @@ -16623,6 +16638,7 @@ cp_parser_explicit_specialization (cp_parser* parser) if (current_lang_name == lang_name_c) { error_at (token->location, "template specialization with C linkage"); + maybe_show_extern_c_location (); /* Give it C++ linkage to avoid confusing other parts of the front end. */ push_lang_context (lang_name_cplusplus); @@ -26858,6 +26874,7 @@ cp_parser_explicit_template_declaration (cp_parser* parser, bool member_p) if (current_lang_name == lang_name_c) { error_at (location, "template with C linkage"); + maybe_show_extern_c_location (); /* Give it C++ linkage to avoid confusing other parts of the front end. */ push_lang_context (lang_name_cplusplus); diff --git a/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C b/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C new file mode 100644 index 0000000..d47a49c --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C @@ -0,0 +1,7 @@ +// { dg-do compile { target c++11 } } + +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" } + +constexpr double operator"" _deg ( double degrees ); // { dg-error "literal operator with C linkage" } + +} diff --git a/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C b/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C index fda3532..44f538e 100644 --- a/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C +++ b/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C @@ -1,3 +1,12 @@ -extern "C" { /* { dg-message "12: to match this '.'" } */ +extern "C" { // { dg-line open_extern_c } + + int foo (void); + +/* Missing close-brace for the extern "C" here. */ + +template <typename T> // { dg-error "template with C linkage" } +void bar (void); +// { dg-message "1: 'extern .C.' linkage started here" "" { target *-*-* } open_extern_c } void test (void); /* { dg-error "17: expected '.' at end of input" } */ +// { message "12: to match this '.'" "" { target *-*-* } open_extern_c } diff --git a/gcc/testsuite/g++.dg/template/extern-c.C b/gcc/testsuite/g++.dg/template/extern-c.C new file mode 100644 index 0000000..8a3a77a --- /dev/null +++ b/gcc/testsuite/g++.dg/template/extern-c.C @@ -0,0 +1,39 @@ +template <typename T> void specializable (T); + +/* Invalid template: within "extern C". */ + +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" } + +template <typename T> // { dg-error "template with C linkage" } +void within_extern_c_braces (void); + +} + +/* Valid template: not within "extern C". */ + +template <typename T> +void not_within_extern_c (void); + + +/* Invalid specialization: within "extern C". */ + +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" } + +template <> // { dg-error "template specialization with C linkage" } +void specializable (int); + +} + + +/* Valid specialization: not within "extern C". */ +template <> +void specializable (char); + + +/* Example of extern C without braces. */ + +extern "C" template <typename T> // { dg-line open_extern_c_no_braces } +void within_extern_c_no_braces (void); +// { dg-error "12: template with C linkage" "" { target *-*-* } open_extern_c_no_braces } +// { dg-message "1: 'extern .C.' linkage started here" "" { target *-*-* } open_extern_c_no_braces } + -- 1.8.5.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* PING Re: [PATCH] C++: show location of problematic extern "C" specifications 2017-09-26 19:27 [PATCH] C++: show location of problematic extern "C" specifications David Malcolm @ 2017-10-11 17:37 ` David Malcolm 2017-10-11 20:50 ` Jason Merrill 1 sibling, 0 replies; 9+ messages in thread From: David Malcolm @ 2017-10-11 17:37 UTC (permalink / raw) To: gcc-patches Ping On Tue, 2017-09-26 at 15:27 -0400, David Malcolm wrote: > There are a few places where the C++ FE will complain when attempting > to do things within an extern "C" linkage specifier. > > I've run into problems where it wasn't clear where the pertinent > extern "C" was; for example, when failing to close an extern "C" > linkage > specifier in a header, leading to "template with C linkage" errors in > a different source file. > > As of r251026 there will be a message highlighting the unclosed '{', > but > this may be hard to spot at the very end of the errors. > > This patch adds a note to the various diagnostics that complain > about C linkage, showing the user where the extern "C" specification > began. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > OK for trunk? > > gcc/cp/ChangeLog: > * cp-tree.h (struct saved_scope): Add "location" field. > (maybe_show_extern_c_location): New decl. > * decl.c (grokfndecl): When complaining about literal operators > with C linkage, issue a note giving the location of the > extern "C". > * parser.c (cp_parser_linkage_specification): Store the > location > of the "extern" token within the scope_chain. > (maybe_show_extern_c_location): New function. > (cp_parser_explicit_specialization): When complaining about > template specializations with C linkage, issue a note giving > the > location of the extern "C". > (cp_parser_explicit_template_declaration): Likewise for > templates. > > gcc/testsuite/ChangeLog: > * g++.dg/cpp0x/udlit-extern-c.C: New test case. > * g++.dg/diagnostic/unclosed-extern-c.C: Add example of a > template > erroneously covered by an unclosed extern "C". > * g++.dg/template/extern-c.C: New test case. > --- > gcc/cp/cp-tree.h | 3 ++ > gcc/cp/decl.c | 1 + > gcc/cp/parser.c | 19 ++++++++++- > gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C | 7 ++++ > .../g++.dg/diagnostic/unclosed-extern-c.C | 11 +++++- > gcc/testsuite/g++.dg/template/extern-c.C | 39 > ++++++++++++++++++++++ > 6 files changed, 78 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C > create mode 100644 gcc/testsuite/g++.dg/template/extern-c.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index e508598..762cc7b 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -1568,6 +1568,8 @@ struct GTY(()) saved_scope { > hash_map<tree, tree> *GTY((skip)) x_local_specializations; > > struct saved_scope *prev; > + > + location_t location; > }; > > extern GTY(()) struct saved_scope *scope_chain; > @@ -6352,6 +6354,7 @@ extern bool parsing_nsdmi (void); > extern bool parsing_default_capturing_generic_lambda_in_template > (void); > extern void inject_this_parameter (tree, cp_cv_quals); > extern location_t defarg_location (tree); > +extern void maybe_show_extern_c_location (void); > > /* in pt.c */ > extern bool check_template_shadow (tree); > diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c > index 50fa1ba..d08ac9a 100644 > --- a/gcc/cp/decl.c > +++ b/gcc/cp/decl.c > @@ -8724,6 +8724,7 @@ grokfndecl (tree ctype, > if (DECL_LANGUAGE (decl) == lang_c) > { > error ("literal operator with C linkage"); > + maybe_show_extern_c_location (); > return NULL_TREE; > } > > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c > index d831d66..b90f40d 100644 > --- a/gcc/cp/parser.c > +++ b/gcc/cp/parser.c > @@ -13823,7 +13823,8 @@ cp_parser_linkage_specification (cp_parser* > parser) > tree linkage; > > /* Look for the `extern' keyword. */ > - cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN); > + cp_token *extern_token > + = cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN); > > /* Look for the string-literal. */ > linkage = cp_parser_string_literal (parser, false, false); > @@ -13843,6 +13844,7 @@ cp_parser_linkage_specification (cp_parser* > parser) > > /* We're now using the new linkage. */ > push_lang_context (linkage); > + scope_chain->location = extern_token->location; > > /* If the next token is a `{', then we're using the first > production. */ > @@ -16589,6 +16591,19 @@ cp_parser_explicit_instantiation (cp_parser* > parser) > timevar_pop (TV_TEMPLATE_INST); > } > > +/* Helper function for diagnostics that have complained about things > + being used with 'extern "C"' linkage. > + > + Attempt to issue a note showing where the 'extern "C"' linkage > began. */ > + > +void > +maybe_show_extern_c_location (void) > +{ > + if (scope_chain->location != UNKNOWN_LOCATION) > + inform (scope_chain->location, "%<extern \"C\"%> linkage started > here"); > +} > + > + > /* Parse an explicit-specialization. > > explicit-specialization: > @@ -16623,6 +16638,7 @@ cp_parser_explicit_specialization (cp_parser* > parser) > if (current_lang_name == lang_name_c) > { > error_at (token->location, "template specialization with C > linkage"); > + maybe_show_extern_c_location (); > /* Give it C++ linkage to avoid confusing other parts of the > front end. */ > push_lang_context (lang_name_cplusplus); > @@ -26858,6 +26874,7 @@ cp_parser_explicit_template_declaration > (cp_parser* parser, bool member_p) > if (current_lang_name == lang_name_c) > { > error_at (location, "template with C linkage"); > + maybe_show_extern_c_location (); > /* Give it C++ linkage to avoid confusing other parts of the > front end. */ > push_lang_context (lang_name_cplusplus); > diff --git a/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C > b/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C > new file mode 100644 > index 0000000..d47a49c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C > @@ -0,0 +1,7 @@ > +// { dg-do compile { target c++11 } } > + > +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" > } > + > +constexpr double operator"" _deg ( double degrees ); // { dg-error > "literal operator with C linkage" } > + > +} > diff --git a/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C > b/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C > index fda3532..44f538e 100644 > --- a/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C > +++ b/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C > @@ -1,3 +1,12 @@ > -extern "C" { /* { dg-message "12: to match this '.'" } */ > +extern "C" { // { dg-line open_extern_c } > + > + int foo (void); > + > +/* Missing close-brace for the extern "C" here. */ > + > +template <typename T> // { dg-error "template with C linkage" } > +void bar (void); > +// { dg-message "1: 'extern .C.' linkage started here" "" { target > *-*-* } open_extern_c } > > void test (void); /* { dg-error "17: expected '.' at end of input" } > */ > +// { message "12: to match this '.'" "" { target *-*-* } > open_extern_c } > diff --git a/gcc/testsuite/g++.dg/template/extern-c.C > b/gcc/testsuite/g++.dg/template/extern-c.C > new file mode 100644 > index 0000000..8a3a77a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/extern-c.C > @@ -0,0 +1,39 @@ > +template <typename T> void specializable (T); > + > +/* Invalid template: within "extern C". */ > + > +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" > } > + > +template <typename T> // { dg-error "template with C linkage" } > +void within_extern_c_braces (void); > + > +} > + > +/* Valid template: not within "extern C". */ > + > +template <typename T> > +void not_within_extern_c (void); > + > + > +/* Invalid specialization: within "extern C". */ > + > +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" > } > + > +template <> // { dg-error "template specialization with C linkage" > } > +void specializable (int); > + > +} > + > + > +/* Valid specialization: not within "extern C". */ > +template <> > +void specializable (char); > + > + > +/* Example of extern C without braces. */ > + > +extern "C" template <typename T> // { dg-line > open_extern_c_no_braces } > +void within_extern_c_no_braces (void); > +// { dg-error "12: template with C linkage" "" { target *-*-* } > open_extern_c_no_braces } > +// { dg-message "1: 'extern .C.' linkage started here" "" { target > *-*-* } open_extern_c_no_braces } > + ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: show location of problematic extern "C" specifications 2017-09-26 19:27 [PATCH] C++: show location of problematic extern "C" specifications David Malcolm 2017-10-11 17:37 ` PING " David Malcolm @ 2017-10-11 20:50 ` Jason Merrill 2017-10-11 20:52 ` David Malcolm 2017-10-12 18:45 ` [PATCH] C++: show location of unclosed extern "C" specifications (v2) David Malcolm 1 sibling, 2 replies; 9+ messages in thread From: Jason Merrill @ 2017-10-11 20:50 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches List On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm <dmalcolm@redhat.com> wrote: > * cp-tree.h (struct saved_scope): Add "location" field. saved_scope seems like the wrong place for this; it's only interesting at parse time, so having it saved during template instantiation doesn't seem useful. I'd prefer to put it in cp_parser and have maybe_show_extern_c_location look in the_parser. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: show location of problematic extern "C" specifications 2017-10-11 20:50 ` Jason Merrill @ 2017-10-11 20:52 ` David Malcolm 2017-10-11 21:22 ` Jason Merrill 2017-10-12 18:45 ` [PATCH] C++: show location of unclosed extern "C" specifications (v2) David Malcolm 1 sibling, 1 reply; 9+ messages in thread From: David Malcolm @ 2017-10-11 20:52 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List On Wed, 2017-10-11 at 15:51 -0400, Jason Merrill wrote: > On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm <dmalcolm@redhat.com> > wrote: > > * cp-tree.h (struct saved_scope): Add "location" field. > > saved_scope seems like the wrong place for this; it's only > interesting > at parse time, so having it saved during template instantiation > doesn't seem useful. I'd prefer to put it in cp_parser and have > maybe_show_extern_c_location look in the_parser. Thanks. I have a new version of the patch *mostly* working that way, but one of the uses of maybe_show_extern_c_location is within decl.c:grokfndecl (when complaining about user-defined literal operators within C linkage), and there doesn't seem to be access to the cp_parser * from there. I could fix this by adding a cp_parser * for this to grokfndecl, which would mean adding it to grokdeclarator, but I get the impression that the code is structured so that the decl-handling isn't meant to know about the parser. Thoughts? Dave ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: show location of problematic extern "C" specifications 2017-10-11 20:52 ` David Malcolm @ 2017-10-11 21:22 ` Jason Merrill 2017-10-11 22:26 ` David Malcolm 0 siblings, 1 reply; 9+ messages in thread From: Jason Merrill @ 2017-10-11 21:22 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches List On Wed, Oct 11, 2017 at 4:50 PM, David Malcolm <dmalcolm@redhat.com> wrote: > On Wed, 2017-10-11 at 15:51 -0400, Jason Merrill wrote: >> On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm <dmalcolm@redhat.com> >> wrote: >> > * cp-tree.h (struct saved_scope): Add "location" field. >> >> saved_scope seems like the wrong place for this; it's only >> interesting >> at parse time, so having it saved during template instantiation >> doesn't seem useful. I'd prefer to put it in cp_parser and have >> maybe_show_extern_c_location look in the_parser. > > Thanks. > > I have a new version of the patch *mostly* working that way, but one of > the uses of maybe_show_extern_c_location is within decl.c:grokfndecl > (when complaining about user-defined literal operators within C > linkage), and there doesn't seem to be access to the cp_parser * from > there. > > I could fix this by adding a cp_parser * for this to grokfndecl, which > would mean adding it to grokdeclarator, but I get the impression that > the code is structured so that the decl-handling isn't meant to know > about the parser. That's why I was thinking to look at "the_parser", which functions in parser.c can get at directly. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: show location of problematic extern "C" specifications 2017-10-11 21:22 ` Jason Merrill @ 2017-10-11 22:26 ` David Malcolm 0 siblings, 0 replies; 9+ messages in thread From: David Malcolm @ 2017-10-11 22:26 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List On Wed, 2017-10-11 at 17:20 -0400, Jason Merrill wrote: > On Wed, Oct 11, 2017 at 4:50 PM, David Malcolm <dmalcolm@redhat.com> > wrote: > > On Wed, 2017-10-11 at 15:51 -0400, Jason Merrill wrote: > > > On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm <dmalcolm@redhat.c > > > om> > > > wrote: > > > > * cp-tree.h (struct saved_scope): Add "location" field. > > > > > > saved_scope seems like the wrong place for this; it's only > > > interesting > > > at parse time, so having it saved during template instantiation > > > doesn't seem useful. I'd prefer to put it in cp_parser and have > > > maybe_show_extern_c_location look in the_parser. > > > > Thanks. > > > > I have a new version of the patch *mostly* working that way, but > > one of > > the uses of maybe_show_extern_c_location is within > > decl.c:grokfndecl > > (when complaining about user-defined literal operators within C > > linkage), and there doesn't seem to be access to the cp_parser * > > from > > there. > > > > I could fix this by adding a cp_parser * for this to grokfndecl, > > which > > would mean adding it to grokdeclarator, but I get the impression > > that > > the code is structured so that the decl-handling isn't meant to > > know > > about the parser. > > That's why I was thinking to look at "the_parser", which functions in > parser.c can get at directly. > > Jason Aha - thanks: I missed the underscore and read it as "the parser". Sorry. Am working on an updated patch. Dave ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] C++: show location of unclosed extern "C" specifications (v2) 2017-10-11 20:50 ` Jason Merrill 2017-10-11 20:52 ` David Malcolm @ 2017-10-12 18:45 ` David Malcolm 2017-10-12 21:18 ` Jason Merrill 1 sibling, 1 reply; 9+ messages in thread From: David Malcolm @ 2017-10-12 18:45 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, David Malcolm On Wed, 2017-10-11 at 15:51 -0400, Jason Merrill wrote: > On Tue, Sep 26, 2017 at 3:27 PM, David Malcolm <dmalcolm@redhat.com> > wrote: > > * cp-tree.h (struct saved_scope): Add "location" field. > > saved_scope seems like the wrong place for this; it's only > interesting > at parse time, so having it saved during template instantiation > doesn't seem useful. I'd prefer to put it in cp_parser and have > maybe_show_extern_c_location look in the_parser. > > Jason Here's an updated version that moves it there. Other changes in v2: - handle nested linkage specifications - put the note on the string-literal, rather than the extern: note: 'extern "C"' linkage started here extern "C" { ^~~ Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. OK for trunk? Thanks Dave Blurb from v1: If the user fails to close an extern "C" linkage specifier, and then uses templates, they will run into "template with C linkage" errors. From personal experience, it can be hard to tell where the extern "C" began. As of r251026 there will be a message highlighting the unclosed '{', but this may be hard to spot at the very end of the errors. This patch adds a note to the various diagnostics that complain about C linkage, showing the user where the extern "C" specification began. gcc/cp/ChangeLog: * cp-tree.h (maybe_show_extern_c_location): New decl. * decl.c (grokfndecl): When complaining about literal operators with C linkage, issue a note giving the location of the extern "C". * parser.c (cp_parser_new): Initialize new field "innermost_linkage_specification_location". (cp_parser_linkage_specification): Store the location of the string-literal token within the cp_parser. (cp_parser_explicit_specialization): When complaining about template specializations with C linkage, issue a note giving the location of the extern "C". (cp_parser_explicit_template_declaration): Likewise for templates. (maybe_show_extern_c_location): New function. * parser.h (struct cp_parser): New field "innermost_linkage_specification_location". gcc/testsuite/ChangeLog: * g++.dg/cpp0x/udlit-extern-c.C: New test case. * g++.dg/diagnostic/unclosed-extern-c.C: Add example of a template erroneously covered by an unclosed extern "C". * g++.dg/template/extern-c.C: New test case. --- gcc/cp/cp-tree.h | 1 + gcc/cp/decl.c | 1 + gcc/cp/parser.c | 29 ++++++++++ gcc/cp/parser.h | 4 ++ gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C | 7 +++ .../g++.dg/diagnostic/unclosed-extern-c.C | 11 +++- gcc/testsuite/g++.dg/template/extern-c.C | 66 ++++++++++++++++++++++ 7 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C create mode 100644 gcc/testsuite/g++.dg/template/extern-c.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 82ebc28..df3d198 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6362,6 +6362,7 @@ extern bool parsing_nsdmi (void); extern bool parsing_default_capturing_generic_lambda_in_template (void); extern void inject_this_parameter (tree, cp_cv_quals); extern location_t defarg_location (tree); +extern void maybe_show_extern_c_location (void); /* in pt.c */ extern bool check_template_shadow (tree); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 99f0af2..51a5305 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -8724,6 +8724,7 @@ grokfndecl (tree ctype, if (DECL_LANGUAGE (decl) == lang_c) { error ("literal operator with C linkage"); + maybe_show_extern_c_location (); return NULL_TREE; } diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 28bc8e4..a556a21 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -3828,6 +3828,9 @@ cp_parser_new (void) /* Allow constrained-type-specifiers. */ parser->prevent_constrained_type_specifiers = 0; + /* We haven't yet seen an 'extern "C"'. */ + parser->innermost_linkage_specification_location = UNKNOWN_LOCATION; + return parser; } @@ -13740,6 +13743,7 @@ cp_parser_linkage_specification (cp_parser* parser) cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN); /* Look for the string-literal. */ + cp_token *string_token = cp_lexer_peek_token (parser->lexer); linkage = cp_parser_string_literal (parser, false, false); /* Transform the literal into an identifier. If the literal is a @@ -13758,6 +13762,13 @@ cp_parser_linkage_specification (cp_parser* parser) /* We're now using the new linkage. */ push_lang_context (linkage); + /* Preserve the location of the string-literal for the innermost linkage + specification, tracking the locations of nested specifications + via a local. */ + location_t saved_location + = parser->innermost_linkage_specification_location; + parser->innermost_linkage_specification_location = string_token->location; + /* If the next token is a `{', then we're using the first production. */ if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE)) @@ -13788,6 +13799,9 @@ cp_parser_linkage_specification (cp_parser* parser) /* We're done with the linkage-specification. */ pop_lang_context (); + + /* Restore location of parent linkage specification, if any. */ + parser->innermost_linkage_specification_location = saved_location; } /* Parse a static_assert-declaration. @@ -16537,6 +16551,7 @@ cp_parser_explicit_specialization (cp_parser* parser) if (current_lang_name == lang_name_c) { error_at (token->location, "template specialization with C linkage"); + maybe_show_extern_c_location (); /* Give it C++ linkage to avoid confusing other parts of the front end. */ push_lang_context (lang_name_cplusplus); @@ -26861,6 +26876,7 @@ cp_parser_explicit_template_declaration (cp_parser* parser, bool member_p) if (current_lang_name == lang_name_c) { error_at (location, "template with C linkage"); + maybe_show_extern_c_location (); /* Give it C++ linkage to avoid confusing other parts of the front end. */ push_lang_context (lang_name_cplusplus); @@ -39479,4 +39495,17 @@ finish_fully_implicit_template (cp_parser *parser, tree member_decl_opt) return member_decl_opt; } +/* Helper function for diagnostics that have complained about things + being used with 'extern "C"' linkage. + + Attempt to issue a note showing where the 'extern "C"' linkage began. */ + +void +maybe_show_extern_c_location (void) +{ + if (the_parser->innermost_linkage_specification_location != UNKNOWN_LOCATION) + inform (the_parser->innermost_linkage_specification_location, + "%<extern \"C\"%> linkage started here"); +} + #include "gt-cp-parser.h" diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h index 0994e1e..f4f4a01 100644 --- a/gcc/cp/parser.h +++ b/gcc/cp/parser.h @@ -412,6 +412,10 @@ struct GTY(()) cp_parser { context e.g., because they could never be deduced. */ int prevent_constrained_type_specifiers; + /* Location of the string-literal token within the current linkage + specification, if any, or UNKNOWN_LOCATION otherwise. */ + location_t innermost_linkage_specification_location; + }; /* In parser.c */ diff --git a/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C b/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C new file mode 100644 index 0000000..ea54cba --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C @@ -0,0 +1,7 @@ +// { dg-do compile { target c++11 } } + +extern "C" { // { dg-message "8: 'extern .C.' linkage started here" } + +constexpr double operator"" _deg ( double degrees ); // { dg-error "literal operator with C linkage" } + +} diff --git a/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C b/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C index fda3532..d85a049 100644 --- a/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C +++ b/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C @@ -1,3 +1,12 @@ -extern "C" { /* { dg-message "12: to match this '.'" } */ +extern "C" { // { dg-line open_extern_c } + + int foo (void); + +/* Missing close-brace for the extern "C" here. */ + +template <typename T> // { dg-error "template with C linkage" } +void bar (void); +// { dg-message "8: 'extern .C.' linkage started here" "" { target *-*-* } open_extern_c } void test (void); /* { dg-error "17: expected '.' at end of input" } */ +// { message "12: to match this '.'" "" { target *-*-* } open_extern_c } diff --git a/gcc/testsuite/g++.dg/template/extern-c.C b/gcc/testsuite/g++.dg/template/extern-c.C new file mode 100644 index 0000000..6cbf69f --- /dev/null +++ b/gcc/testsuite/g++.dg/template/extern-c.C @@ -0,0 +1,66 @@ +template <typename T> void specializable (T); + +/* Invalid template: within "extern C". */ + +extern "C" { // { dg-message "8: 'extern .C.' linkage started here" } + +template <typename T> // { dg-error "template with C linkage" } +void within_extern_c_braces (void); + +} + +/* Valid template: not within "extern C". */ + +template <typename T> +void not_within_extern_c (void); + + +/* Invalid specialization: within "extern C". */ + +extern "C" { // { dg-message "8: 'extern .C.' linkage started here" } + +template <> // { dg-error "template specialization with C linkage" } +void specializable (int); + +} + + +/* Valid specialization: not within "extern C". */ +template <> +void specializable (char); + + +/* Example of extern C without braces. */ + +extern "C" template <typename T> // { dg-line open_extern_c_no_braces } +void within_extern_c_no_braces (void); +// { dg-error "12: template with C linkage" "" { target *-*-* } open_extern_c_no_braces } +// { dg-message "8: 'extern .C.' linkage started here" "" { target *-*-* } open_extern_c_no_braces } + + +/* Nested extern "C" specifications. + We should report within the innermost extern "C" that's still open. */ + +extern "C" { + extern "C" { // { dg-line middle_open_extern_c } + extern "C" { + } + + template <typename T> // { dg-error "template with C linkage" } + void within_nested_extern_c (void); + // { dg-message "10: 'extern .C.' linkage started here" "" { target *-*-* } middle_open_extern_c } + + extern "C++" { + /* Valid template: within extern "C++". */ + template <typename T> + void within_nested_extern_cpp (void); + + extern "C" { // { dg-line last_open_extern_c } + /* Invalid template: within "extern C". */ + template <typename T> // { dg-error "template with C linkage" } + void within_extern_c_within_extern_cpp (void); + // { dg-message "14: 'extern .C.' linkage started here" "" { target *-*-* } last_open_extern_c } + } + } + } +} -- 1.8.5.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] C++: show location of unclosed extern "C" specifications (v2) 2017-10-12 18:45 ` [PATCH] C++: show location of unclosed extern "C" specifications (v2) David Malcolm @ 2017-10-12 21:18 ` Jason Merrill 2017-10-13 12:53 ` [committed] C++: show location of unclosed extern "C" specifications (v3) David Malcolm 0 siblings, 1 reply; 9+ messages in thread From: Jason Merrill @ 2017-10-12 21:18 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches List On Thu, Oct 12, 2017 at 2:45 PM, David Malcolm <dmalcolm@redhat.com> wrote: > - put the note on the string-literal, rather than the extern: > note: 'extern "C"' linkage started here > extern "C" { > ^~~ Maybe a range spanning both tokens? OK with or without that change. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* [committed] C++: show location of unclosed extern "C" specifications (v3) 2017-10-12 21:18 ` Jason Merrill @ 2017-10-13 12:53 ` David Malcolm 0 siblings, 0 replies; 9+ messages in thread From: David Malcolm @ 2017-10-13 12:53 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches List, David Malcolm On Thu, 2017-10-12 at 17:07 -0400, Jason Merrill wrote: > On Thu, Oct 12, 2017 at 2:45 PM, David Malcolm <dmalcolm@redhat.com> > wrote: > > - put the note on the string-literal, rather than the extern: > > note: 'extern "C"' linkage started here > > extern "C" { > > ^~~ > > Maybe a range spanning both tokens? > > OK with or without that change. > > Jason Good idea; I've implemented it. Committed to trunk as r253726 (after usual testing). For reference, here's what I committed: gcc/cp/ChangeLog: * cp-tree.h (maybe_show_extern_c_location): New decl. * decl.c (grokfndecl): When complaining about literal operators with C linkage, issue a note giving the location of the extern "C". * parser.c (cp_parser_new): Initialize new field "innermost_linkage_specification_location". (cp_parser_linkage_specification): Store the location of the linkage specification within the cp_parser. (cp_parser_explicit_specialization): When complaining about template specializations with C linkage, issue a note giving the location of the extern "C". (cp_parser_explicit_template_declaration): Likewise for templates. (maybe_show_extern_c_location): New function. * parser.h (struct cp_parser): New field "innermost_linkage_specification_location". gcc/testsuite/ChangeLog: * g++.dg/cpp0x/udlit-extern-c.C: New test case. * g++.dg/diagnostic/unclosed-extern-c.C: Add example of a template erroneously covered by an unclosed extern "C". * g++.dg/template/extern-c.C: New test case. --- gcc/cp/cp-tree.h | 1 + gcc/cp/decl.c | 1 + gcc/cp/parser.c | 39 ++++++++++++- gcc/cp/parser.h | 4 ++ gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C | 7 +++ .../g++.dg/diagnostic/unclosed-extern-c.C | 11 +++- gcc/testsuite/g++.dg/template/extern-c.C | 66 ++++++++++++++++++++++ 7 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C create mode 100644 gcc/testsuite/g++.dg/template/extern-c.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index dc98dd8..b74b6d9 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -6356,6 +6356,7 @@ extern bool parsing_nsdmi (void); extern bool parsing_default_capturing_generic_lambda_in_template (void); extern void inject_this_parameter (tree, cp_cv_quals); extern location_t defarg_location (tree); +extern void maybe_show_extern_c_location (void); /* in pt.c */ extern bool check_template_shadow (tree); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index f251a90..a3cc80c 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -8729,6 +8729,7 @@ grokfndecl (tree ctype, if (DECL_LANGUAGE (decl) == lang_c) { error ("literal operator with C linkage"); + maybe_show_extern_c_location (); return NULL_TREE; } diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 810e2b7..2337be5 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -3937,6 +3937,9 @@ cp_parser_new (void) /* Allow constrained-type-specifiers. */ parser->prevent_constrained_type_specifiers = 0; + /* We haven't yet seen an 'extern "C"'. */ + parser->innermost_linkage_specification_location = UNKNOWN_LOCATION; + return parser; } @@ -13848,9 +13851,11 @@ cp_parser_linkage_specification (cp_parser* parser) tree linkage; /* Look for the `extern' keyword. */ - cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN); + cp_token *extern_token + = cp_parser_require_keyword (parser, RID_EXTERN, RT_EXTERN); /* Look for the string-literal. */ + cp_token *string_token = cp_lexer_peek_token (parser->lexer); linkage = cp_parser_string_literal (parser, false, false); /* Transform the literal into an identifier. If the literal is a @@ -13869,6 +13874,20 @@ cp_parser_linkage_specification (cp_parser* parser) /* We're now using the new linkage. */ push_lang_context (linkage); + /* Preserve the location of the the innermost linkage specification, + tracking the locations of nested specifications via a local. */ + location_t saved_location + = parser->innermost_linkage_specification_location; + /* Construct a location ranging from the start of the "extern" to + the end of the string-literal, with the caret at the start, e.g.: + extern "C" { + ^~~~~~~~~~ + */ + parser->innermost_linkage_specification_location + = make_location (extern_token->location, + extern_token->location, + get_finish (string_token->location)); + /* If the next token is a `{', then we're using the first production. */ if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE)) @@ -13899,6 +13918,9 @@ cp_parser_linkage_specification (cp_parser* parser) /* We're done with the linkage-specification. */ pop_lang_context (); + + /* Restore location of parent linkage specification, if any. */ + parser->innermost_linkage_specification_location = saved_location; } /* Parse a static_assert-declaration. @@ -16643,6 +16665,7 @@ cp_parser_explicit_specialization (cp_parser* parser) if (current_lang_name == lang_name_c) { error_at (token->location, "template specialization with C linkage"); + maybe_show_extern_c_location (); /* Give it C++ linkage to avoid confusing other parts of the front end. */ push_lang_context (lang_name_cplusplus); @@ -26979,6 +27002,7 @@ cp_parser_explicit_template_declaration (cp_parser* parser, bool member_p) if (current_lang_name == lang_name_c) { error_at (location, "template with C linkage"); + maybe_show_extern_c_location (); /* Give it C++ linkage to avoid confusing other parts of the front end. */ push_lang_context (lang_name_cplusplus); @@ -39552,4 +39576,17 @@ finish_fully_implicit_template (cp_parser *parser, tree member_decl_opt) return member_decl_opt; } +/* Helper function for diagnostics that have complained about things + being used with 'extern "C"' linkage. + + Attempt to issue a note showing where the 'extern "C"' linkage began. */ + +void +maybe_show_extern_c_location (void) +{ + if (the_parser->innermost_linkage_specification_location != UNKNOWN_LOCATION) + inform (the_parser->innermost_linkage_specification_location, + "%<extern \"C\"%> linkage started here"); +} + #include "gt-cp-parser.h" diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h index 0994e1e..f4f4a01 100644 --- a/gcc/cp/parser.h +++ b/gcc/cp/parser.h @@ -412,6 +412,10 @@ struct GTY(()) cp_parser { context e.g., because they could never be deduced. */ int prevent_constrained_type_specifiers; + /* Location of the string-literal token within the current linkage + specification, if any, or UNKNOWN_LOCATION otherwise. */ + location_t innermost_linkage_specification_location; + }; /* In parser.c */ diff --git a/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C b/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C new file mode 100644 index 0000000..d47a49c --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/udlit-extern-c.C @@ -0,0 +1,7 @@ +// { dg-do compile { target c++11 } } + +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" } + +constexpr double operator"" _deg ( double degrees ); // { dg-error "literal operator with C linkage" } + +} diff --git a/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C b/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C index fda3532..44f538e 100644 --- a/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C +++ b/gcc/testsuite/g++.dg/diagnostic/unclosed-extern-c.C @@ -1,3 +1,12 @@ -extern "C" { /* { dg-message "12: to match this '.'" } */ +extern "C" { // { dg-line open_extern_c } + + int foo (void); + +/* Missing close-brace for the extern "C" here. */ + +template <typename T> // { dg-error "template with C linkage" } +void bar (void); +// { dg-message "1: 'extern .C.' linkage started here" "" { target *-*-* } open_extern_c } void test (void); /* { dg-error "17: expected '.' at end of input" } */ +// { message "12: to match this '.'" "" { target *-*-* } open_extern_c } diff --git a/gcc/testsuite/g++.dg/template/extern-c.C b/gcc/testsuite/g++.dg/template/extern-c.C new file mode 100644 index 0000000..c0dd7cb --- /dev/null +++ b/gcc/testsuite/g++.dg/template/extern-c.C @@ -0,0 +1,66 @@ +template <typename T> void specializable (T); + +/* Invalid template: within "extern C". */ + +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" } + +template <typename T> // { dg-error "template with C linkage" } +void within_extern_c_braces (void); + +} + +/* Valid template: not within "extern C". */ + +template <typename T> +void not_within_extern_c (void); + + +/* Invalid specialization: within "extern C". */ + +extern "C" { // { dg-message "1: 'extern .C.' linkage started here" } + +template <> // { dg-error "template specialization with C linkage" } +void specializable (int); + +} + + +/* Valid specialization: not within "extern C". */ +template <> +void specializable (char); + + +/* Example of extern C without braces. */ + +extern "C" template <typename T> // { dg-line open_extern_c_no_braces } +void within_extern_c_no_braces (void); +// { dg-error "12: template with C linkage" "" { target *-*-* } open_extern_c_no_braces } +// { dg-message "1: 'extern .C.' linkage started here" "" { target *-*-* } open_extern_c_no_braces } + + +/* Nested extern "C" specifications. + We should report within the innermost extern "C" that's still open. */ + +extern "C" { + extern "C" { // { dg-line middle_open_extern_c } + extern "C" { + } + + template <typename T> // { dg-error "template with C linkage" } + void within_nested_extern_c (void); + // { dg-message "3: 'extern .C.' linkage started here" "" { target *-*-* } middle_open_extern_c } + + extern "C++" { + /* Valid template: within extern "C++". */ + template <typename T> + void within_nested_extern_cpp (void); + + extern "C" { // { dg-line last_open_extern_c } + /* Invalid template: within "extern C". */ + template <typename T> // { dg-error "template with C linkage" } + void within_extern_c_within_extern_cpp (void); + // { dg-message "7: 'extern .C.' linkage started here" "" { target *-*-* } last_open_extern_c } + } + } + } +} -- 1.8.5.3 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-10-13 12:45 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-26 19:27 [PATCH] C++: show location of problematic extern "C" specifications David Malcolm 2017-10-11 17:37 ` PING " David Malcolm 2017-10-11 20:50 ` Jason Merrill 2017-10-11 20:52 ` David Malcolm 2017-10-11 21:22 ` Jason Merrill 2017-10-11 22:26 ` David Malcolm 2017-10-12 18:45 ` [PATCH] C++: show location of unclosed extern "C" specifications (v2) David Malcolm 2017-10-12 21:18 ` Jason Merrill 2017-10-13 12:53 ` [committed] C++: show location of unclosed extern "C" specifications (v3) David Malcolm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).