public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RFC: Provide diagnostic hints for missing inttypes.h string constants.
@ 2020-05-21 23:30 Mark Wielaard
  2020-05-22 13:42 ` David Malcolm
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2020-05-21 23:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm, Mark Wielaard

This is on top of the stdbool.h and stdint.h patches.

This adds a flag to c_parser so we know when we were trying to
constract a string literal. If there is a parse error and we were
constructing a string literal, and the next token is an unknown
identifier name, and we know there is a standard header that defines
that name as a string literal, then add a missing header hint to
the error messsage.

I haven't checked yet if we can do something similar for the C++
parser. And the testcase needs to be extended a bit. But I hope the
direction is OK.

For the following source:

const char *hex64_fmt =  PRIx64;
const char *msg_fmt = "Provide %" PRIx64 "\n";

void foo (uint32_t v)
{
  printf ("%" PRIu32, v);
}

We will get the following:

$ /opt/local/gcc/bin/gcc -c t.c
t.c:4:26: error: ‘PRIx64’ undeclared here (not in a function)
    4 | const char *hex64_fmt =  PRIx64;
      |                          ^~~~~~
t.c:3:1: note: ‘PRIx64’ is defined in header ‘<inttypes.h>’; did you forget to ‘#include <inttypes.h>’?
    2 | #include <stdint.h>
  +++ |+#include <inttypes.h>
    3 |
t.c:5:35: error: expected ‘,’ or ‘;’ before ‘PRIx64’
    5 | const char *msg_fmt = "Provide %" PRIx64 "\n";
      |                                   ^~~~~~
t.c:5:35: note: ‘PRIx64’ is defined in header ‘<inttypes.h>’; did you forget to ‘#include <inttypes.h>’?
t.c: In function ‘foo’:
t.c:9:14: error: expected ‘)’ before ‘PRIu32’
    9 |   printf ("%" PRIu32, v);
      |              ^~~~~~~
      |              )
t.c:9:15: note: ‘PRIu32’ is defined in header ‘<inttypes.h>’; did you forget to ‘#include <inttypes.h>’?
    9 |   printf ("%" PRIu32, v);
      |               ^~~~~~
---
 gcc/c-family/known-headers.cc              | 88 ++++++++++++++++++++++
 gcc/c-family/known-headers.h               |  2 +
 gcc/c/c-parser.c                           | 29 +++++++
 gcc/testsuite/gcc.dg/spellcheck-inttypes.c | 32 ++++++++
 4 files changed, 151 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-inttypes.c

diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index 1e2bf49c439a..b9772af21863 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -46,6 +46,71 @@ struct stdlib_hint
   const char *header[NUM_STDLIBS];
 };
 
+/* These can be used as string macros or as identifiers. Must all be
+   string-literals.  Used in get_stdlib_header_for_name and
+   get_c_stdlib_header_for_string_macro_name.  */
+static const stdlib_hint c99_cxx11_macro_hints[] = {
+    /* <inttypes.h> and <cinttypes>.  */
+    {"PRId8", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRId16", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRId32", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRId64", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIi8", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIi16", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIi32", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIi64", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIo8", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIo16", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIo32", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIo64", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIu8", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIu16", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIu32", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIu64", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIx8", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIx16", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIx32", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIx64", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIX8", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIX16", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIX32", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIX64", {"<inttypes.h>", "<cinttypes>"} },
+
+    {"PRIdPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIiPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIoPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIuPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIxPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"PRIXPTR", {"<inttypes.h>", "<cinttypes>"} },
+
+    {"SCNd8", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNd16", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNd32", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNd64", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNi8", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNi16", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNi32", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNi64", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNo8", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNo16", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNo32", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNo64", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNu8", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNu16", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNu32", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNu64", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNx8", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNx16", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNx32", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNx64", {"<inttypes.h>", "<cinttypes>"} },
+
+    {"SCNdPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNiPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNoPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNuPTR", {"<inttypes.h>", "<cinttypes>"} },
+    {"SCNxPTR", {"<inttypes.h>", "<cinttypes>"} }
+};
+
 /* Given non-NULL NAME, return the header name defining it within either
    the standard library (with '<' and '>'), or NULL.
    Only handles a subset of the most common names within the stdlibs.  */
@@ -196,6 +261,14 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib)
       if (strcmp (name, c99_cxx11_hints[i].name) == 0)
 	return c99_cxx11_hints[i].header[lib];
 
+  const size_t num_c99_cxx11_macro_hints
+    = sizeof (c99_cxx11_macro_hints) / sizeof (c99_cxx11_macro_hints[0]);
+  if ((lib == STDLIB_C && flag_isoc99)
+      || (lib == STDLIB_CPLUSPLUS && cxx_dialect >= cxx11 ))
+    for (size_t i = 0; i < num_c99_cxx11_macro_hints; i++)
+      if (strcmp (name, c99_cxx11_macro_hints[i].name) == 0)
+	return c99_cxx11_macro_hints[i].header[lib];
+
   return NULL;
 }
 
@@ -217,6 +290,21 @@ get_cp_stdlib_header_for_name (const char *name)
   return get_stdlib_header_for_name (name, STDLIB_CPLUSPLUS);
 }
 
+/* Given non-NULL NAME, return the header name defining a string macro
+   within the C standard library (with '<' and '>'), or NULL.  */
+const char *
+get_c_stdlib_header_for_string_macro_name (const char *name)
+{
+  const size_t num_c99_cxx11_macro_hints
+    = sizeof (c99_cxx11_macro_hints) / sizeof (c99_cxx11_macro_hints[0]);
+  if (flag_isoc99)
+    for (size_t i = 0; i < num_c99_cxx11_macro_hints; i++)
+      if (strcmp (name, c99_cxx11_macro_hints[i].name) == 0)
+	return c99_cxx11_macro_hints[i].header[STDLIB_C];
+
+  return NULL;
+}
+
 /* Implementation of class suggest_missing_header.  */
 
 /* suggest_missing_header's ctor.  */
diff --git a/gcc/c-family/known-headers.h b/gcc/c-family/known-headers.h
index 4ec4e23fc88d..a69bbbf28e76 100644
--- a/gcc/c-family/known-headers.h
+++ b/gcc/c-family/known-headers.h
@@ -23,6 +23,8 @@ along with GCC; see the file COPYING3.  If not see
 extern const char *get_c_stdlib_header_for_name (const char *name);
 extern const char *get_cp_stdlib_header_for_name (const char *name);
 
+extern const char *get_c_stdlib_header_for_string_macro_name (const char *n);
+
 /* Subclass of deferred_diagnostic for suggesting to the user
    that they have missed a #include.  */
 
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 5d11e7e73c16..6b2ae5688a72 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -69,6 +69,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/name-hint.h"
 #include "tree-iterator.h"
 #include "memmodel.h"
+#include "c-family/known-headers.h"
 
 /* We need to walk over decls with incomplete struct/union/enum types
    after parsing the whole translation unit.
@@ -223,6 +224,13 @@ struct GTY(()) c_parser {
      keywords are valid.  */
   BOOL_BITFIELD objc_property_attr_context : 1;
 
+  /* Whether we have just seen/constructed a string-literal.  Set when
+     returning a string-literal from c_parser_string_literal.  Reset
+     in consume_token.  Useful when we get a parse error and see an
+     unknown token, which could have been a string-literal constant
+     macro.  */
+  BOOL_BITFIELD seen_string_literal : 1;
+
   /* Location of the last consumed token.  */
   location_t last_token_location;
 };
@@ -853,6 +861,7 @@ c_parser_consume_token (c_parser *parser)
         }
     }
   parser->tokens_avail--;
+  parser->seen_string_literal = false;
 }
 
 /* Expect the current token to be a #pragma.  Consume it and remember
@@ -966,6 +975,25 @@ c_parser_error_richloc (c_parser *parser, const char *gmsgid,
 	}
     }
 
+  /* If we were parsing a string-literal and there is an unknown name
+     token right after, then check to see if that could also have been
+     a literal string by checking the name against a list of known
+     standard string literal constants defined in header files. If
+     there is one, then add that as an hint to the error message. */
+  auto_diagnostic_group d;
+  name_hint h;
+  if (parser->seen_string_literal && token->type == CPP_NAME)
+    {
+      tree name = token->value;
+      const char *header_hint
+	= get_c_stdlib_header_for_name (IDENTIFIER_POINTER (name));
+      if (header_hint != NULL)
+	h = name_hint (NULL,
+		       new suggest_missing_header (token->location,
+						   IDENTIFIER_POINTER (name),
+						   header_hint));
+    }
+
   c_parse_error (gmsgid,
 		 /* Because c_parse_error does not understand
 		    CPP_KEYWORD, keywords are treated like
@@ -7539,6 +7567,7 @@ c_parser_string_literal (c_parser *parser, bool translate, bool wide_ok)
   ret.original_code = STRING_CST;
   ret.original_type = NULL_TREE;
   set_c_expr_source_range (&ret, get_range_from_loc (line_table, loc));
+  parser->seen_string_literal = true;
   return ret;
 }
 
diff --git a/gcc/testsuite/gcc.dg/spellcheck-inttypes.c b/gcc/testsuite/gcc.dg/spellcheck-inttypes.c
new file mode 100644
index 000000000000..91c082d606d9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-inttypes.c
@@ -0,0 +1,32 @@
+/* { dg-options "-std=c99" } */
+#include <stdio.h>
+#include <stdint.h>
+/* Missing <inttypes.h>.  */
+
+int8_t i8;
+int16_t i16;
+int32_t i32;
+int64_t i64;
+
+intptr_t ip;
+uintptr_t up;
+
+/* As an identifier.  */
+const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+/* As a part of a string-literal.  */
+const char *msg_fmt = "Provide %" PRIx32 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+void test_printf (void)
+{
+  printf ("%" PRId8 "\n", i8); /* { dg-error "expected" } */
+/* { dg-message "'PRId8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+}
+
+void test_scanf (void)
+{
+  scanf ("%" SCNd8 "\n", &i8); /* { dg-error "expected" } */
+/* { dg-message "'SCNd8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: RFC: Provide diagnostic hints for missing inttypes.h string constants.
  2020-05-21 23:30 RFC: Provide diagnostic hints for missing inttypes.h string constants Mark Wielaard
@ 2020-05-22 13:42 ` David Malcolm
  2020-05-23  3:01   ` [PATCH] " Mark Wielaard
  0 siblings, 1 reply; 11+ messages in thread
From: David Malcolm @ 2020-05-22 13:42 UTC (permalink / raw)
  To: Mark Wielaard, gcc-patches

On Fri, 2020-05-22 at 01:30 +0200, Mark Wielaard wrote:
Hi Mark

> This is on top of the stdbool.h and stdint.h patches.

Sorry, I didn't see those patches; I've replied to them now.

> This adds a flag to c_parser so we know when we were trying to
> constract a string literal. If there is a parse error and we were
> constructing a string literal, and the next token is an unknown
> identifier name, and we know there is a standard header that defines
> that name as a string literal, then add a missing header hint to
> the error messsage.

By comparison, what's the existing behavior for the example?
Is it just what you posted below, but without the "note" diagnostics?

> I haven't checked yet if we can do something similar for the C++
> parser. And the testcase needs to be extended a bit. But I hope the
> direction is OK.

I think it's a worthy goal; as noted below I'd want a C frontend
maintainer to approve those parts of it.

> For the following source:
> 
> const char *hex64_fmt =  PRIx64;
> const char *msg_fmt = "Provide %" PRIx64 "\n";
> 
> void foo (uint32_t v)
> {
>   printf ("%" PRIu32, v);
> }
> 
> We will get the following:
> 
> $ /opt/local/gcc/bin/gcc -c t.c
> t.c:4:26: error: ‘PRIx64’ undeclared here (not in a function)
>     4 | const char *hex64_fmt =  PRIx64;
>       |                          ^~~~~~
> t.c:3:1: note: ‘PRIx64’ is defined in header ‘<inttypes.h>’; did you forget to ‘#include <inttypes.h>’?
>     2 | #include <stdint.h>
>   +++ |+#include <inttypes.h>
>     3 |
> t.c:5:35: error: expected ‘,’ or ‘;’ before ‘PRIx64’
>     5 | const char *msg_fmt = "Provide %" PRIx64 "\n";
>       |                                   ^~~~~~
> t.c:5:35: note: ‘PRIx64’ is defined in header ‘<inttypes.h>’; did you forget to ‘#include <inttypes.h>’?

It's a big improvement, but it's still a little clunky.  The
  expected ‘,’ or ‘;’ before ‘PRIx64’
is the sort of thing we've gotten used to with years of GCC messages,
but might make little sense to a neophyte.
I wonder if it's possible to improve this further, but I fear that that
might overcomplicate things (if I understand things correctly, the
string concatenation fails in the tokenizer, and then the PRIx64 fails
in the parser, so we have a bad interaction involving two different
levels of abstraction within the frontend - I think).

> t.c: In function ‘foo’:
> t.c:9:14: error: expected ‘)’ before ‘PRIu32’
>     9 |   printf ("%" PRIu32, v);
>       |              ^~~~~~~
>       |              )
> t.c:9:15: note: ‘PRIu32’ is defined in header ‘<inttypes.h>’; did you forget to ‘#include <inttypes.h>’?
>     9 |   printf ("%" PRIu32, v);
>       |               ^~~~~~
> ---

[...snip...]
  
> +/* These can be used as string macros or as identifiers. Must all be
> +   string-literals.  Used in get_stdlib_header_for_name and
> +   get_c_stdlib_header_for_string_macro_name.  */
> +static const stdlib_hint c99_cxx11_macro_hints[] = {
> +    /* <inttypes.h> and <cinttypes>.  */
> +    {"PRId8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRId16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRId32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRId64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIi8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIi16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIi32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIi64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIo8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIo16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIo32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIo64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIu8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIu16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIu32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIu64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIx8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIx16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIx32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIx64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIX8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIX16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIX32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIX64", {"<inttypes.h>", "<cinttypes>"} },
> +
> +    {"PRIdPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIiPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIoPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIuPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIxPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"PRIXPTR", {"<inttypes.h>", "<cinttypes>"} },
> +
> +    {"SCNd8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNd16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNd32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNd64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNi8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNi16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNi32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNi64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNo8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNo16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNo32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNo64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNu8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNu16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNu32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNu64", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNx8", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNx16", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNx32", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNx64", {"<inttypes.h>", "<cinttypes>"} },
> +
> +    {"SCNdPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNiPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNoPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNuPTR", {"<inttypes.h>", "<cinttypes>"} },
> +    {"SCNxPTR", {"<inttypes.h>", "<cinttypes>"} }
> +};

I found myself squinting at the array trying to decide if every entry
had
   {"<inttypes.h>", "<cinttypes>"}
as its second element.  I *think* that's the case, right? 

I know there's a lot of pre-existing duplication in this file, but
maybe this would be better as simply an array of const char *?
It could probably be reformatted to take up far fewer lines by grouping
them logically.

Maybe have a

  static const char *
  get_c99_cxx11_macro_hint (const char *, enum stdlib lib);

to do the predicate, and return one of "<inttypes.h>", "<cinttypes>", or NULL?

[...snip...]

> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index 5d11e7e73c16..6b2ae5688a72 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -69,6 +69,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "c-family/name-hint.h"
>  #include "tree-iterator.h"
>  #include "memmodel.h"
> +#include "c-family/known-headers.h"
>  
>  /* We need to walk over decls with incomplete struct/union/enum types
>     after parsing the whole translation unit.
> @@ -223,6 +224,13 @@ struct GTY(()) c_parser {
>       keywords are valid.  */
>    BOOL_BITFIELD objc_property_attr_context : 1;
>  
> +  /* Whether we have just seen/constructed a string-literal.  Set when
> +     returning a string-literal from c_parser_string_literal.  Reset
> +     in consume_token.  Useful when we get a parse error and see an
> +     unknown token, which could have been a string-literal constant
> +     macro.  */
> +  BOOL_BITFIELD seen_string_literal : 1;
> +
>    /* Location of the last consumed token.  */
>    location_t last_token_location;
>  };
> @@ -853,6 +861,7 @@ c_parser_consume_token (c_parser *parser)
>          }
>      }
>    parser->tokens_avail--;
> +  parser->seen_string_literal = false;
>  }
>

These hunks need review from a C frontend maintainer, as they're adding
a little extra work to every token.


>  /* Expect the current token to be a #pragma.  Consume it and remember
> @@ -966,6 +975,25 @@ c_parser_error_richloc (c_parser *parser, const char *gmsgid,
>         }
>      }
>  
> +  /* If we were parsing a string-literal and there is an unknown name
> +     token right after, then check to see if that could also have been
> +     a literal string by checking the name against a list of known
> +     standard string literal constants defined in header files. If
> +     there is one, then add that as an hint to the error message. */
> +  auto_diagnostic_group d;
> +  name_hint h;
> +  if (parser->seen_string_literal && token->type == CPP_NAME)
> +    {
> +      tree name = token->value;
> +      const char *header_hint
> +       = get_c_stdlib_header_for_name (IDENTIFIER_POINTER (name));
> +      if (header_hint != NULL)
> +       h = name_hint (NULL,
> +                      new suggest_missing_header (token->location,
> +                                                  IDENTIFIER_POINTER (name),
> +                                                  header_hint));
> +    }
> +
>    c_parse_error (gmsgid,
>                  /* Because c_parse_error does not understand
>                     CPP_KEYWORD, keywords are treated like
> @@ -7539,6 +7567,7 @@ c_parser_string_literal (c_parser *parser, bool translate, bool wide_ok)
>    ret.original_code = STRING_CST;
>    ret.original_type = NULL_TREE;
>    set_c_expr_source_range (&ret, get_range_from_loc (line_table, loc));
> +  parser->seen_string_literal = true;
>    return ret;
>  }

[...snip...]

Hope this is constructive
Dave


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Provide diagnostic hints for missing inttypes.h string constants.
  2020-05-22 13:42 ` David Malcolm
