c-family: Have -Wformat-diag accept "decl-specifier" [PR103758]
Commit Message
I'm tired of seeing
cp/parser.c:15923:55: warning: misspelled term 'decl' in format; use 'declaration' instead [-Wformat-diag]
cp/parser.c:15925:57: warning: misspelled term 'decl' in format; use 'declaration' instead [-Wformat-diag]
every time I compile cp/parser.c, which happens...a lot. I'd like my
compilation to be free of warnings, otherwise I'm going to miss some
important ones.
"decl-specifiers" is a C++ grammar term; it is not actual code, so
should not be wrapped with %< %>. I hope we can accept it as an exception
in check_tokens.
It was surrounded by %< %> in cp_parser_decl_specifier_seq, so fix that.
In passing, fix a misspelling in missspellings.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
In fact, I think I'd like to backport to 11 too, so that eventually
even my system compiler stops warning about this.
PR c++/103758
gcc/c-family/ChangeLog:
* c-format.c (check_tokens): Accept "decl-specifier*".
gcc/cp/ChangeLog:
* parser.c (cp_parser_decl_specifier_seq): Replace %<decl-specifier%>
with %qD.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-condition.C: Adjust dg-error.
---
gcc/c-family/c-format.c | 10 +++++++++-
gcc/cp/parser.c | 2 +-
gcc/testsuite/g++.dg/cpp0x/constexpr-condition.C | 2 +-
3 files changed, 11 insertions(+), 3 deletions(-)
base-commit: d7ca2a79b82c6500ead6ab983d14c609e2124eee
Comments
On 12/17/2021 3:59 PM, Marek Polacek via Gcc-patches wrote:
> I'm tired of seeing
>
> cp/parser.c:15923:55: warning: misspelled term 'decl' in format; use 'declaration' instead [-Wformat-diag]
> cp/parser.c:15925:57: warning: misspelled term 'decl' in format; use 'declaration' instead [-Wformat-diag]
>
> every time I compile cp/parser.c, which happens...a lot. I'd like my
> compilation to be free of warnings, otherwise I'm going to miss some
> important ones.
>
> "decl-specifiers" is a C++ grammar term; it is not actual code, so
> should not be wrapped with %< %>. I hope we can accept it as an exception
> in check_tokens.
>
> It was surrounded by %< %> in cp_parser_decl_specifier_seq, so fix that.
>
> In passing, fix a misspelling in missspellings.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> In fact, I think I'd like to backport to 11 too, so that eventually
> even my system compiler stops warning about this.
>
> PR c++/103758
>
> gcc/c-family/ChangeLog:
>
> * c-format.c (check_tokens): Accept "decl-specifier*".
>
> gcc/cp/ChangeLog:
>
> * parser.c (cp_parser_decl_specifier_seq): Replace %<decl-specifier%>
> with %qD.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/constexpr-condition.C: Adjust dg-error.
OK. I know, cp/, but it seems trivial enough that I don't mind ACKing
this one.
jeff
On 12/17/21 17:59, Marek Polacek wrote:
> I'm tired of seeing
>
> cp/parser.c:15923:55: warning: misspelled term 'decl' in format; use 'declaration' instead [-Wformat-diag]
> cp/parser.c:15925:57: warning: misspelled term 'decl' in format; use 'declaration' instead [-Wformat-diag]
>
> every time I compile cp/parser.c, which happens...a lot. I'd like my
> compilation to be free of warnings, otherwise I'm going to miss some
> important ones.
>
> "decl-specifiers" is a C++ grammar term; it is not actual code, so
> should not be wrapped with %< %>. I hope we can accept it as an exception
> in check_tokens.
>
> It was surrounded by %< %> in cp_parser_decl_specifier_seq, so fix that.
>
> In passing, fix a misspelling in missspellings.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> In fact, I think I'd like to backport to 11 too, so that eventually
> even my system compiler stops warning about this.
>
> PR c++/103758
>
> gcc/c-family/ChangeLog:
>
> * c-format.c (check_tokens): Accept "decl-specifier*".
>
> gcc/cp/ChangeLog:
>
> * parser.c (cp_parser_decl_specifier_seq): Replace %<decl-specifier%>
> with %qD.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/constexpr-condition.C: Adjust dg-error.
> ---
> gcc/c-family/c-format.c | 10 +++++++++-
> gcc/cp/parser.c | 2 +-
> gcc/testsuite/g++.dg/cpp0x/constexpr-condition.C | 2 +-
> 3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
> index 617fb5ea626..e1b1d6ba31e 100644
> --- a/gcc/c-family/c-format.c
> +++ b/gcc/c-family/c-format.c
> @@ -3194,7 +3194,7 @@ check_tokens (const token_t *tokens, unsigned ntoks,
> wlen, format_chars);
> else
> {
> - /* Diagnose some common missspellings. */
> + /* Diagnose some common misspellings. */
> for (unsigned i = 0; i != sizeof badwords / sizeof *badwords; ++i)
> {
> unsigned badwlen = strspn (badwords[i].name, " -");
> @@ -3215,6 +3215,14 @@ check_tokens (const token_t *tokens, unsigned ntoks,
> plural = "s";
> }
>
> + /* As an exception, don't warn about "decl-specifier*" since
> + it's a C++ grammar production. */
> + {
> + const size_t l = strlen ("decl-specifier");
> + if (!strncmp (format_chars, "decl-specifier", l))
> + return format_chars + l - 1;
> + }
I'd prefer to avoid repeating the string literal, but OK.
> format_warning_substr (format_string_loc, format_string_cst,
> fmtchrpos, fmtchrpos + badwords[i].len,
> opt,
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 44eed7ea638..3b33ae0cc21 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -15820,7 +15820,7 @@ cp_parser_decl_specifier_seq (cp_parser* parser,
> if (found_decl_spec
> && (flags & CP_PARSER_FLAGS_ONLY_TYPE_OR_CONSTEXPR)
> && token->keyword != RID_CONSTEXPR)
> - error ("%<decl-specifier%> invalid in condition");
> + error ("%qD invalid in condition", ridpointers[token->keyword]);
>
> if (found_decl_spec
> && (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-condition.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-condition.C
> index 733d494c4d7..e81acba68ae 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/constexpr-condition.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-condition.C
> @@ -5,5 +5,5 @@ constexpr int something() { return 3; }
>
> int main() {
> if (constexpr long v = something()) {}
> - if (static long v = something()) { } // { dg-error "'decl-specifier' invalid" }
> + if (static long v = something()) { } // { dg-error "'static' invalid" }
> }
>
> base-commit: d7ca2a79b82c6500ead6ab983d14c609e2124eee
On Mon, Dec 20, 2021 at 03:21:04PM -0500, Jason Merrill via Gcc-patches wrote:
> > @@ -3215,6 +3215,14 @@ check_tokens (const token_t *tokens, unsigned ntoks,
> > plural = "s";
> > }
> > + /* As an exception, don't warn about "decl-specifier*" since
> > + it's a C++ grammar production. */
> > + {
> > + const size_t l = strlen ("decl-specifier");
> > + if (!strncmp (format_chars, "decl-specifier", l))
> > + return format_chars + l - 1;
> > + }
>
> I'd prefer to avoid repeating the string literal, but OK.
We have the startswith inline function that allows to avoid the repetition.
It wouldn't work in the above case due to the return format_chars + l - 1,
but that in itself looks quite weird to me, I'd think better would be
continue; instead of the return ...;, i.e. whenever we see decl-specifier
in the text pretend we haven't seen decl.
So
/* As an exception, don't warn about "decl-specifier*" since
it's a C++ grammar production. */
if (startswith (format_chars, "decl-specifier"))
continue;
?
Or perhaps even optimize and don't compare uselessly everything with
"decl-specifier" and do it only for the "decl" badword if it matches,
so
if (badwords[i].name[0] == 'd'
&& startswith (format_chars, "decl-specifier"))
continue;
?
Jakub
@@ -3194,7 +3194,7 @@ check_tokens (const token_t *tokens, unsigned ntoks,
wlen, format_chars);
else
{
- /* Diagnose some common missspellings. */
+ /* Diagnose some common misspellings. */
for (unsigned i = 0; i != sizeof badwords / sizeof *badwords; ++i)
{
unsigned badwlen = strspn (badwords[i].name, " -");
@@ -3215,6 +3215,14 @@ check_tokens (const token_t *tokens, unsigned ntoks,
plural = "s";
}
+ /* As an exception, don't warn about "decl-specifier*" since
+ it's a C++ grammar production. */
+ {
+ const size_t l = strlen ("decl-specifier");
+ if (!strncmp (format_chars, "decl-specifier", l))
+ return format_chars + l - 1;
+ }
+
format_warning_substr (format_string_loc, format_string_cst,
fmtchrpos, fmtchrpos + badwords[i].len,
opt,
@@ -15820,7 +15820,7 @@ cp_parser_decl_specifier_seq (cp_parser* parser,
if (found_decl_spec
&& (flags & CP_PARSER_FLAGS_ONLY_TYPE_OR_CONSTEXPR)
&& token->keyword != RID_CONSTEXPR)
- error ("%<decl-specifier%> invalid in condition");
+ error ("%qD invalid in condition", ridpointers[token->keyword]);
if (found_decl_spec
&& (flags & CP_PARSER_FLAGS_ONLY_MUTABLE_OR_CONSTEXPR)
@@ -5,5 +5,5 @@ constexpr int something() { return 3; }
int main() {
if (constexpr long v = something()) {}
- if (static long v = something()) { } // { dg-error "'decl-specifier' invalid" }
+ if (static long v = something()) { } // { dg-error "'static' invalid" }
}