Re: [PATCH] pch: Specify reason of -Winvalid-pch warning [PR86674]
Hello! On Wed, 2020-03-25 at 16:46 -0600, Jeff Law wrote: > On Mon, 2020-03-09 at 11:55 +0300, Nicholas Guriev wrote: > > gcc/c-family/ChangeLog: > > > > PR pch/86674 > > * c-pch.c (c_common_valid_pch): Use cpp_warning with CPP_W_INVALID_PCH > > reason to fix -Werror=invalid-pch and -Wno-error=invalid-pch switches. > THanks. This looks pretty reasonable, but I'm going to queue it for gcc-11. What's the status of the patch? Is it good enough for accepting? I don't see it in the master branch of the GCC repository. Is it needed anything more from my side? I'm new for GCC development yet and perhaps missed something... signature.asc Description: This is a digitally signed message part
[PATCH] pch: Specify reason of -Winvalid-pch warning [PR86674]
gcc/c-family/ChangeLog:
PR pch/86674
* c-pch.c (c_common_valid_pch): Use cpp_warning with CPP_W_INVALID_PCH
reason to fix -Werror=invalid-pch and -Wno-error=invalid-pch switches.
---
gcc/c-family/ChangeLog | 6 ++
gcc/c-family/c-pch.c | 40 +++-
libcpp/files.c | 2 +-
3 files changed, 22 insertions(+), 26 deletions(-)
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 2e11b..1c83eeb0f 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,9 @@
+2020-03-09 Nicholas Guriev
+
+ PR pch/86674
+ * c-pch.c (c_common_valid_pch): Use cpp_warning with CPP_W_INVALID_PCH
+ reason to fix -Werror=invalid-pch and -Wno-error=invalid-pch switches.
+
2020-03-02 Marek Polacek
PR c++/93958 - add missing -std=gnu++20.
diff --git a/gcc/c-family/c-pch.c b/gcc/c-family/c-pch.c
index a2292f46a..9c0bd0b63 100644
--- a/gcc/c-family/c-pch.c
+++ b/gcc/c-family/c-pch.c
@@ -211,8 +211,7 @@ c_common_valid_pch (cpp_reader *pfile, const char *name,
int fd)
fatal_error (input_location, "cannot read %s: %m", name);
else if (sizeread != IDENT_LENGTH + 16)
{
- if (cpp_get_options (pfile)->warn_invalid_pch)
- cpp_error (pfile, CPP_DL_WARNING, "%s: too short to be a PCH file",
+ cpp_warning (pfile, CPP_W_INVALID_PCH, "%s: too short to be a PCH file",
name);
return 2;
}
@@ -220,27 +219,22 @@ c_common_valid_pch (cpp_reader *pfile, const char *name,
int fd)
pch_ident = get_ident();
if (memcmp (ident, pch_ident, IDENT_LENGTH) != 0)
{
- if (cpp_get_options (pfile)->warn_invalid_pch)
- {
- if (memcmp (ident, pch_ident, 5) == 0)
- /* It's a PCH, for the right language, but has the wrong version.
-*/
- cpp_error (pfile, CPP_DL_WARNING,
+ if (memcmp (ident, pch_ident, 5) == 0)
+ /* It's a PCH, for the right language, but has the wrong version. */
+ cpp_warning (pfile, CPP_W_INVALID_PCH,
"%s: not compatible with this GCC version", name);
- else if (memcmp (ident, pch_ident, 4) == 0)
- /* It's a PCH for the wrong language. */
- cpp_error (pfile, CPP_DL_WARNING, "%s: not for %s", name,
+ else if (memcmp (ident, pch_ident, 4) == 0)
+ /* It's a PCH for the wrong language. */
+ cpp_warning (pfile, CPP_W_INVALID_PCH, "%s: not for %s", name,
lang_hooks.name);
- else
- /* Not any kind of PCH. */
- cpp_error (pfile, CPP_DL_WARNING, "%s: not a PCH file", name);
- }
+ else
+ /* Not any kind of PCH. */
+ cpp_warning (pfile, CPP_W_INVALID_PCH, "%s: not a PCH file", name);
return 2;
}
if (memcmp (ident + IDENT_LENGTH, executable_checksum, 16) != 0)
{
- if (cpp_get_options (pfile)->warn_invalid_pch)
- cpp_error (pfile, CPP_DL_WARNING,
+ cpp_warning (pfile, CPP_W_INVALID_PCH,
"%s: created by a different GCC executable", name);
return 2;
}
@@ -257,8 +251,7 @@ c_common_valid_pch (cpp_reader *pfile, const char *name,
int fd)
if (v.debug_info_type != write_symbols
&& write_symbols != NO_DEBUG)
{
- if (cpp_get_options (pfile)->warn_invalid_pch)
- cpp_error (pfile, CPP_DL_WARNING,
+ cpp_warning (pfile, CPP_W_INVALID_PCH,
"%s: created with -g%s, but used with -g%s", name,
debug_type_names[v.debug_info_type],
debug_type_names[write_symbols]);
@@ -271,8 +264,7 @@ c_common_valid_pch (cpp_reader *pfile, const char *name,
int fd)
for (i = 0; i < MATCH_SIZE; i++)
if (*pch_matching[i].flag_var != v.match[i])
{
- if (cpp_get_options (pfile)->warn_invalid_pch)
- cpp_error (pfile, CPP_DL_WARNING,
+ cpp_warning (pfile, CPP_W_INVALID_PCH,
"%s: settings for %s do not match", name,
pch_matching[i].flag_name);
return 2;
@@ -287,8 +279,7 @@ c_common_valid_pch (cpp_reader *pfile, const char *name,
int fd)
check one function. */
if (v.pch_init != &pch_init)
{
- if (cpp_get_options (pfile)->warn_invalid_pch)
- cpp_error (pfile, CPP_DL_WARNING,
+ cpp_warning (pfile, CPP_W_INVALID_PCH,
"%s: had text segment at different address", name);
return 2;
}
@@ -305,8 +296,7 @@ c_common_valid_pch (cpp_reader *pfile, const char *name,
int fd)
free (this_file_data);
if (msg != NULL)
{
- if (cpp_get_options (pfile)->warn_invalid_pch)
- cpp_error (pfile, CPP_DL_WARNING, "%s: %s", name, msg);
+ cpp_warning (pfile, CPP_W_I
[PATCH] opts: Do not stop when unknown -Wno-error value encountered [PR/65403]
gcc
PR c/65403
* opts.c (enable_warning_as_error): Do not enable errors for
negative options. Added parameter orig_text for better diagnose
mistakes in options.
gcc/testsuite
* gcc.dg/Werror-13.c: Check a note about misspelled no-warning
tag rather than a hard error.
* gcc.dg/Wno-error-1.c, gcc.dg/Wno-error-2.c: New group of tests
for parametrized -Wno-error options.
Signed-off-by: Nicholas Guriev
---
I am CC'ing Alex Henrie because he already worked on fixing this bug and
he may be interested in my solution.
gcc/opts.c | 48 --
gcc/testsuite/gcc.dg/Werror-13.c | 2 +-
gcc/testsuite/gcc.dg/Wno-error-1.c | 6
gcc/testsuite/gcc.dg/Wno-error-2.c | 8 +
4 files changed, 40 insertions(+), 24 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wno-error-1.c
create mode 100644 gcc/testsuite/gcc.dg/Wno-error-2.c
diff --git a/gcc/opts.c b/gcc/opts.c
index 52e9e3a9df9..9ec0846198c 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -291,12 +291,10 @@ static void decode_d_option (const char *arg, struct
gcc_options *opts,
static void set_unsafe_math_optimizations_flags (struct gcc_options *opts,
int set);
static void enable_warning_as_error (const char *arg, int value,
-unsigned int lang_mask,
-const struct cl_option_handlers *handlers,
-struct gcc_options *opts,
-struct gcc_options *opts_set,
-location_t loc,
-diagnostic_context *dc);
+const char *orig_text, unsigned lang_mask,
+const cl_option_handlers *handlers,
+gcc_options *opts, gcc_options *opts_set,
+location_t loc, diagnostic_context *dc);
/* Handle a back-end option; arguments and return value as for
handle_option. */
@@ -2526,8 +2524,8 @@ common_handle_option (struct gcc_options *opts,
if (lang_mask == CL_DRIVER)
break;
- enable_warning_as_error (arg, value, lang_mask, handlers,
- opts, opts_set, loc, dc);
+ enable_warning_as_error (arg, value, decoded->orig_option_with_args_text,
+ lang_mask, handlers, opts, opts_set, loc, dc);
break;
case OPT_Wfatal_errors:
@@ -3236,10 +3234,9 @@ decode_d_option (const char *arg, struct gcc_options
*opts,
NULL), location LOC. This is used by -Werror=. */
static void
-enable_warning_as_error (const char *arg, int value, unsigned int lang_mask,
-const struct cl_option_handlers *handlers,
-struct gcc_options *opts,
-struct gcc_options *opts_set,
+enable_warning_as_error (const char *arg, int value, const char *orig_text,
+unsigned lang_mask, const cl_option_handlers *handlers,
+gcc_options *opts, gcc_options *opts_set,
location_t loc, diagnostic_context *dc)
{
char *new_option;
@@ -3251,19 +3248,24 @@ enable_warning_as_error (const char *arg, int value,
unsigned int lang_mask,
option_index = find_opt (new_option, lang_mask);
if (option_index == OPT_SPECIAL_unknown)
{
- option_proposer op;
- const char *hint = op.suggest_option (new_option);
- if (hint)
- error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>;"
- " did you mean %<-%s%>?", value ? "" : "no-",
- arg, new_option, hint);
- else
- error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>",
- value ? "" : "no-", arg, new_option);
+ cl_decoded_option fake_decoded = {};
+ fake_decoded.opt_index = OPT_SPECIAL_unknown;
+ fake_decoded.arg = orig_text;
+
+ if (handlers->unknown_option_callback (&fake_decoded))
+ {
+ option_proposer op;
+ const char *hint = op.suggest_option (new_option);
+ if (hint)
+ error_at (loc, _("%qs: no option %<-%s%>; did you mean %<-%s%>?"),
+ orig_text, new_option, hint);
+ else
+ error_at (loc, _("%qs: no option %<-%s%>"), orig_text, new_option);
+ }
}
else if (!(cl_options[option_index].flags & CL_WARNING))
-error_at (loc, "%<-Werror=%s%>: %<-%s%> is not an option that "
- "controls warnings", arg, new_option);
+error_at (loc, _("%qs: %<-%s%> is not an option that controls warnings"),
+ orig_text, new_option);