@ 2020-05-23  3:01   ` Mark Wielaard
  2020-05-24  0:30     ` Mark Wielaard
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2020-05-23  3:01 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 4614 bytes --]

Hi,

On Fri, May 22, 2020 at 09:42:28AM -0400, David Malcolm wrote:
> On Fri, 2020-05-22 at 01:30 +0200, Mark Wielaard wrote:
> > This is on top of the stdbool.h and stdint.h patches.
> 
> Sorry, I didn't see those patches; I've replied to them now.

No worries, there was no hurry and I didn't CC you on those.
I pushed them both now. Thanks.

> > This adds a flag to c_parser so we know when we were trying to
> > constract a string literal. If there is a parse error and we were
> > constructing a string literal, and the next token is an unknown
> > identifier name, and we know there is a standard header that defines
> > that name as a string literal, then add a missing header hint to
> > the error messsage.
> 
> By comparison, what's the existing behavior for the example?
> Is it just what you posted below, but without the "note" diagnostics?

Yes.

> > I haven't checked yet if we can do something similar for the C++
> > parser. And the testcase needs to be extended a bit. But I hope the
> > direction is OK.
> 
> I think it's a worthy goal; as noted below I'd want a C frontend
> maintainer to approve those parts of it.

OK.

> > $ /opt/local/gcc/bin/gcc -c t.c
> > t.c:4:26: error: ‘PRIx64’ undeclared here (not in a function)
> >     4 | const char *hex64_fmt =  PRIx64;
> >       |                          ^~~~~~
> > t.c:3:1: note: ‘PRIx64’ is defined in header ‘<inttypes.h>’; did you forget to ‘#include <inttypes.h>’?
> >     2 | #include <stdint.h>
> >   +++ |+#include <inttypes.h>
> >     3 |
> > t.c:5:35: error: expected ‘,’ or ‘;’ before ‘PRIx64’
> >     5 | const char *msg_fmt = "Provide %" PRIx64 "\n";
> >       |                                   ^~~~~~
> > t.c:5:35: note: ‘PRIx64’ is defined in header ‘<inttypes.h>’; did you forget to ‘#include <inttypes.h>’?
> 
> It's a big improvement, but it's still a little clunky.  The
>   expected ‘,’ or ‘;’ before ‘PRIx64’
> is the sort of thing we've gotten used to with years of GCC messages,
> but might make little sense to a neophyte.
> I wonder if it's possible to improve this further, but I fear that that
> might overcomplicate things (if I understand things correctly, the
> string concatenation fails in the tokenizer, and then the PRIx64 fails
> in the parser, so we have a bad interaction involving two different
> levels of abstraction within the frontend - I think).

Yes, part of the string concatenation is done by the tokenizer, the
other part by the parser. With this patch we now track when we were
trying to concatenate a string. Then if any error occurs and we notice
an unknown token is next that could be a string literal then we add
the hint to the error message with an alternative suggestion/hint for
a fix. I didn't want to override the existing error message, because
it might be right or better, and we don't actually know whether with
the suggestion the rest will actully parse correctly.

> > +/* These can be used as string macros or as identifiers. Must all be
> > +   string-literals.  Used in get_stdlib_header_for_name and
> > +   get_c_stdlib_header_for_string_macro_name.  */
> > +static const stdlib_hint c99_cxx11_macro_hints[] = {
> > +    /* <inttypes.h> and <cinttypes>.  */
> > +    {"PRId8", {"<inttypes.h>", "<cinttypes>"} },
> > [...]
> > +    {"SCNxPTR", {"<inttypes.h>", "<cinttypes>"} }
> > +};
> 
> I found myself squinting at the array trying to decide if every entry
> had
>    {"<inttypes.h>", "<cinttypes>"}
> as its second element.  I *think* that's the case, right? 
> 
> I know there's a lot of pre-existing duplication in this file, but
> maybe this would be better as simply an array of const char *?
> It could probably be reformatted to take up far fewer lines by grouping
> them logically.
> 
> Maybe have a
> 
>   static const char *
>   get_c99_cxx11_macro_hint (const char *, enum stdlib lib);
> 
> to do the predicate, and return one of "<inttypes.h>", "<cinttypes>", or NULL?

Yes, that is actually better. And much easier to read. And the code
can still be shared between get_c_stdlib_header_for_string_macro_name
and get_stdlib_header_for_name. Changed in the attached patch.

I also extended the testcase so it covers both identifiers and string
concatenation cases. The identifier hints also work for C++, but I
haven't yet added the string token concatenation detection to the
cp_parser. It looks like it can be done similar as done for the
c_parser, but the parsers are different enough that it seems
better/simpler to do that in a followup patch once we have this patch
in the the c_parser.

Thanks,

Mark

[-- Attachment #2: 0001-Provide-diagnostic-hints-for-missing-inttypes.h-stri.patch --]
[-- Type: text/x-diff, Size: 18130 bytes --]

From 98be2e83b6d0e69d769e8eff0f4b815240e51e1c Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Fri, 22 May 2020 01:10:50 +0200
Subject: [PATCH] Provide diagnostic hints for missing inttypes.h string
 constants.

This adds a flag to c_parser so we know when we were trying to
construct a string literal. If there is a parse error and we were
constructing a string literal, and the next token is an unknown
identifier name, and we know there is a standard header that defines
that name as a string literal, then add a missing header hint to
the error messsage.

gcc/c-family/ChangeLog:

	* known-headers.cc (get_string_macro_hint): New function.
	(get_stdlib_header_for_name): Use get_string_macro_hint.
	(get_c_stdlib_header_for_string_macro_name): New function.
	* known-headers.h (get_c_stdlib_header_for_string_macro_name):
	New function definition.

gcc/c/ChangeLog:

	* c-parser.c (struct c_parser): Add seen_string_literal
	bitfield.
	(c_parser_consume_token): Reset seen_string_literal.
	(c_parser_error_richloc): Add name_hint if seen_string_literal
	and next token is a CPP_NAME and we have a missing header
	suggestion for the name.
	(c_parser_string_literal): Set seen_string_literal.

gcc/testsuite/ChangeLog:

	* gcc.dg/spellcheck-inttypes.c: New test.
	* g++.dg/spellcheck-inttypes.C: Likewise.
---
 gcc/c-family/known-headers.cc              | 53 ++++++++++++++-
 gcc/c-family/known-headers.h               |  2 +
 gcc/c/c-parser.c                           | 29 ++++++++
 gcc/testsuite/g++.dg/spellcheck-inttypes.C | 41 ++++++++++++
 gcc/testsuite/gcc.dg/spellcheck-inttypes.c | 78 ++++++++++++++++++++++
 5 files changed, 202 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-inttypes.C
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-inttypes.c

diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index 1e2bf49c439a..c07cfd1db815 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -46,6 +46,49 @@ struct stdlib_hint
   const char *header[NUM_STDLIBS];
 };
 
+/* Given non-NULL NAME, return the header name defining it (as literal
+   string) within either the standard library (with '<' and '>'), or
+   NULL.
+
+   Only handle string macros, so that this can be used for
+   get_stdlib_header_for_name and
+   get_c_stdlib_header_for_string_macro_name.  */
+
+static const char *
+get_string_macro_hint (const char *name, enum stdlib lib)
+{
+  /* <inttypes.h> and <cinttypes>.  */
+  static const char *c99_cxx11_macros[] =
+    { "PRId8", "PRId16", "PRId32", "PRId64",
+      "PRIi8", "PRIi16", "PRIi32", "PRIi64",
+      "PRIo8", "PRIo16", "PRIo32", "PRIo64",
+      "PRIu8", "PRIu16", "PRIu32", "PRIu64",
+      "PRIx8", "PRIx16", "PRIx32", "PRIx64",
+      "PRIX8", "PRIX16", "PRIX32", "PRIX64",
+
+      "PRIdPTR", "PRIiPTR", "PRIoPTR", "PRIuPTR", "PRIxPTR", "PRIXPTR",
+
+      "SCNd8", "SCNd16", "SCNd32", "SCNd64",
+      "SCNi8", "SCNi16", "SCNi32", "SCNi64",
+      "SCNo8", "SCNo16", "SCNo32", "SCNo64",
+      "SCNu8", "SCNu16", "SCNu32", "SCNu64",
+      "SCNx8", "SCNx16", "SCNx32", "SCNx64",
+
+      "SCNdPTR", "SCNiPTR", "SCNoPTR", "SCNuPTR", "SCNxPTR" };
+
+  if ((lib == STDLIB_C && flag_isoc99)
+      || (lib == STDLIB_CPLUSPLUS && cxx_dialect >= cxx11 ))
+    {
+      const size_t num_c99_cxx11_macros
+	= sizeof (c99_cxx11_macros) / sizeof (c99_cxx11_macros[0]);
+      for (size_t i = 0; i < num_c99_cxx11_macros; i++)
+	if (strcmp (name, c99_cxx11_macros[i]) == 0)
+	  return lib == STDLIB_C ? "<inttypes.h>" : "<cinttypes>";
+    }
+
+  return NULL;
+}
+
 /* Given non-NULL NAME, return the header name defining it within either
    the standard library (with '<' and '>'), or NULL.
    Only handles a subset of the most common names within the stdlibs.  */
@@ -196,7 +239,7 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib)
       if (strcmp (name, c99_cxx11_hints[i].name) == 0)
 	return c99_cxx11_hints[i].header[lib];
 
-  return NULL;
+  return get_string_macro_hint (name, lib);
 }
 
 /* Given non-NULL NAME, return the header name defining it within the C
@@ -217,6 +260,14 @@ get_cp_stdlib_header_for_name (const char *name)
   return get_stdlib_header_for_name (name, STDLIB_CPLUSPLUS);
 }
 
+/* Given non-NULL NAME, return the header name defining a string macro
+   within the C standard library (with '<' and '>'), or NULL.  */
+const char *
+get_c_stdlib_header_for_string_macro_name (const char *name)
+{
+  return get_string_macro_hint (name, STDLIB_C);
+}
+
 /* Implementation of class suggest_missing_header.  */
 
 /* suggest_missing_header's ctor.  */
diff --git a/gcc/c-family/known-headers.h b/gcc/c-family/known-headers.h
index 4ec4e23fc88d..a69bbbf28e76 100644
--- a/gcc/c-family/known-headers.h
+++ b/gcc/c-family/known-headers.h
@@ -23,6 +23,8 @@ along with GCC; see the file COPYING3.  If not see
 extern const char *get_c_stdlib_header_for_name (const char *name);
 extern const char *get_cp_stdlib_header_for_name (const char *name);
 
+extern const char *get_c_stdlib_header_for_string_macro_name (const char *n);
+
 /* Subclass of deferred_diagnostic for suggesting to the user
    that they have missed a #include.  */
 
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 5d11e7e73c16..6b2ae5688a72 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -69,6 +69,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/name-hint.h"
 #include "tree-iterator.h"
 #include "memmodel.h"
+#include "c-family/known-headers.h"
 
 /* We need to walk over decls with incomplete struct/union/enum types
    after parsing the whole translation unit.
@@ -223,6 +224,13 @@ struct GTY(()) c_parser {
      keywords are valid.  */
   BOOL_BITFIELD objc_property_attr_context : 1;
 
+  /* Whether we have just seen/constructed a string-literal.  Set when
+     returning a string-literal from c_parser_string_literal.  Reset
+     in consume_token.  Useful when we get a parse error and see an
+     unknown token, which could have been a string-literal constant
+     macro.  */
+  BOOL_BITFIELD seen_string_literal : 1;
+
   /* Location of the last consumed token.  */
   location_t last_token_location;
 };
@@ -853,6 +861,7 @@ c_parser_consume_token (c_parser *parser)
         }
     }
   parser->tokens_avail--;
+  parser->seen_string_literal = false;
 }
 
 /* Expect the current token to be a #pragma.  Consume it and remember
@@ -966,6 +975,25 @@ c_parser_error_richloc (c_parser *parser, const char *gmsgid,
 	}
     }
 
+  /* If we were parsing a string-literal and there is an unknown name
+     token right after, then check to see if that could also have been
+     a literal string by checking the name against a list of known
+     standard string literal constants defined in header files. If
+     there is one, then add that as an hint to the error message. */
+  auto_diagnostic_group d;
+  name_hint h;
+  if (parser->seen_string_literal && token->type == CPP_NAME)
+    {
+      tree name = token->value;
+      const char *header_hint
+	= get_c_stdlib_header_for_name (IDENTIFIER_POINTER (name));
+      if (header_hint != NULL)
+	h = name_hint (NULL,
+		       new suggest_missing_header (token->location,
+						   IDENTIFIER_POINTER (name),
+						   header_hint));
+    }
+
   c_parse_error (gmsgid,
 		 /* Because c_parse_error does not understand
 		    CPP_KEYWORD, keywords are treated like
@@ -7539,6 +7567,7 @@ c_parser_string_literal (c_parser *parser, bool translate, bool wide_ok)
   ret.original_code = STRING_CST;
   ret.original_type = NULL_TREE;
   set_c_expr_source_range (&ret, get_range_from_loc (line_table, loc));
+  parser->seen_string_literal = true;
   return ret;
 }
 
diff --git a/gcc/testsuite/g++.dg/spellcheck-inttypes.C b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
new file mode 100644
index 000000000000..c5861127ca6d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
@@ -0,0 +1,41 @@
+/* { dg-options "-std=c++11" } */
+#include <cstdio>
+#include <cstdint>
+/* Missing <cinttypes>.  */
+
+int8_t i8;
+int16_t i16;
+int32_t i32;
+int64_t i64;
+
+intptr_t ip;
+uintptr_t up;
+
+/* As an identifier.  */
+const char *hex8_fmt = PRIx8; /* { dg-error "'PRIx8' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex16_fmt = PRIx16; /* { dg-error "'PRIx16' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex32_fmt = PRIx32; /* { dg-error "'PRIx32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+
+void test_printf (void)
+{
+  printf ("some format strings %s, %s, %s, %s, %s, %s\n",
+	  PRId8, /* { dg-error "'PRId8' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIi16, /* { dg-error "'PRIi16' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIi16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIo32, /* { dg-error "'PRIo32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIo32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIu64, /* { dg-error "'PRIu64' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIu64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIx32, /* { dg-error "'PRIx32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIoPTR);  /* { dg-error "'PRIoPTR' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+}
diff --git a/gcc/testsuite/gcc.dg/spellcheck-inttypes.c b/gcc/testsuite/gcc.dg/spellcheck-inttypes.c
new file mode 100644
index 000000000000..56dc2f3dcfd9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-inttypes.c
@@ -0,0 +1,78 @@
+/* { dg-options "-std=c99" } */
+#include <stdio.h>
+#include <stdint.h>
+/* Missing <inttypes.h>.  */
+
+int8_t i8;
+int16_t i16;
+int32_t i32;
+int64_t i64;
+
+intptr_t ip;
+uintptr_t up;
+
+/* As an identifier.  */
+const char *hex8_fmt = PRIx8; /* { dg-error "'PRIx8' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex16_fmt = PRIx16; /* { dg-error "'PRIx16' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex32_fmt = PRIx32; /* { dg-error "'PRIx32' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIxPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+/* As a part of a string-literal.  */
+const char *dec8msg_fmt = "Provide %" PRId8 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec16msg_fmt = "Provide %" PRId16 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec32msg_fmt = "Provide %" PRId32 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec64msg_fmt = "Provide %" PRId64 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *decptrmsg_fmt = "Provide %" PRIdPTR "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRIdPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+void test_printf (void)
+{
+  printf ("some format strings %s, %s, %s, %s, %s, %s\n",
+	  PRId8, /* { dg-error "'PRId8' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIi16, /* { dg-error "'PRIi16' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIi16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIo32, /* { dg-error "'PRIo32' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIo32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIu64, /* { dg-error "'PRIu64' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIu64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIoPTR);  /* { dg-error "'PRIoPTR' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIoPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+  printf ("%" PRIo8 "\n", i8); /* { dg-error "expected" } */
+/* { dg-message "'PRIo8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo16 "\n", i16); /* { dg-error "expected" } */
+/* { dg-message "'PRIo16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo32 "\n", i32); /* { dg-error "expected" } */
+/* { dg-message "'PRIo32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo64 "\n", i64); /* { dg-error "expected" } */
+/* { dg-message "'PRIo64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIoPTR "\n", ip); /* { dg-error "expected" } */
+/* { dg-message "'PRIoPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+}
+
+void test_scanf (void)
+{
+  scanf ("%" SCNu8 "\n", &i8); /* { dg-error "expected" } */
+/* { dg-message "'SCNu8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu16 "\n", &i16); /* { dg-error "expected" } */
+/* { dg-message "'SCNu16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu32 "\n", &i32); /* { dg-error "expected" } */
+/* { dg-message "'SCNu32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu64 "\n", &i64); /* { dg-error "expected" } */
+/* { dg-message "'SCNu64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNuPTR "\n", &ip); /* { dg-error "expected" } */
+/* { dg-message "'SCNuPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNxPTR "\n", &up); /* { dg-error "expected" } */
+/* { dg-message "'SCNxPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Provide diagnostic hints for missing inttypes.h string constants.
  2020-05-23  3:01   ` [PATCH] " Mark Wielaard
@ 2020-05-24  0:30     ` Mark Wielaard
  2020-05-24  0:30       ` [PATCH 1/2] Provide diagnostic hints for missing C " Mark Wielaard
  2020-05-24  0:30       ` [PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes " Mark Wielaard
  0 siblings, 2 replies; 11+ messages in thread
From: Mark Wielaard @ 2020-05-24  0:30 UTC (permalink / raw)
  To: gcc-patches

On Sat, May 23, 2020 at 05:01:21AM +0200, Mark Wielaard wrote:
> Yes, that is actually better. And much easier to read. And the code
> can still be shared between get_c_stdlib_header_for_string_macro_name
> and get_stdlib_header_for_name. Changed in the attached patch.
>
> I also extended the testcase so it covers both identifiers and string
> concatenation cases. The identifier hints also work for C++, but I
> haven't yet added the string token concatenation detection to the
> cp_parser. It looks like it can be done similar as done for the
> c_parser, but the parsers are different enough that it seems
> better/simpler to do that in a followup patch once we have this patch
> in the the c_parser.

The cp_parser keeps a stack of tokens, so it was actually easier to
add the same functionality. I kept the patch separate to make review
easier. The c_parser patch is almost the same as what I posted earlier,
it just uses get_c_stdlib_header_for_string_macro_name explicitly. The
cp_parser patch adds get_cpp_stdlib_header_for_string_macro_name and
uses that. The testcase for the C++ case is extended like the c_parser
one.

[PATCH 1/2] Provide diagnostic hints for missing C inttypes.h string
[PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes string

Cheers,

Mark

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] Provide diagnostic hints for missing C inttypes.h string constants.
  2020-05-24  0:30     ` Mark Wielaard
@ 2020-05-24  0:30       ` Mark Wielaard
  2020-06-01 12:01         ` Mark Wielaard
  2020-05-24  0:30       ` [PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes " Mark Wielaard
  1 sibling, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2020-05-24  0:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm, Mark Wielaard

This adds a flag to c_parser so we know when we were trying to
construct a string literal. If there is a parse error and we were
constructing a string literal, and the next token is an unknown
identifier name, and we know there is a standard header that defines
that name as a string literal, then add a missing header hint to
the error messsage.

The list of macro names are also used when providing a hint for
missing identifiers.

gcc/c-family/ChangeLog:

	* known-headers.cc (get_string_macro_hint): New function.
	(get_stdlib_header_for_name): Use get_string_macro_hint.
	(get_c_stdlib_header_for_string_macro_name): New function.
	* known-headers.h (get_c_stdlib_header_for_string_macro_name):
	New function definition.

gcc/c/ChangeLog:

	* c-parser.c (struct c_parser): Add seen_string_literal
	bitfield.
	(c_parser_consume_token): Reset seen_string_literal.
	(c_parser_error_richloc): Add name_hint if seen_string_literal
	and next token is a CPP_NAME and we have a missing header
	suggestion for the name.
	(c_parser_string_literal): Set seen_string_literal.

gcc/testsuite/ChangeLog:

	* gcc.dg/spellcheck-inttypes.c: New test.
	* g++.dg/spellcheck-inttypes.C: Likewise.
---
 gcc/c-family/known-headers.cc              | 53 ++++++++++++++-
 gcc/c-family/known-headers.h               |  2 +
 gcc/c/c-parser.c                           | 29 ++++++++
 gcc/testsuite/g++.dg/spellcheck-inttypes.C | 41 ++++++++++++
 gcc/testsuite/gcc.dg/spellcheck-inttypes.c | 78 ++++++++++++++++++++++
 5 files changed, 202 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-inttypes.C
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-inttypes.c

diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index 1e2bf49c439a..c07cfd1db815 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -46,6 +46,49 @@ struct stdlib_hint
   const char *header[NUM_STDLIBS];
 };
 
+/* Given non-NULL NAME, return the header name defining it (as literal
+   string) within either the standard library (with '<' and '>'), or
+   NULL.
+
+   Only handle string macros, so that this can be used for
+   get_stdlib_header_for_name and
+   get_c_stdlib_header_for_string_macro_name.  */
+
+static const char *
+get_string_macro_hint (const char *name, enum stdlib lib)
+{
+  /* <inttypes.h> and <cinttypes>.  */
+  static const char *c99_cxx11_macros[] =
+    { "PRId8", "PRId16", "PRId32", "PRId64",
+      "PRIi8", "PRIi16", "PRIi32", "PRIi64",
+      "PRIo8", "PRIo16", "PRIo32", "PRIo64",
+      "PRIu8", "PRIu16", "PRIu32", "PRIu64",
+      "PRIx8", "PRIx16", "PRIx32", "PRIx64",
+      "PRIX8", "PRIX16", "PRIX32", "PRIX64",
+
+      "PRIdPTR", "PRIiPTR", "PRIoPTR", "PRIuPTR", "PRIxPTR", "PRIXPTR",
+
+      "SCNd8", "SCNd16", "SCNd32", "SCNd64",
+      "SCNi8", "SCNi16", "SCNi32", "SCNi64",
+      "SCNo8", "SCNo16", "SCNo32", "SCNo64",
+      "SCNu8", "SCNu16", "SCNu32", "SCNu64",
+      "SCNx8", "SCNx16", "SCNx32", "SCNx64",
+
+      "SCNdPTR", "SCNiPTR", "SCNoPTR", "SCNuPTR", "SCNxPTR" };
+
+  if ((lib == STDLIB_C && flag_isoc99)
+      || (lib == STDLIB_CPLUSPLUS && cxx_dialect >= cxx11 ))
+    {
+      const size_t num_c99_cxx11_macros
+	= sizeof (c99_cxx11_macros) / sizeof (c99_cxx11_macros[0]);
+      for (size_t i = 0; i < num_c99_cxx11_macros; i++)
+	if (strcmp (name, c99_cxx11_macros[i]) == 0)
+	  return lib == STDLIB_C ? "<inttypes.h>" : "<cinttypes>";
+    }
+
+  return NULL;
+}
+
 /* Given non-NULL NAME, return the header name defining it within either
    the standard library (with '<' and '>'), or NULL.
    Only handles a subset of the most common names within the stdlibs.  */
@@ -196,7 +239,7 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib)
       if (strcmp (name, c99_cxx11_hints[i].name) == 0)
 	return c99_cxx11_hints[i].header[lib];
 
-  return NULL;
+  return get_string_macro_hint (name, lib);
 }
 
 /* Given non-NULL NAME, return the header name defining it within the C
@@ -217,6 +260,14 @@ get_cp_stdlib_header_for_name (const char *name)
   return get_stdlib_header_for_name (name, STDLIB_CPLUSPLUS);
 }
 
+/* Given non-NULL NAME, return the header name defining a string macro
+   within the C standard library (with '<' and '>'), or NULL.  */
+const char *
+get_c_stdlib_header_for_string_macro_name (const char *name)
+{
+  return get_string_macro_hint (name, STDLIB_C);
+}
+
 /* Implementation of class suggest_missing_header.  */
 
 /* suggest_missing_header's ctor.  */
diff --git a/gcc/c-family/known-headers.h b/gcc/c-family/known-headers.h
index 4ec4e23fc88d..a69bbbf28e76 100644
--- a/gcc/c-family/known-headers.h
+++ b/gcc/c-family/known-headers.h
@@ -23,6 +23,8 @@ along with GCC; see the file COPYING3.  If not see
 extern const char *get_c_stdlib_header_for_name (const char *name);
 extern const char *get_cp_stdlib_header_for_name (const char *name);
 
+extern const char *get_c_stdlib_header_for_string_macro_name (const char *n);
+
 /* Subclass of deferred_diagnostic for suggesting to the user
    that they have missed a #include.  */
 
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 5d11e7e73c16..db8f86a8e2f0 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -69,6 +69,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/name-hint.h"
 #include "tree-iterator.h"
 #include "memmodel.h"
+#include "c-family/known-headers.h"
 
 /* We need to walk over decls with incomplete struct/union/enum types
    after parsing the whole translation unit.
@@ -223,6 +224,13 @@ struct GTY(()) c_parser {
      keywords are valid.  */
   BOOL_BITFIELD objc_property_attr_context : 1;
 
+  /* Whether we have just seen/constructed a string-literal.  Set when
+     returning a string-literal from c_parser_string_literal.  Reset
+     in consume_token.  Useful when we get a parse error and see an
+     unknown token, which could have been a string-literal constant
+     macro.  */
+  BOOL_BITFIELD seen_string_literal : 1;
+
   /* Location of the last consumed token.  */
   location_t last_token_location;
 };
@@ -853,6 +861,7 @@ c_parser_consume_token (c_parser *parser)
         }
     }
   parser->tokens_avail--;
+  parser->seen_string_literal = false;
 }
 
 /* Expect the current token to be a #pragma.  Consume it and remember
@@ -966,6 +975,25 @@ c_parser_error_richloc (c_parser *parser, const char *gmsgid,
 	}
     }
 
+  /* If we were parsing a string-literal and there is an unknown name
+     token right after, then check to see if that could also have been
+     a literal string by checking the name against a list of known
+     standard string literal constants defined in header files. If
+     there is one, then add that as an hint to the error message. */
+  auto_diagnostic_group d;
+  name_hint h;
+  if (parser->seen_string_literal && token->type == CPP_NAME)
+    {
+      tree name = token->value;
+      const char *token_name = IDENTIFIER_POINTER (name);
+      const char *header_hint
+	= get_c_stdlib_header_for_string_macro_name (token_name);
+      if (header_hint != NULL)
+	h = name_hint (NULL, new suggest_missing_header (token->location,
+							 token_name,
+							 header_hint));
+    }
+
   c_parse_error (gmsgid,
 		 /* Because c_parse_error does not understand
 		    CPP_KEYWORD, keywords are treated like
@@ -7539,6 +7567,7 @@ c_parser_string_literal (c_parser *parser, bool translate, bool wide_ok)
   ret.original_code = STRING_CST;
   ret.original_type = NULL_TREE;
   set_c_expr_source_range (&ret, get_range_from_loc (line_table, loc));
+  parser->seen_string_literal = true;
   return ret;
 }
 
diff --git a/gcc/testsuite/g++.dg/spellcheck-inttypes.C b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
new file mode 100644
index 000000000000..c5861127ca6d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
@@ -0,0 +1,41 @@
+/* { dg-options "-std=c++11" } */
+#include <cstdio>
+#include <cstdint>
+/* Missing <cinttypes>.  */
+
+int8_t i8;
+int16_t i16;
+int32_t i32;
+int64_t i64;
+
+intptr_t ip;
+uintptr_t up;
+
+/* As an identifier.  */
+const char *hex8_fmt = PRIx8; /* { dg-error "'PRIx8' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex16_fmt = PRIx16; /* { dg-error "'PRIx16' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex32_fmt = PRIx32; /* { dg-error "'PRIx32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+
+void test_printf (void)
+{
+  printf ("some format strings %s, %s, %s, %s, %s, %s\n",
+	  PRId8, /* { dg-error "'PRId8' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIi16, /* { dg-error "'PRIi16' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIi16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIo32, /* { dg-error "'PRIo32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIo32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIu64, /* { dg-error "'PRIu64' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIu64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIx32, /* { dg-error "'PRIx32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIoPTR);  /* { dg-error "'PRIoPTR' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+}
diff --git a/gcc/testsuite/gcc.dg/spellcheck-inttypes.c b/gcc/testsuite/gcc.dg/spellcheck-inttypes.c
new file mode 100644
index 000000000000..56dc2f3dcfd9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-inttypes.c
@@ -0,0 +1,78 @@
+/* { dg-options "-std=c99" } */
+#include <stdio.h>
+#include <stdint.h>
+/* Missing <inttypes.h>.  */
+
+int8_t i8;
+int16_t i16;
+int32_t i32;
+int64_t i64;
+
+intptr_t ip;
+uintptr_t up;
+
+/* As an identifier.  */
+const char *hex8_fmt = PRIx8; /* { dg-error "'PRIx8' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex16_fmt = PRIx16; /* { dg-error "'PRIx16' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex32_fmt = PRIx32; /* { dg-error "'PRIx32' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIxPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+/* As a part of a string-literal.  */
+const char *dec8msg_fmt = "Provide %" PRId8 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec16msg_fmt = "Provide %" PRId16 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec32msg_fmt = "Provide %" PRId32 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec64msg_fmt = "Provide %" PRId64 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *decptrmsg_fmt = "Provide %" PRIdPTR "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRIdPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+void test_printf (void)
+{
+  printf ("some format strings %s, %s, %s, %s, %s, %s\n",
+	  PRId8, /* { dg-error "'PRId8' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIi16, /* { dg-error "'PRIi16' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIi16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIo32, /* { dg-error "'PRIo32' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIo32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIu64, /* { dg-error "'PRIu64' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIu64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIoPTR);  /* { dg-error "'PRIoPTR' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIoPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+  printf ("%" PRIo8 "\n", i8); /* { dg-error "expected" } */
+/* { dg-message "'PRIo8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo16 "\n", i16); /* { dg-error "expected" } */
+/* { dg-message "'PRIo16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo32 "\n", i32); /* { dg-error "expected" } */
+/* { dg-message "'PRIo32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo64 "\n", i64); /* { dg-error "expected" } */
+/* { dg-message "'PRIo64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIoPTR "\n", ip); /* { dg-error "expected" } */
+/* { dg-message "'PRIoPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+}
+
+void test_scanf (void)
+{
+  scanf ("%" SCNu8 "\n", &i8); /* { dg-error "expected" } */
+/* { dg-message "'SCNu8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu16 "\n", &i16); /* { dg-error "expected" } */
+/* { dg-message "'SCNu16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu32 "\n", &i32); /* { dg-error "expected" } */
+/* { dg-message "'SCNu32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu64 "\n", &i64); /* { dg-error "expected" } */
+/* { dg-message "'SCNu64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNuPTR "\n", &ip); /* { dg-error "expected" } */
+/* { dg-message "'SCNuPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNxPTR "\n", &up); /* { dg-error "expected" } */
+/* { dg-message "'SCNxPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes string constants.
  2020-05-24  0:30     ` Mark Wielaard
  2020-05-24  0:30       ` [PATCH 1/2] Provide diagnostic hints for missing C " Mark Wielaard
@ 2020-05-24  0:30       ` Mark Wielaard
  2020-05-25 16:26         ` Jason Merrill
  1 sibling, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2020-05-24  0:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm, Mark Wielaard

When reporting an error in cp_parser and we notice a string literal
followed by an unknown name check whether there is a known standard
header containing a string macro with the same name, then add a hint
to the error message to include that header.

gcc/c-family/ChangeLog:

	* known-headers.cc (get_cp_stdlib_header_for_string_macro_name):
	New function.
	* known-headers.h (get_c_stdlib_header_for_string_macro_name):
	New function definition.

gcc/cp/ChangeLog:

	* parser.c (cp_lexer_safe_previous_token): New function.
	(cp_parser_error_1): Add name_hint if the previous token is
	a string literal and next token is a CPP_NAME and we have a
	missing header suggestion for the name.

gcc/testsuite/ChangeLog:

	* g++.dg/spellcheck-inttypes.C: Add string-literal testcases.
---
 gcc/c-family/known-headers.cc              |  8 +++++
 gcc/c-family/known-headers.h               |  1 +
 gcc/cp/parser.c                            | 36 ++++++++++++++++++++
 gcc/testsuite/g++.dg/spellcheck-inttypes.C | 39 ++++++++++++++++++++++
 4 files changed, 84 insertions(+)

diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index c07cfd1db815..977230a586db 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -268,6 +268,14 @@ get_c_stdlib_header_for_string_macro_name (const char *name)
   return get_string_macro_hint (name, STDLIB_C);
 }
 
+/* Given non-NULL NAME, return the header name defining a string macro
+   within the C++ standard library (with '<' and '>'), or NULL.  */
+const char *
+get_cp_stdlib_header_for_string_macro_name (const char *name)
+{
+  return get_string_macro_hint (name, STDLIB_CPLUSPLUS);
+}
+
 /* Implementation of class suggest_missing_header.  */
 
 /* suggest_missing_header's ctor.  */
diff --git a/gcc/c-family/known-headers.h b/gcc/c-family/known-headers.h
index a69bbbf28e76..f0c89dc9019d 100644
--- a/gcc/c-family/known-headers.h
+++ b/gcc/c-family/known-headers.h
@@ -24,6 +24,7 @@ extern const char *get_c_stdlib_header_for_name (const char *name);
 extern const char *get_cp_stdlib_header_for_name (const char *name);
 
 extern const char *get_c_stdlib_header_for_string_macro_name (const char *n);
+extern const char *get_cp_stdlib_header_for_string_macro_name (const char *n);
 
 /* Subclass of deferred_diagnostic for suggesting to the user
    that they have missed a #include.  */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 54ca875ce54c..95b8c635fc65 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-iterator.h"
 #include "cp-name-hint.h"
 #include "memmodel.h"
+#include "c-family/known-headers.h"
 
 \f
 /* The lexer.  */
@@ -776,6 +777,20 @@ cp_lexer_previous_token (cp_lexer *lexer)
   return cp_lexer_token_at (lexer, tp);
 }
 
+/* Same as above, but return NULL when the lexer doesn't own the token
+   buffer or if the next_token is at the start of the token
+   vector.  */
+
+static cp_token *
+cp_lexer_safe_previous_token (cp_lexer *lexer)
+{
+  if (lexer->buffer)
+    if (lexer->next_token != lexer->buffer->address ())
+      return cp_lexer_previous_token (lexer);
+
+  return NULL;
+}
+
 /* Overload for make_location, taking the lexer to mean the location of the
    previous token.  */
 
@@ -2919,6 +2934,7 @@ cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
 	}
     }
 
+  auto_diagnostic_group d;
   gcc_rich_location richloc (input_location);
 
   bool added_matching_location = false;
@@ -2941,6 +2957,26 @@ cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
 	  = richloc.add_location_if_nearby (matching_location);
     }
 
+  /* If we were parsing a string-literal and there is an unknown name
+     token right after, then check to see if that could also have been
+     a literal string by checking the name against a list of known
+     standard string literal constants defined in header files. If
+     there is one, then add that as an hint to the error message. */
+  name_hint h;
+  cp_token *prev_token = cp_lexer_safe_previous_token (parser->lexer);
+  if (prev_token && cp_parser_is_string_literal (prev_token)
+      && token->type == CPP_NAME)
+    {
+      tree name = token->u.value;
+      const char *token_name = IDENTIFIER_POINTER (name);
+      const char *header_hint
+	= get_cp_stdlib_header_for_string_macro_name (token_name);
+      if (header_hint != NULL)
+	h = name_hint (NULL, new suggest_missing_header (token->location,
+							 token_name,
+							 header_hint));
+    }
+
   /* Actually emit the error.  */
   c_parse_error (gmsgid,
 		 /* Because c_parser_error does not understand
diff --git a/gcc/testsuite/g++.dg/spellcheck-inttypes.C b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
index c5861127ca6d..84bfc125513c 100644
--- a/gcc/testsuite/g++.dg/spellcheck-inttypes.C
+++ b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
@@ -23,6 +23,18 @@ const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' was not declared" "undec
 const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' was not declared" "undeclared identifier" { target *-*-* } } */
 /* { dg-message "'PRIxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
 
+/* As a part of a string-literal.  */
+const char *dec8msg_fmt = "Provide %" PRId8 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec16msg_fmt = "Provide %" PRId16 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec32msg_fmt = "Provide %" PRId32 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec64msg_fmt = "Provide %" PRId64 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *decptrmsg_fmt = "Provide %" PRIdPTR "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRIdPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+
 void test_printf (void)
 {
   printf ("some format strings %s, %s, %s, %s, %s, %s\n",
@@ -38,4 +50,31 @@ void test_printf (void)
 /* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
 	  PRIoPTR);  /* { dg-error "'PRIoPTR' was not declared" "undeclared identifier" { target *-*-* } } */
 /* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+
+  printf ("%" PRIo8 "\n", i8); /* { dg-error "expected" } */
+/* { dg-message "'PRIo8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo16 "\n", i16); /* { dg-error "expected" } */
+/* { dg-message "'PRIo16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo32 "\n", i32); /* { dg-error "expected" } */
+/* { dg-message "'PRIo32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo64 "\n", i64); /* { dg-error "expected" } */
+/* { dg-message "'PRIo64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIoPTR "\n", ip); /* { dg-error "expected" } */
+/* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+}
+
+void test_scanf (void)
+{
+  scanf ("%" SCNu8 "\n", &i8); /* { dg-error "expected" } */
+/* { dg-message "'SCNu8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu16 "\n", &i16); /* { dg-error "expected" } */
+/* { dg-message "'SCNu16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu32 "\n", &i32); /* { dg-error "expected" } */
+/* { dg-message "'SCNu32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu64 "\n", &i64); /* { dg-error "expected" } */
+/* { dg-message "'SCNu64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNuPTR "\n", &ip); /* { dg-error "expected" } */
+/* { dg-message "'SCNuPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNxPTR "\n", &up); /* { dg-error "expected" } */
+/* { dg-message "'SCNxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
 }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes string constants.
  2020-05-24  0:30       ` [PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes " Mark Wielaard
@ 2020-05-25 16:26         ` Jason Merrill
  2020-05-28 23:33           ` Mark Wielaard
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2020-05-25 16:26 UTC (permalink / raw)
  To: Mark Wielaard, gcc-patches

On 5/23/20 8:30 PM, Mark Wielaard wrote:
> When reporting an error in cp_parser and we notice a string literal
> followed by an unknown name check whether there is a known standard
> header containing a string macro with the same name, then add a hint
> to the error message to include that header.
> 
> gcc/c-family/ChangeLog:
> 
> 	* known-headers.cc (get_cp_stdlib_header_for_string_macro_name):
> 	New function.
> 	* known-headers.h (get_c_stdlib_header_for_string_macro_name):

Missing 'p'.

> 	New function definition.

Declaration, not definition.

The C++ changes are OK with these ChangeLog corrections.

> gcc/cp/ChangeLog:
> 
> 	* parser.c (cp_lexer_safe_previous_token): New function.
> 	(cp_parser_error_1): Add name_hint if the previous token is
> 	a string literal and next token is a CPP_NAME and we have a
> 	missing header suggestion for the name.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/spellcheck-inttypes.C: Add string-literal testcases.
> ---
>   gcc/c-family/known-headers.cc              |  8 +++++
>   gcc/c-family/known-headers.h               |  1 +
>   gcc/cp/parser.c                            | 36 ++++++++++++++++++++
>   gcc/testsuite/g++.dg/spellcheck-inttypes.C | 39 ++++++++++++++++++++++
>   4 files changed, 84 insertions(+)
> 
> diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
> index c07cfd1db815..977230a586db 100644
> --- a/gcc/c-family/known-headers.cc
> +++ b/gcc/c-family/known-headers.cc
> @@ -268,6 +268,14 @@ get_c_stdlib_header_for_string_macro_name (const char *name)
>     return get_string_macro_hint (name, STDLIB_C);
>   }
>   
> +/* Given non-NULL NAME, return the header name defining a string macro
> +   within the C++ standard library (with '<' and '>'), or NULL.  */
> +const char *
> +get_cp_stdlib_header_for_string_macro_name (const char *name)
> +{
> +  return get_string_macro_hint (name, STDLIB_CPLUSPLUS);
> +}
> +
>   /* Implementation of class suggest_missing_header.  */
>   
>   /* suggest_missing_header's ctor.  */
> diff --git a/gcc/c-family/known-headers.h b/gcc/c-family/known-headers.h
> index a69bbbf28e76..f0c89dc9019d 100644
> --- a/gcc/c-family/known-headers.h
> +++ b/gcc/c-family/known-headers.h
> @@ -24,6 +24,7 @@ extern const char *get_c_stdlib_header_for_name (const char *name);
>   extern const char *get_cp_stdlib_header_for_name (const char *name);
>   
>   extern const char *get_c_stdlib_header_for_string_macro_name (const char *n);
> +extern const char *get_cp_stdlib_header_for_string_macro_name (const char *n);
>   
>   /* Subclass of deferred_diagnostic for suggesting to the user
>      that they have missed a #include.  */
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 54ca875ce54c..95b8c635fc65 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "tree-iterator.h"
>   #include "cp-name-hint.h"
>   #include "memmodel.h"
> +#include "c-family/known-headers.h"
>   
>   \f
>   /* The lexer.  */
> @@ -776,6 +777,20 @@ cp_lexer_previous_token (cp_lexer *lexer)
>     return cp_lexer_token_at (lexer, tp);
>   }
>   
> +/* Same as above, but return NULL when the lexer doesn't own the token
> +   buffer or if the next_token is at the start of the token
> +   vector.  */
> +
> +static cp_token *
> +cp_lexer_safe_previous_token (cp_lexer *lexer)
> +{
> +  if (lexer->buffer)
> +    if (lexer->next_token != lexer->buffer->address ())
> +      return cp_lexer_previous_token (lexer);
> +
> +  return NULL;
> +}
> +
>   /* Overload for make_location, taking the lexer to mean the location of the
>      previous token.  */
>   
> @@ -2919,6 +2934,7 @@ cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
>   	}
>       }
>   
> +  auto_diagnostic_group d;
>     gcc_rich_location richloc (input_location);
>   
>     bool added_matching_location = false;
> @@ -2941,6 +2957,26 @@ cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
>   	  = richloc.add_location_if_nearby (matching_location);
>       }
>   
> +  /* If we were parsing a string-literal and there is an unknown name
> +     token right after, then check to see if that could also have been
> +     a literal string by checking the name against a list of known
> +     standard string literal constants defined in header files. If
> +     there is one, then add that as an hint to the error message. */
> +  name_hint h;
> +  cp_token *prev_token = cp_lexer_safe_previous_token (parser->lexer);
> +  if (prev_token && cp_parser_is_string_literal (prev_token)
> +      && token->type == CPP_NAME)
> +    {
> +      tree name = token->u.value;
> +      const char *token_name = IDENTIFIER_POINTER (name);
> +      const char *header_hint
> +	= get_cp_stdlib_header_for_string_macro_name (token_name);
> +      if (header_hint != NULL)
> +	h = name_hint (NULL, new suggest_missing_header (token->location,
> +							 token_name,
> +							 header_hint));
> +    }
> +
>     /* Actually emit the error.  */
>     c_parse_error (gmsgid,
>   		 /* Because c_parser_error does not understand
> diff --git a/gcc/testsuite/g++.dg/spellcheck-inttypes.C b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
> index c5861127ca6d..84bfc125513c 100644
> --- a/gcc/testsuite/g++.dg/spellcheck-inttypes.C
> +++ b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
> @@ -23,6 +23,18 @@ const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' was not declared" "undec
>   const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' was not declared" "undeclared identifier" { target *-*-* } } */
>   /* { dg-message "'PRIxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
>   
> +/* As a part of a string-literal.  */
> +const char *dec8msg_fmt = "Provide %" PRId8 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> +/* { dg-message "'PRId8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +const char *dec16msg_fmt = "Provide %" PRId16 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> +/* { dg-message "'PRId16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +const char *dec32msg_fmt = "Provide %" PRId32 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> +/* { dg-message "'PRId32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +const char *dec64msg_fmt = "Provide %" PRId64 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> +/* { dg-message "'PRId64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +const char *decptrmsg_fmt = "Provide %" PRIdPTR "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> +/* { dg-message "'PRIdPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +
>   void test_printf (void)
>   {
>     printf ("some format strings %s, %s, %s, %s, %s, %s\n",
> @@ -38,4 +50,31 @@ void test_printf (void)
>   /* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
>   	  PRIoPTR);  /* { dg-error "'PRIoPTR' was not declared" "undeclared identifier" { target *-*-* } } */
>   /* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +
> +  printf ("%" PRIo8 "\n", i8); /* { dg-error "expected" } */
> +/* { dg-message "'PRIo8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  printf ("%" PRIo16 "\n", i16); /* { dg-error "expected" } */
> +/* { dg-message "'PRIo16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  printf ("%" PRIo32 "\n", i32); /* { dg-error "expected" } */
> +/* { dg-message "'PRIo32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  printf ("%" PRIo64 "\n", i64); /* { dg-error "expected" } */
> +/* { dg-message "'PRIo64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  printf ("%" PRIoPTR "\n", ip); /* { dg-error "expected" } */
> +/* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +}
> +
> +void test_scanf (void)
> +{
> +  scanf ("%" SCNu8 "\n", &i8); /* { dg-error "expected" } */
> +/* { dg-message "'SCNu8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  scanf ("%" SCNu16 "\n", &i16); /* { dg-error "expected" } */
> +/* { dg-message "'SCNu16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  scanf ("%" SCNu32 "\n", &i32); /* { dg-error "expected" } */
> +/* { dg-message "'SCNu32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  scanf ("%" SCNu64 "\n", &i64); /* { dg-error "expected" } */
> +/* { dg-message "'SCNu64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  scanf ("%" SCNuPTR "\n", &ip); /* { dg-error "expected" } */
> +/* { dg-message "'SCNuPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> +  scanf ("%" SCNxPTR "\n", &up); /* { dg-error "expected" } */
> +/* { dg-message "'SCNxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
>   }
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes string constants.
  2020-05-25 16:26         ` Jason Merrill
@ 2020-05-28 23:33           ` Mark Wielaard
  2020-05-29 12:36             ` David Malcolm
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2020-05-28 23:33 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, David Malcolm

Hi,

On Mon, May 25, 2020 at 12:26:33PM -0400, Jason Merrill wrote:
> On 5/23/20 8:30 PM, Mark Wielaard wrote:
> > When reporting an error in cp_parser and we notice a string literal
> > followed by an unknown name check whether there is a known standard
> > header containing a string macro with the same name, then add a hint
> > to the error message to include that header.
> > 
> > gcc/c-family/ChangeLog:
> > 
> > 	* known-headers.cc (get_cp_stdlib_header_for_string_macro_name):
> > 	New function.
> > 	* known-headers.h (get_c_stdlib_header_for_string_macro_name):
> 
> Missing 'p'.
> 
> > 	New function definition.
> 
> Declaration, not definition.
> 
> The C++ changes are OK with these ChangeLog corrections.

Thanks. David, are you OK with the diagnostic changes?

Who can we trick into reviewing the C frontend changes in the 1/2
patch that this depends on?

Cheers,

Mark

> > gcc/cp/ChangeLog:
> > 
> > 	* parser.c (cp_lexer_safe_previous_token): New function.
> > 	(cp_parser_error_1): Add name_hint if the previous token is
> > 	a string literal and next token is a CPP_NAME and we have a
> > 	missing header suggestion for the name.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/spellcheck-inttypes.C: Add string-literal testcases.
> > ---
> >   gcc/c-family/known-headers.cc              |  8 +++++
> >   gcc/c-family/known-headers.h               |  1 +
> >   gcc/cp/parser.c                            | 36 ++++++++++++++++++++
> >   gcc/testsuite/g++.dg/spellcheck-inttypes.C | 39 ++++++++++++++++++++++
> >   4 files changed, 84 insertions(+)
> > 
> > diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
> > index c07cfd1db815..977230a586db 100644
> > --- a/gcc/c-family/known-headers.cc
> > +++ b/gcc/c-family/known-headers.cc
> > @@ -268,6 +268,14 @@ get_c_stdlib_header_for_string_macro_name (const char *name)
> >     return get_string_macro_hint (name, STDLIB_C);
> >   }
> > +/* Given non-NULL NAME, return the header name defining a string macro
> > +   within the C++ standard library (with '<' and '>'), or NULL.  */
> > +const char *
> > +get_cp_stdlib_header_for_string_macro_name (const char *name)
> > +{
> > +  return get_string_macro_hint (name, STDLIB_CPLUSPLUS);
> > +}
> > +
> >   /* Implementation of class suggest_missing_header.  */
> >   /* suggest_missing_header's ctor.  */
> > diff --git a/gcc/c-family/known-headers.h b/gcc/c-family/known-headers.h
> > index a69bbbf28e76..f0c89dc9019d 100644
> > --- a/gcc/c-family/known-headers.h
> > +++ b/gcc/c-family/known-headers.h
> > @@ -24,6 +24,7 @@ extern const char *get_c_stdlib_header_for_name (const char *name);
> >   extern const char *get_cp_stdlib_header_for_name (const char *name);
> >   extern const char *get_c_stdlib_header_for_string_macro_name (const char *n);
> > +extern const char *get_cp_stdlib_header_for_string_macro_name (const char *n);
> >   /* Subclass of deferred_diagnostic for suggesting to the user
> >      that they have missed a #include.  */
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 54ca875ce54c..95b8c635fc65 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
> >   #include "tree-iterator.h"
> >   #include "cp-name-hint.h"
> >   #include "memmodel.h"
> > +#include "c-family/known-headers.h"
> >   \f
> >   /* The lexer.  */
> > @@ -776,6 +777,20 @@ cp_lexer_previous_token (cp_lexer *lexer)
> >     return cp_lexer_token_at (lexer, tp);
> >   }
> > +/* Same as above, but return NULL when the lexer doesn't own the token
> > +   buffer or if the next_token is at the start of the token
> > +   vector.  */
> > +
> > +static cp_token *
> > +cp_lexer_safe_previous_token (cp_lexer *lexer)
> > +{
> > +  if (lexer->buffer)
> > +    if (lexer->next_token != lexer->buffer->address ())
> > +      return cp_lexer_previous_token (lexer);
> > +
> > +  return NULL;
> > +}
> > +
> >   /* Overload for make_location, taking the lexer to mean the location of the
> >      previous token.  */
> > @@ -2919,6 +2934,7 @@ cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
> >   	}
> >       }
> > +  auto_diagnostic_group d;
> >     gcc_rich_location richloc (input_location);
> >     bool added_matching_location = false;
> > @@ -2941,6 +2957,26 @@ cp_parser_error_1 (cp_parser* parser, const char* gmsgid,
> >   	  = richloc.add_location_if_nearby (matching_location);
> >       }
> > +  /* If we were parsing a string-literal and there is an unknown name
> > +     token right after, then check to see if that could also have been
> > +     a literal string by checking the name against a list of known
> > +     standard string literal constants defined in header files. If
> > +     there is one, then add that as an hint to the error message. */
> > +  name_hint h;
> > +  cp_token *prev_token = cp_lexer_safe_previous_token (parser->lexer);
> > +  if (prev_token && cp_parser_is_string_literal (prev_token)
> > +      && token->type == CPP_NAME)
> > +    {
> > +      tree name = token->u.value;
> > +      const char *token_name = IDENTIFIER_POINTER (name);
> > +      const char *header_hint
> > +	= get_cp_stdlib_header_for_string_macro_name (token_name);
> > +      if (header_hint != NULL)
> > +	h = name_hint (NULL, new suggest_missing_header (token->location,
> > +							 token_name,
> > +							 header_hint));
> > +    }
> > +
> >     /* Actually emit the error.  */
> >     c_parse_error (gmsgid,
> >   		 /* Because c_parser_error does not understand
> > diff --git a/gcc/testsuite/g++.dg/spellcheck-inttypes.C b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
> > index c5861127ca6d..84bfc125513c 100644
> > --- a/gcc/testsuite/g++.dg/spellcheck-inttypes.C
> > +++ b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
> > @@ -23,6 +23,18 @@ const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' was not declared" "undec
> >   const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' was not declared" "undeclared identifier" { target *-*-* } } */
> >   /* { dg-message "'PRIxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +/* As a part of a string-literal.  */
> > +const char *dec8msg_fmt = "Provide %" PRId8 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> > +/* { dg-message "'PRId8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +const char *dec16msg_fmt = "Provide %" PRId16 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> > +/* { dg-message "'PRId16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +const char *dec32msg_fmt = "Provide %" PRId32 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> > +/* { dg-message "'PRId32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +const char *dec64msg_fmt = "Provide %" PRId64 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> > +/* { dg-message "'PRId64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +const char *decptrmsg_fmt = "Provide %" PRIdPTR "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
> > +/* { dg-message "'PRIdPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +
> >   void test_printf (void)
> >   {
> >     printf ("some format strings %s, %s, %s, %s, %s, %s\n",
> > @@ -38,4 +50,31 @@ void test_printf (void)
> >   /* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> >   	  PRIoPTR);  /* { dg-error "'PRIoPTR' was not declared" "undeclared identifier" { target *-*-* } } */
> >   /* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +
> > +  printf ("%" PRIo8 "\n", i8); /* { dg-error "expected" } */
> > +/* { dg-message "'PRIo8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  printf ("%" PRIo16 "\n", i16); /* { dg-error "expected" } */
> > +/* { dg-message "'PRIo16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  printf ("%" PRIo32 "\n", i32); /* { dg-error "expected" } */
> > +/* { dg-message "'PRIo32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  printf ("%" PRIo64 "\n", i64); /* { dg-error "expected" } */
> > +/* { dg-message "'PRIo64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  printf ("%" PRIoPTR "\n", ip); /* { dg-error "expected" } */
> > +/* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +}
> > +
> > +void test_scanf (void)
> > +{
> > +  scanf ("%" SCNu8 "\n", &i8); /* { dg-error "expected" } */
> > +/* { dg-message "'SCNu8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  scanf ("%" SCNu16 "\n", &i16); /* { dg-error "expected" } */
> > +/* { dg-message "'SCNu16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  scanf ("%" SCNu32 "\n", &i32); /* { dg-error "expected" } */
> > +/* { dg-message "'SCNu32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  scanf ("%" SCNu64 "\n", &i64); /* { dg-error "expected" } */
> > +/* { dg-message "'SCNu64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  scanf ("%" SCNuPTR "\n", &ip); /* { dg-error "expected" } */
> > +/* { dg-message "'SCNuPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> > +  scanf ("%" SCNxPTR "\n", &up); /* { dg-error "expected" } */
> > +/* { dg-message "'SCNxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
> >   }
> > 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes string constants.
  2020-05-28 23:33           ` Mark Wielaard
@ 2020-05-29 12:36             ` David Malcolm
  0 siblings, 0 replies; 11+ messages in thread
From: David Malcolm @ 2020-05-29 12:36 UTC (permalink / raw)
  To: Mark Wielaard, Jason Merrill; +Cc: gcc-patches

On Fri, 2020-05-29 at 01:33 +0200, Mark Wielaard wrote:
> Hi,
> 
> On Mon, May 25, 2020 at 12:26:33PM -0400, Jason Merrill wrote:
> > On 5/23/20 8:30 PM, Mark Wielaard wrote:
> > > When reporting an error in cp_parser and we notice a string
> > > literal
> > > followed by an unknown name check whether there is a known
> > > standard
> > > header containing a string macro with the same name, then add a
> > > hint
> > > to the error message to include that header.
> > > 
> > > gcc/c-family/ChangeLog:
> > > 
> > > 	* known-headers.cc
> > > (get_cp_stdlib_header_for_string_macro_name):
> > > 	New function.
> > > 	* known-headers.h (get_c_stdlib_header_for_string_macro_name):
> > 
> > Missing 'p'.
> > 
> > > 	New function definition.
> > 
> > Declaration, not definition.
> > 
> > The C++ changes are OK with these ChangeLog corrections.
> 
> Thanks. David, are you OK with the diagnostic changes?

Yes.

> Who can we trick into reviewing the C frontend changes in the 1/2
> patch that this depends on?
> 
> Cheers,
> 
> Mark



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] Provide diagnostic hints for missing C inttypes.h string constants.
  2020-05-24  0:30       ` [PATCH 1/2] Provide diagnostic hints for missing C " Mark Wielaard
@ 2020-06-01 12:01         ` Mark Wielaard
  2020-06-03 23:17           ` Joseph Myers
  0 siblings, 1 reply; 11+ messages in thread
From: Mark Wielaard @ 2020-06-01 12:01 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

On Sun, May 24, 2020 at 02:30:13AM +0200, Mark Wielaard wrote:
> This adds a flag to c_parser so we know when we were trying to
> construct a string literal. If there is a parse error and we were
> constructing a string literal, and the next token is an unknown
> identifier name, and we know there is a standard header that defines
> that name as a string literal, then add a missing header hint to
> the error messsage.
> 
> The list of macro names are also used when providing a hint for
> missing identifiers.

Ping. Note the followup patch that introduces the same functionality
for the C++ parser was already approved. This patch (as attached) only
needs review/approval from a C-frontend maintainer for some of the
gcc/c/c-parser.c bits.

Thanks,

Mark

[-- Attachment #2: 0001-Provide-diagnostic-hints-for-missing-C-inttypes.h-st.patch --]
[-- Type: text/x-diff, Size: 18249 bytes --]

From 1aceca275a73b4c7991a6fbde45f4d6da1a9daf5 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Fri, 22 May 2020 01:10:50 +0200
Subject: [PATCH] Provide diagnostic hints for missing C inttypes.h string
 constants.

This adds a flag to c_parser so we know when we were trying to
construct a string literal. If there is a parse error and we were
constructing a string literal, and the next token is an unknown
identifier name, and we know there is a standard header that defines
that name as a string literal, then add a missing header hint to
the error messsage.

The list of macro names are also used when providing a hint for
missing identifiers.

gcc/c-family/ChangeLog:

	* known-headers.cc (get_string_macro_hint): New function.
	(get_stdlib_header_for_name): Use get_string_macro_hint.
	(get_c_stdlib_header_for_string_macro_name): New function.
	* known-headers.h (get_c_stdlib_header_for_string_macro_name):
	New function declaration.

gcc/c/ChangeLog:

	* c-parser.c (struct c_parser): Add seen_string_literal
	bitfield.
	(c_parser_consume_token): Reset seen_string_literal.
	(c_parser_error_richloc): Add name_hint if seen_string_literal
	and next token is a CPP_NAME and we have a missing header
	suggestion for the name.
	(c_parser_string_literal): Set seen_string_literal.

gcc/testsuite/ChangeLog:

	* gcc.dg/spellcheck-inttypes.c: New test.
	* g++.dg/spellcheck-inttypes.C: Likewise.
---
 gcc/c-family/known-headers.cc              | 53 ++++++++++++++-
 gcc/c-family/known-headers.h               |  2 +
 gcc/c/c-parser.c                           | 29 ++++++++
 gcc/testsuite/g++.dg/spellcheck-inttypes.C | 41 ++++++++++++
 gcc/testsuite/gcc.dg/spellcheck-inttypes.c | 78 ++++++++++++++++++++++
 5 files changed, 202 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-inttypes.C
 create mode 100644 gcc/testsuite/gcc.dg/spellcheck-inttypes.c

diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index 1e2bf49c439a..c07cfd1db815 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -46,6 +46,49 @@ struct stdlib_hint
   const char *header[NUM_STDLIBS];
 };
 
+/* Given non-NULL NAME, return the header name defining it (as literal
+   string) within either the standard library (with '<' and '>'), or
+   NULL.
+
+   Only handle string macros, so that this can be used for
+   get_stdlib_header_for_name and
+   get_c_stdlib_header_for_string_macro_name.  */
+
+static const char *
+get_string_macro_hint (const char *name, enum stdlib lib)
+{
+  /* <inttypes.h> and <cinttypes>.  */
+  static const char *c99_cxx11_macros[] =
+    { "PRId8", "PRId16", "PRId32", "PRId64",
+      "PRIi8", "PRIi16", "PRIi32", "PRIi64",
+      "PRIo8", "PRIo16", "PRIo32", "PRIo64",
+      "PRIu8", "PRIu16", "PRIu32", "PRIu64",
+      "PRIx8", "PRIx16", "PRIx32", "PRIx64",
+      "PRIX8", "PRIX16", "PRIX32", "PRIX64",
+
+      "PRIdPTR", "PRIiPTR", "PRIoPTR", "PRIuPTR", "PRIxPTR", "PRIXPTR",
+
+      "SCNd8", "SCNd16", "SCNd32", "SCNd64",
+      "SCNi8", "SCNi16", "SCNi32", "SCNi64",
+      "SCNo8", "SCNo16", "SCNo32", "SCNo64",
+      "SCNu8", "SCNu16", "SCNu32", "SCNu64",
+      "SCNx8", "SCNx16", "SCNx32", "SCNx64",
+
+      "SCNdPTR", "SCNiPTR", "SCNoPTR", "SCNuPTR", "SCNxPTR" };
+
+  if ((lib == STDLIB_C && flag_isoc99)
+      || (lib == STDLIB_CPLUSPLUS && cxx_dialect >= cxx11 ))
+    {
+      const size_t num_c99_cxx11_macros
+	= sizeof (c99_cxx11_macros) / sizeof (c99_cxx11_macros[0]);
+      for (size_t i = 0; i < num_c99_cxx11_macros; i++)
+	if (strcmp (name, c99_cxx11_macros[i]) == 0)
+	  return lib == STDLIB_C ? "<inttypes.h>" : "<cinttypes>";
+    }
+
+  return NULL;
+}
+
 /* Given non-NULL NAME, return the header name defining it within either
    the standard library (with '<' and '>'), or NULL.
    Only handles a subset of the most common names within the stdlibs.  */
@@ -196,7 +239,7 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib)
       if (strcmp (name, c99_cxx11_hints[i].name) == 0)
 	return c99_cxx11_hints[i].header[lib];
 
-  return NULL;
+  return get_string_macro_hint (name, lib);
 }
 
 /* Given non-NULL NAME, return the header name defining it within the C
@@ -217,6 +260,14 @@ get_cp_stdlib_header_for_name (const char *name)
   return get_stdlib_header_for_name (name, STDLIB_CPLUSPLUS);
 }
 
+/* Given non-NULL NAME, return the header name defining a string macro
+   within the C standard library (with '<' and '>'), or NULL.  */
+const char *
+get_c_stdlib_header_for_string_macro_name (const char *name)
+{
+  return get_string_macro_hint (name, STDLIB_C);
+}
+
 /* Implementation of class suggest_missing_header.  */
 
 /* suggest_missing_header's ctor.  */
diff --git a/gcc/c-family/known-headers.h b/gcc/c-family/known-headers.h
index 4ec4e23fc88d..a69bbbf28e76 100644
--- a/gcc/c-family/known-headers.h
+++ b/gcc/c-family/known-headers.h
@@ -23,6 +23,8 @@ along with GCC; see the file COPYING3.  If not see
 extern const char *get_c_stdlib_header_for_name (const char *name);
 extern const char *get_cp_stdlib_header_for_name (const char *name);
 
+extern const char *get_c_stdlib_header_for_string_macro_name (const char *n);
+
 /* Subclass of deferred_diagnostic for suggesting to the user
    that they have missed a #include.  */
 
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 23d6fa22b685..df0b59f9f95c 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -69,6 +69,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/name-hint.h"
 #include "tree-iterator.h"
 #include "memmodel.h"
+#include "c-family/known-headers.h"
 
 /* We need to walk over decls with incomplete struct/union/enum types
    after parsing the whole translation unit.
@@ -223,6 +224,13 @@ struct GTY(()) c_parser {
      keywords are valid.  */
   BOOL_BITFIELD objc_property_attr_context : 1;
 
+  /* Whether we have just seen/constructed a string-literal.  Set when
+     returning a string-literal from c_parser_string_literal.  Reset
+     in consume_token.  Useful when we get a parse error and see an
+     unknown token, which could have been a string-literal constant
+     macro.  */
+  BOOL_BITFIELD seen_string_literal : 1;
+
   /* Location of the last consumed token.  */
   location_t last_token_location;
 };
@@ -853,6 +861,7 @@ c_parser_consume_token (c_parser *parser)
         }
     }
   parser->tokens_avail--;
+  parser->seen_string_literal = false;
 }
 
 /* Expect the current token to be a #pragma.  Consume it and remember
@@ -966,6 +975,25 @@ c_parser_error_richloc (c_parser *parser, const char *gmsgid,
 	}
     }
 
+  /* If we were parsing a string-literal and there is an unknown name
+     token right after, then check to see if that could also have been
+     a literal string by checking the name against a list of known
+     standard string literal constants defined in header files. If
+     there is one, then add that as an hint to the error message. */
+  auto_diagnostic_group d;
+  name_hint h;
+  if (parser->seen_string_literal && token->type == CPP_NAME)
+    {
+      tree name = token->value;
+      const char *token_name = IDENTIFIER_POINTER (name);
+      const char *header_hint
+	= get_c_stdlib_header_for_string_macro_name (token_name);
+      if (header_hint != NULL)
+	h = name_hint (NULL, new suggest_missing_header (token->location,
+							 token_name,
+							 header_hint));
+    }
+
   c_parse_error (gmsgid,
 		 /* Because c_parse_error does not understand
 		    CPP_KEYWORD, keywords are treated like
@@ -7539,6 +7567,7 @@ c_parser_string_literal (c_parser *parser, bool translate, bool wide_ok)
   ret.original_code = STRING_CST;
   ret.original_type = NULL_TREE;
   set_c_expr_source_range (&ret, get_range_from_loc (line_table, loc));
+  parser->seen_string_literal = true;
   return ret;
 }
 
diff --git a/gcc/testsuite/g++.dg/spellcheck-inttypes.C b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
new file mode 100644
index 000000000000..c5861127ca6d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-inttypes.C
@@ -0,0 +1,41 @@
+/* { dg-options "-std=c++11" } */
+#include <cstdio>
+#include <cstdint>
+/* Missing <cinttypes>.  */
+
+int8_t i8;
+int16_t i16;
+int32_t i32;
+int64_t i64;
+
+intptr_t ip;
+uintptr_t up;
+
+/* As an identifier.  */
+const char *hex8_fmt = PRIx8; /* { dg-error "'PRIx8' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex16_fmt = PRIx16; /* { dg-error "'PRIx16' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex32_fmt = PRIx32; /* { dg-error "'PRIx32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIxPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+
+void test_printf (void)
+{
+  printf ("some format strings %s, %s, %s, %s, %s, %s\n",
+	  PRId8, /* { dg-error "'PRId8' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIi16, /* { dg-error "'PRIi16' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIi16' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIo32, /* { dg-error "'PRIo32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIo32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIu64, /* { dg-error "'PRIu64' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIu64' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIx32, /* { dg-error "'PRIx32' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIoPTR);  /* { dg-error "'PRIoPTR' was not declared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIoPTR' is defined in header '<cinttypes>'; did you forget to '#include <cinttypes>'?" "replacement note" { target *-*-* } .-1 } */
+}
diff --git a/gcc/testsuite/gcc.dg/spellcheck-inttypes.c b/gcc/testsuite/gcc.dg/spellcheck-inttypes.c
new file mode 100644
index 000000000000..56dc2f3dcfd9
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-inttypes.c
@@ -0,0 +1,78 @@
+/* { dg-options "-std=c99" } */
+#include <stdio.h>
+#include <stdint.h>
+/* Missing <inttypes.h>.  */
+
+int8_t i8;
+int16_t i16;
+int32_t i32;
+int64_t i64;
+
+intptr_t ip;
+uintptr_t up;
+
+/* As an identifier.  */
+const char *hex8_fmt = PRIx8; /* { dg-error "'PRIx8' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex16_fmt = PRIx16; /* { dg-error "'PRIx16' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex32_fmt = PRIx32; /* { dg-error "'PRIx32' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hex64_fmt = PRIx64; /* { dg-error "'PRIx64' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIx64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *hexptr_fmt = PRIxPTR; /* { dg-error "'PRIxPTR' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIxPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+/* As a part of a string-literal.  */
+const char *dec8msg_fmt = "Provide %" PRId8 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec16msg_fmt = "Provide %" PRId16 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec32msg_fmt = "Provide %" PRId32 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *dec64msg_fmt = "Provide %" PRId64 "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRId64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+const char *decptrmsg_fmt = "Provide %" PRIdPTR "\n"; /* { dg-error "expected" "expected string-literal" { target *-*-* } } */
+/* { dg-message "'PRIdPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+void test_printf (void)
+{
+  printf ("some format strings %s, %s, %s, %s, %s, %s\n",
+	  PRId8, /* { dg-error "'PRId8' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRId8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIi16, /* { dg-error "'PRIi16' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIi16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIo32, /* { dg-error "'PRIo32' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIo32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIu64, /* { dg-error "'PRIu64' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIu64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+	  PRIoPTR);  /* { dg-error "'PRIoPTR' undeclared" "undeclared identifier" { target *-*-* } } */
+/* { dg-message "'PRIoPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+
+  printf ("%" PRIo8 "\n", i8); /* { dg-error "expected" } */
+/* { dg-message "'PRIo8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo16 "\n", i16); /* { dg-error "expected" } */
+/* { dg-message "'PRIo16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo32 "\n", i32); /* { dg-error "expected" } */
+/* { dg-message "'PRIo32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIo64 "\n", i64); /* { dg-error "expected" } */
+/* { dg-message "'PRIo64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  printf ("%" PRIoPTR "\n", ip); /* { dg-error "expected" } */
+/* { dg-message "'PRIoPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+}
+
+void test_scanf (void)
+{
+  scanf ("%" SCNu8 "\n", &i8); /* { dg-error "expected" } */
+/* { dg-message "'SCNu8' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu16 "\n", &i16); /* { dg-error "expected" } */
+/* { dg-message "'SCNu16' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu32 "\n", &i32); /* { dg-error "expected" } */
+/* { dg-message "'SCNu32' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNu64 "\n", &i64); /* { dg-error "expected" } */
+/* { dg-message "'SCNu64' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNuPTR "\n", &ip); /* { dg-error "expected" } */
+/* { dg-message "'SCNuPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+  scanf ("%" SCNxPTR "\n", &up); /* { dg-error "expected" } */
+/* { dg-message "'SCNxPTR' is defined in header '<inttypes.h>'; did you forget to '#include <inttypes.h>'?" "replacement note" { target *-*-* } .-1 } */
+}
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] Provide diagnostic hints for missing C inttypes.h string constants.
  2020-06-01 12:01         ` Mark Wielaard
@ 2020-06-03 23:17           ` Joseph Myers
  0 siblings, 0 replies; 11+ messages in thread
From: Joseph Myers @ 2020-06-03 23:17 UTC (permalink / raw)
  To: Mark Wielaard; +Cc: gcc-patches

On Mon, 1 Jun 2020, Mark Wielaard wrote:

> On Sun, May 24, 2020 at 02:30:13AM +0200, Mark Wielaard wrote:
> > This adds a flag to c_parser so we know when we were trying to
> > construct a string literal. If there is a parse error and we were
> > constructing a string literal, and the next token is an unknown
> > identifier name, and we know there is a standard header that defines
> > that name as a string literal, then add a missing header hint to
> > the error messsage.
> > 
> > The list of macro names are also used when providing a hint for
> > missing identifiers.
> 
> Ping. Note the followup patch that introduces the same functionality
> for the C++ parser was already approved. This patch (as attached) only
> needs review/approval from a C-frontend maintainer for some of the
> gcc/c/c-parser.c bits.

This patch is OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-06-03 23:17 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 23:30 RFC: Provide diagnostic hints for missing inttypes.h string constants Mark Wielaard
2020-05-22 13:42 ` David Malcolm
2020-05-23  3:01   ` [PATCH] " Mark Wielaard
2020-05-24  0:30     ` Mark Wielaard
2020-05-24  0:30       ` [PATCH 1/2] Provide diagnostic hints for missing C " Mark Wielaard
2020-06-01 12:01         ` Mark Wielaard
2020-06-03 23:17           ` Joseph Myers
2020-05-24  0:30       ` [PATCH 2/2] Provide diagnostic hints for missing C++ cinttypes " Mark Wielaard
2020-05-25 16:26         ` Jason Merrill
2020-05-28 23:33           ` Mark Wielaard
2020-05-29 12:36             ` 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).