Re: [PATCH resend 1/2] PR c/65403 - Ignore -Wno-error=
On Fri, Aug 23, 2019 at 3:13 AM Martin Liška wrote:
> On 8/23/19 6:17 AM, Alex Henrie wrote:
> > +vec ignored_wnoerror_options;
>
> Here you'll need to add a comment.
The declaration of ignored_options in opts-global.c doesn't have a
comment either. What would you like the comment to say?
> > + const char * opt = ignored_wnoerror_options.pop ();
>
> No space between '*' and 'opt' please.
Okay.
> You don't want to append and option that is already in the
> ignored_wnoerror_options:
>
> $
> ./xgcc -B. -Wunused-variable -Werror -Wno-error=some-future-warning
> -Wno-error=some-future-warning /tmp/main2.c
> /tmp/main2.c: In function ‘main’:
> /tmp/main2.c:3:7: error: unused variable ‘foo’ [-Werror=unused-variable]
> 3 | int foo; /* { dg-error "unused variable 'foo'" } */
> | ^~~
> /tmp/main2.c: At top level:
> cc1: error: ‘-Wno-error=some-future-warning’: no option
> ‘-Wsome-future-warning’ [-Werror]
> cc1: error: ‘-Wno-error=some-future-warning’: no option
> ‘-Wsome-future-warning’ [-Werror]
> cc1: all warnings being treated as errors
>
> As seen the error is there twice.
Joseph explicitly asked me to make -Wno-error=some-future-warning
behave the same as -Wno-some-future-warning (see
<https://gcc.gnu.org/ml/gcc-patches/2019-03/msg00065.html>), and
-Wno-some-future-warning prints multiple warnings if the option is
given multiple times.
> One question about the behavior:
>
> Why do I need to have another warning to get the warning:
> ‘-Wno-error=some-future-warning’ printed?
If we always give a warning about -Wno-error=some-future-warning then
combining that option with -Werror would cause compilation to fail,
which we don't want.
-Alex
[PATCH resend 2/2] PR c/65403 - Add tests for -Wno-error=
* c-c++-common/pr65403-1.c: New test.
* c-c++-common/pr65403-2.c: New test.
---
gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++
gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++
2 files changed, 25 insertions(+)
create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c
create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c
diff --git a/gcc/testsuite/c-c++-common/pr65403-1.c
b/gcc/testsuite/c-c++-common/pr65403-1.c
new file mode 100644
index 000..fbe004a1f78
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-1.c
@@ -0,0 +1,10 @@
+/* PR c/65403 */
+/* Test an unrecognized -Wno-error option in the absence of any other
+ diagnostics. The -Wno-error option should be ignored. */
+
+/* { dg-options "-Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/pr65403-2.c
b/gcc/testsuite/c-c++-common/pr65403-2.c
new file mode 100644
index 000..8b5faa7270e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-2.c
@@ -0,0 +1,15 @@
+/* PR c/65403 */
+/* Test a warning, treated as an error, that some future -Wno-error option
+ might downgrade back to a warning. The -Wno-error option should produce a
+ warning in this case. */
+
+/* { dg-options "-Wunused-variable -Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ int foo; /* { dg-error "unused variable 'foo'" } */
+ return 0;
+}
+
+/* { dg-error "no option '-Wsome-future-warning'" "" { target *-*-* } 0 } */
+/* { dg-message "all warnings being treated as errors" "" { target *-*-* } 0 }
*/
--
2.23.0
[PATCH resend 1/2] PR c/65403 - Ignore -Wno-error=
From: Manuel López-Ibáñez
* opts-common.c (ignored_wnoerror_options): New global variable.
* opts-global.c (print_ignored_options): Ignore
-Wno-error= except if there are other
diagnostics.
* opts.c (enable_warning_as_error): Record ignored -Wno-error
options.
* opts.h (ignored_wnoerror_options): Declare.
* gcc.dg/Werror-13.c: Don't expect hints for
-Wno-error=.
---
gcc/opts-common.c| 2 ++
gcc/opts-global.c| 10 +++---
gcc/opts.c | 21 +
gcc/opts.h | 2 ++
gcc/testsuite/gcc.dg/Werror-13.c | 2 +-
5 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index e2a315ba229..86e518bdd2b 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -26,6 +26,8 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic.h"
#include "spellcheck.h"
+vec ignored_wnoerror_options;
+
static void prune_options (struct cl_decoded_option **, unsigned int *);
/* An option that is undocumented, that takes a joined argument, and
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index 7c5bd16c7ea..b4b4576e450 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -136,12 +136,16 @@ print_ignored_options (void)
{
while (!ignored_options.is_empty ())
{
- const char *opt;
-
- opt = ignored_options.pop ();
+ const char * opt = ignored_options.pop ();
warning_at (UNKNOWN_LOCATION, 0,
"unrecognized command-line option %qs", opt);
}
+ while (!ignored_wnoerror_options.is_empty ())
+{
+ const char * opt = ignored_wnoerror_options.pop ();
+ warning_at (UNKNOWN_LOCATION, 0,
+ "%<-Wno-error=%s%>: no option %<-W%s%>", opt, opt);
+}
}
/* Handle an unknown option DECODED, returning true if an error should
diff --git a/gcc/opts.c b/gcc/opts.c
index bb0d8b5e7db..a78e5cf1949 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3175,15 +3175,20 @@ 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);
+ if (value)
+ {
+ 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);
+ }
else
- error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>",
- value ? "" : "no-", arg, new_option);
+ ignored_wnoerror_options.safe_push (arg);
}
else if (!(cl_options[option_index].flags & CL_WARNING))
error_at (loc, "%<-Werror=%s%>: %<-%s%> is not an option that "
diff --git a/gcc/opts.h b/gcc/opts.h
index 47223229388..ac281aef540 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -461,4 +461,6 @@ extern bool parse_and_check_align_values (const char *flag,
bool report_error,
location_t loc);
+extern vec ignored_wnoerror_options;
+
#endif
diff --git a/gcc/testsuite/gcc.dg/Werror-13.c b/gcc/testsuite/gcc.dg/Werror-13.c
index 3a02b7ea2b5..7c2bf6836ed 100644
--- a/gcc/testsuite/gcc.dg/Werror-13.c
+++ b/gcc/testsuite/gcc.dg/Werror-13.c
@@ -5,6 +5,6 @@
/* { dg-error "'-Werror' is not an option that controls warnings" "" { target
*-*-* } 0 } */
/* { dg-error "'-Wfatal-errors' is not an option that controls warnings" "" {
target *-*-* } 0 } */
/* { dg-error "'-Werror=vla2': no option '-Wvla2'; did you mean '-Wvla." "" {
target *-*-* } 0 } */
-/* { dg-error "'-Wno-error=misleading-indentation2': no option
'-Wmisleading-indentation2'; did you mean '-Wmisleading-indentation'" "" {
target *-*-* } 0 } */
+/* { dg-warning "'-Wno-error=misleading-indentation2': no option
'-Wmisleading-indentation2'" "" { target *-*-* } 0 } */
int i;
--
2.23.0
[PATCH v3 2/2] PR c/65403 - Add tests for -Wno-error=
* c-c++-common/pr65403-1.c: New test.
* c-c++-common/pr65403-2.c: New test.
---
gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++
gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++
2 files changed, 25 insertions(+)
create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c
create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c
diff --git a/gcc/testsuite/c-c++-common/pr65403-1.c
b/gcc/testsuite/c-c++-common/pr65403-1.c
new file mode 100644
index 000..fbe004a1f78
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-1.c
@@ -0,0 +1,10 @@
+/* PR c/65403 */
+/* Test an unrecognized -Wno-error option in the absence of any other
+ diagnostics. The -Wno-error option should be ignored. */
+
+/* { dg-options "-Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/pr65403-2.c
b/gcc/testsuite/c-c++-common/pr65403-2.c
new file mode 100644
index 000..8b5faa7270e
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-2.c
@@ -0,0 +1,15 @@
+/* PR c/65403 */
+/* Test a warning, treated as an error, that some future -Wno-error option
+ might downgrade back to a warning. The -Wno-error option should produce a
+ warning in this case. */
+
+/* { dg-options "-Wunused-variable -Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ int foo; /* { dg-error "unused variable 'foo'" } */
+ return 0;
+}
+
+/* { dg-error "no option '-Wsome-future-warning'" "" { target *-*-* } 0 } */
+/* { dg-message "all warnings being treated as errors" "" { target *-*-* } 0 }
*/
--
2.22.0
[PATCH v3 1/2] PR c/65403 - Ignore -Wno-error=
From: Manuel López-Ibáñez
* opts-common.c (ignored_wnoerror_options): New global variable.
* opts-global.c (print_ignored_options): Ignore
-Wno-error= except if there are other
diagnostics.
* opts.c (enable_warning_as_error): Record ignored -Wno-error
options.
* opts.h (ignored_wnoerror_options): Declare.
* gcc.dg/Werror-13.c: Don't expect hints for
-Wno-error=.
---
gcc/opts-common.c| 2 ++
gcc/opts-global.c| 10 +++---
gcc/opts.c | 21 +
gcc/opts.h | 2 ++
gcc/testsuite/gcc.dg/Werror-13.c | 2 +-
5 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index 660dfe63858..8ceb8461f97 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -26,6 +26,8 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic.h"
#include "spellcheck.h"
+vec ignored_wnoerror_options;
+
static void prune_options (struct cl_decoded_option **, unsigned int *);
/* An option that is undocumented, that takes a joined argument, and
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index bf4db775928..1d5d4e69dfc 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -132,12 +132,16 @@ print_ignored_options (void)
{
while (!ignored_options.is_empty ())
{
- const char *opt;
-
- opt = ignored_options.pop ();
+ const char * opt = ignored_options.pop ();
warning_at (UNKNOWN_LOCATION, 0,
"unrecognized command-line option %qs", opt);
}
+ while (!ignored_wnoerror_options.is_empty ())
+{
+ const char * opt = ignored_wnoerror_options.pop ();
+ warning_at (UNKNOWN_LOCATION, 0,
+ "%<-Wno-error=%s%>: no option %<-W%s%>", opt, opt);
+}
}
/* Handle an unknown option DECODED, returning true if an error should
diff --git a/gcc/opts.c b/gcc/opts.c
index b38bfb15a56..f31b6aa877e 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3095,15 +3095,20 @@ 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);
+ if (value)
+ {
+ 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);
+ }
else
- error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>",
- value ? "" : "no-", arg, new_option);
+ ignored_wnoerror_options.safe_push (arg);
}
else if (!(cl_options[option_index].flags & CL_WARNING))
error_at (loc, "%<-Werror=%s%>: %<-%s%> is not an option that "
diff --git a/gcc/opts.h b/gcc/opts.h
index e5723a946f7..f553e8d00f0 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -460,4 +460,6 @@ extern bool parse_and_check_align_values (const char *flag,
bool report_error,
location_t loc);
+extern vec ignored_wnoerror_options;
+
#endif
diff --git a/gcc/testsuite/gcc.dg/Werror-13.c b/gcc/testsuite/gcc.dg/Werror-13.c
index 3a02b7ea2b5..7c2bf6836ed 100644
--- a/gcc/testsuite/gcc.dg/Werror-13.c
+++ b/gcc/testsuite/gcc.dg/Werror-13.c
@@ -5,6 +5,6 @@
/* { dg-error "'-Werror' is not an option that controls warnings" "" { target
*-*-* } 0 } */
/* { dg-error "'-Wfatal-errors' is not an option that controls warnings" "" {
target *-*-* } 0 } */
/* { dg-error "'-Werror=vla2': no option '-Wvla2'; did you mean '-Wvla." "" {
target *-*-* } 0 } */
-/* { dg-error "'-Wno-error=misleading-indentation2': no option
'-Wmisleading-indentation2'; did you mean '-Wmisleading-indentation'" "" {
target *-*-* } 0 } */
+/* { dg-warning "'-Wno-error=misleading-indentation2': no option
'-Wmisleading-indentation2'" "" { target *-*-* } 0 } */
int i;
--
2.22.0
Re: [PATCH 1/2] PR c/65403 - Ignore -Wno-error=
On Wed, Jun 19, 2019 at 11:52 AM Jeff Law wrote: > > On 3/18/19 8:46 PM, Alex Henrie wrote: > > From: Manuel López-Ibáñez > > > > * opts.c: Ignore -Wno-error= except if there are > > other diagnostics. > That's not a complete ChangeLog entry. Each file/function changed > should be mentioned. Something like this: > > * opts-common.c (ignored_wnoerror_options): New global variable. > * opts-global.c (print_ignored_options): Ignore > -Wno-error= except if there are other > diagnostics. > * opts.c (enable_warning_as_error): Record ignored -Wno-error > options. > opts.h (ignored_wnoerror_options): Declare. Thanks! > If HINT is set, do we still want to potentially push the argument onto > the ignored_wnoerror_options list? My inclination is yes since the hint > is just that, a fuzzily matched hint. That hint may be appropriate > (user typo'd or somesuch) or it may be totally offbase (user was trying > to turn off some future warning. I don't think we need to support hints in the case of -Wno-error= because we don't support hints for -Wno- and as you pointed out, hints are less likely to be helpful here because the warning may be perfectly valid in a newer version of GCC. I'll send rebased patches later tonight. Thanks for the feedback! -Alex
Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers
On Fri, May 31, 2019 at 1:38 AM Richard Biener wrote:
>
> On Thu, 30 May 2019, Alex Henrie wrote:
>
> > In Wine we need a way to (without warnings) put ms_hook_prologue into
> > a macro that is applied to functions, function pointers, and function
> > pointer typedefs. It sounds like you're saying that you will not
> > accept a patch that silences or splits off warnings about using
> > ms_hook_prologue with function pointers and function pointer typedefs.
> > So how do you think Wine's problem should be solved?
>
> I think ms_hook_prologue should be allowed to apply to function types
> and function decls. If you say it should apply to function pointers
> then I suppose you want to have it apply to the pointed to function
> of the function pointer - but that's not possible without an indirection
> via a function pointer typedef IIRC.
No, if ms_hook_prologue is applied to a function pointer, it shouldn't
do anything except maybe trigger some optimization of the code around
the indirect function call.
> I also have the following which _may_ motivate that attributes
> currently not applying to function types (because they only
> affect function definitions) should also apply there:
>
> typedef int (myfun) (int *) __attribute__((nonnull(1)));
> myfun x;
> int x(int *p) { return p != (int*)0; }
>
> this applies nonnull to the function definition of 'x' but
> I put the attribute on the typedef. I didn't manage to
> do without the myfun x; declaration.
That is a great example and another compelling reason to allow
"fndecl" attributes in more places.
> > It seems to me that any information about the target of a function
> > pointer, even the flatten attribute or the ms_hook_prologue attribute,
> > provides information that could be useful for optimizing the code
> > around the indirect function call. That sounds like a compelling
> > argument for allowing these attributes in more places without
> > warnings.
>
> Sure. Can you write down the three cases after macro expansion
> here to clarify what you need? Esp. say what the attribute should
> apply to. Just silencing the warning without actually achieving
> what you want would be bad I think ;)
Essentially, the following needs to compile without warnings:
#define WINAPI __attribute__((__stdcall__)) \
__attribute__((__ms_hook_prologue__))
typedef unsigned int (WINAPI *APPLICATION_RECOVERY_CALLBACK)(void*);
void WINAPI foo()
{
APPLICATION_RECOVERY_CALLBACK bar;
unsigned int (WINAPI *baz)(void*);
}
-Alex
Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers
On Thu, May 30, 2019 at 6:59 PM Joseph Myers wrote: > > On Thu, 30 May 2019, Alex Henrie wrote: > > > At this point I think I'm convinced that any attribute that applies to > > a function should also be allowed on a function pointer without any > > warnings. We can start by turning off the warnings for the "fndecl" > > attributes and then clean up the other attributes as time goes on. > > This is inherently a property of the attribute in question. The issue is > not whether it applies to function pointers; it's whether it applies to > function types. > > For example, the "section" or "alias" attributes are attributes that apply > to a declaration, but not a type. Because they apply to variables as well > as functions, they are meaningful on function pointers - but the meaning > is *not* the same as applying them to the pointed-to function. > > The "flatten" attribute, however, seems only meaningful for functions, not > variables, not function types and not function pointers. > > We should try to work out for each attribute exactly what construct it > appertains to - which for many but not all function attributes is indeed > the type of the function rather than the function itself. Then move to > making such attributes work on types. But for attributes such as > "flatten" that logically appertain to the declaration not its type, we > should continue to diagnose them on function pointers or types. In Wine we need a way to (without warnings) put ms_hook_prologue into a macro that is applied to functions, function pointers, and function pointer typedefs. It sounds like you're saying that you will not accept a patch that silences or splits off warnings about using ms_hook_prologue with function pointers and function pointer typedefs. So how do you think Wine's problem should be solved? It seems to me that any information about the target of a function pointer, even the flatten attribute or the ms_hook_prologue attribute, provides information that could be useful for optimizing the code around the indirect function call. That sounds like a compelling argument for allowing these attributes in more places without warnings. -Alex
Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers
On Tue, May 28, 2019 at 1:05 PM Martin Sebor wrote: > > On 5/24/19 9:49 AM, Alex Henrie wrote: > > Then is it preferable to simply silence Wattributes in this case? > > This case being PR86407? I'd say yes. > Attribute malloc attaches only to fndecl and not its type but > doesn't affect the code for a function definition. FWIW, I > think this is just a bug -- attribute malloc should apply > to a function type for the same reason the closely related > attribute alloc_size does. > > Attribute constructor also attaches to a fndecl even though it > doesn't affect the function's codegen but that of its caller > (the runtime). In this case, though, I'd say that's fine. > Should it be classified as a function definition attribute? > > Attributes cold and hot also apply to a fndecl and not its type > but they affect both the function's definition and its callers. > I think this also makes sense. Should they be classified as > function definition attributes? Those are very good points. The more information the optimizer has about how the function works, the better it can optimize the code near the function call. This would even apply to ms_hook_prologue because the optimizer should definitely not inline indirect calls to hookable functions even if it would inline a similar non-hookable function. At this point I think I'm convinced that any attribute that applies to a function should also be allowed on a function pointer without any warnings. We can start by turning off the warnings for the "fndecl" attributes and then clean up the other attributes as time goes on. Sound good? -Alex On Tue, May 28, 2019 at 1:05 PM Martin Sebor wrote: > > On 5/24/19 9:49 AM, Alex Henrie wrote: > > On Fri, May 24, 2019 at 2:01 AM Richard Biener wrote: > >> > >> I'm not sure we really need a new warning for this. > > > > On Fri, May 24, 2019 at 9:23 AM Martin Sebor wrote: > >> > >> I don't think GCC makes a formal distinction between function > >> attributes that affect only function definitions vs those that > >> affect its users, or both. It might be a useful distinction > >> to introduce, perhaps even for functions as well as variables, > >> but as it is, users (as well as GCC developers) are on our own > >> to figure it out. > > > > Then is it preferable to simply silence Wattributes in this case? > > This case being PR86407? I'd say yes. I think one general problem > to solve is the missing suppression for typedefs. This could be > useful for other warnings as well. Another is providing a knob to > control the warning when one kind of an attribute is used with > an entity of a different kind (function vs type, or variable vs > type). Yet another might be to control warnings for individual > attributes. > > > > > On Fri, May 24, 2019 at 2:01 AM Richard Biener wrote: > >> > >>> +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */ > >>> + > >> > >> But this is a declaration? > > > > On Fri, May 24, 2019 at 9:23 AM Martin Sebor wrote: > >> > >> My first question is: what is the meaning of "function definition > >> attributes?" Presumably those that affect only the definition of > >> a function and not its callers or other users? > > > > As far as I can tell, "fndecl" is a misnomer: these attributes are > > more accurately called "function definition attributes", i.e. > > attributes that affect the assembly code of the function but do not > > affect its calling convention. > > That's one possible definition but there are examples that don't > fit it (at least not very neatly). > > Attribute malloc attaches only to fndecl and not its type but > doesn't affect the code for a function definition. FWIW, I > think this is just a bug -- attribute malloc should apply > to a function type for the same reason the closely related > attribute alloc_size does. > > Attribute constructor also attaches to a fndecl even though it > doesn't affect the function's codegen but that of its caller > (the runtime). In this case, though, I'd say that's fine. > Should it be classified as a function definition attribute? > > Attributes cold and hot also apply to a fndecl and not its type > but they affect both the function's definition and its callers. > I think this also makes sense. Should they be classified as > function definition attributes? > > > On Fri, May 24, 2019 at 9:23 AM Martin Sebor wrote: > >> > >> Finally, with this as a prerequisite, if we decided that a warning > >> like this would be useful, tests to verify that it works
Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers
On Sat, May 25, 2019 at 12:34 AM Richard Biener wrote: > > On May 24, 2019 5:49:38 PM GMT+02:00, Alex Henrie > wrote: > >As far as I can tell, "fndecl" is a misnomer: these attributes are > >more accurately called "function definition attributes", i.e. > >attributes that affect the assembly code of the function but do not > >affect its calling convention. > > Yes. The others are attributes also applicable to function types to properly > support attributed indirect calls. I'm happy to rename ix86_handle_fndecl_attribute to ix86_handle_fndef_attribute then. Should I do that in this patch or a separate patch? -Alex
Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers
On Fri, May 24, 2019 at 2:01 AM Richard Biener wrote: > > I'm not sure we really need a new warning for this. On Fri, May 24, 2019 at 9:23 AM Martin Sebor wrote: > > I don't think GCC makes a formal distinction between function > attributes that affect only function definitions vs those that > affect its users, or both. It might be a useful distinction > to introduce, perhaps even for functions as well as variables, > but as it is, users (as well as GCC developers) are on our own > to figure it out. Then is it preferable to simply silence Wattributes in this case? On Fri, May 24, 2019 at 2:01 AM Richard Biener wrote: > > > +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */ > > + > > But this is a declaration? On Fri, May 24, 2019 at 9:23 AM Martin Sebor wrote: > > My first question is: what is the meaning of "function definition > attributes?" Presumably those that affect only the definition of > a function and not its callers or other users? As far as I can tell, "fndecl" is a misnomer: these attributes are more accurately called "function definition attributes", i.e. attributes that affect the assembly code of the function but do not affect its calling convention. On Fri, May 24, 2019 at 9:23 AM Martin Sebor wrote: > > Finally, with this as a prerequisite, if we decided that a warning > like this would be useful, tests to verify that it works for all > the definition attributes and not for the rest would need to be > added (i.e., in addition to ms_hook_prologue). Okay, I will add tests for the other function attributes that should behave in the same way, commenting out the tests that will require more work to pass. The end goal is to include __ms_hook_prologue__ in the WINAPI macro on Wine without causing spurious warnings. This will go a long way towards making Wine compatible with current and future Windows programs. Thank you for help. -Alex
[PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers
---
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86407#c6
---
gcc/c-family/c.opt | 4
gcc/c/c-decl.c | 4 +++-
gcc/config/i386/i386-options.c | 12 ++--
gcc/testsuite/c-c++-common/pr86407-1.c | 23 +++
gcc/testsuite/c-c++-common/pr86407-2.c | 25 +
5 files changed, 65 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/pr86407-1.c
create mode 100644 gcc/testsuite/c-c++-common/pr86407-2.c
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 046d489f7eb..3b99c96e53d 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -776,6 +776,10 @@ Wsizeof-array-argument
C ObjC C++ ObjC++ Var(warn_sizeof_array_argument) Warning Init(1)
Warn when sizeof is applied on a parameter declared as an array.
+Wstrict-function-attributes
+C C++ Var(warn_strict_function_attributes) Init(1) Warning
+Warn when function definition attributes are applied to function pointers.
+
Wstringop-overflow
C ObjC C++ LTO ObjC++ Warning Alias(Wstringop-overflow=, 2, 0)
Warn about buffer overflow in string manipulation functions like memcpy
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 181a8c2e9aa..427d573c8d3 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -6192,7 +6192,9 @@ grokdeclarator (const struct c_declarator *declarator,
inner_decl = inner_decl->declarator;
if (inner_decl->kind == cdk_id)
attr_flags |= (int) ATTR_FLAG_DECL_NEXT;
- else if (inner_decl->kind == cdk_function)
+ else if (inner_decl->kind == cdk_function
+|| (inner_decl->kind == cdk_pointer
+&& TREE_CODE (type) == FUNCTION_TYPE))
attr_flags |= (int) ATTR_FLAG_FUNCTION_NEXT;
else if (inner_decl->kind == cdk_array)
attr_flags |= (int) ATTR_FLAG_ARRAY_NEXT;
diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
index 0f236626005..6b50f143b05 100644
--- a/gcc/config/i386/i386-options.c
+++ b/gcc/config/i386/i386-options.c
@@ -3489,8 +3489,16 @@ ix86_handle_fndecl_attribute (tree *node, tree name,
tree args, int,
{
if (TREE_CODE (*node) != FUNCTION_DECL)
{
- warning (OPT_Wattributes, "%qE attribute only applies to functions",
- name);
+ if (FUNCTION_POINTER_TYPE_P (TREE_TYPE (*node)))
+ {
+ warning (OPT_Wstrict_function_attributes,
+ "%qE attribute only applies to function definitions", name);
+ }
+ else
+ {
+ warning (OPT_Wattributes,
+ "%qE attribute only applies to functions", name);
+ }
*no_add_attrs = true;
}
diff --git a/gcc/testsuite/c-c++-common/pr86407-1.c
b/gcc/testsuite/c-c++-common/pr86407-1.c
new file mode 100644
index 000..37993d8c051
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr86407-1.c
@@ -0,0 +1,23 @@
+/* PR c/86407 */
+/* Test a function attribute that is not a calling convention and does not
+ affect the function type. There should be no warnings when using the
+ attribute on a function declaration, function pointer, or function pointer
+ typedef, but there should be a warning when using the attribute on
+ non-function-pointer variables and typedefs. */
+
+/* { dg-options "-Wstrict-function-attributes -Wno-ignored-attributes" } */
+
+int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
+
+int (__attribute__((__ms_hook_prologue__)) *func_ptr)(); /* { dg-warning
"'ms_hook_prologue' attribute only applies to function definitions" } */
+
+typedef int (__attribute__((__ms_hook_prologue__)) *FUNC_PTR)(); /* {
dg-warning "'ms_hook_prologue' attribute only applies to function definitions"
} */
+
+int __attribute__((__ms_hook_prologue__)) *var_ptr; /* { dg-warning
"'ms_hook_prologue' attribute only applies to functions" } */
+
+typedef int __attribute__((__ms_hook_prologue__)) *VAR_PTR; /* { dg-warning
"'ms_hook_prologue' attribute only applies to functions" } */
+
+int main(int argc, char **argv)
+{
+return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/pr86407-2.c
b/gcc/testsuite/c-c++-common/pr86407-2.c
new file mode 100644
index 000..6840b0180dd
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr86407-2.c
@@ -0,0 +1,25 @@
+/* PR c/86407 */
+/* Test a function attribute that is not a calling convention and does not
+ affect the function type. A function pointer without the attribute may point
+ to a function with the attribute and vice-versa. */
+
+/* { dg-options "-Wno-strict-function-attributes -Wno-ignored-attributes" } */
+
+int ordinary_func()
+{
+return 0;
+}
+
+int __attribute__((__ms_hook_prologue__)) special_func()
+{
+return 0;
+}
+
+int main(int argc, char **argv)
+{
+int (*ordinary_func_ptr)() = special_func;
+
+int (__attribute__((__ms_hook_prologue__)) *special_func_ptr)() =
ordinary_func;
+
+return 0;
+}
--
2.21.0
Re: [PATCH 0/2] Future warnings not treated as errors
On Tue, Mar 19, 2019 at 10:09 PM Alex Henrie wrote: > > On Tue, Mar 19, 2019 at 11:03 AM Martin Liška wrote: > > > > What's Joseph telling is that right now the patch can't be approved (and > > installed). > > It's due to fact that we are close to the release in stage4: > > https://www.gnu.org/software/gcc/develop.html#stage4 > > > > So that a proper review will happen in couple of weeks. > > > > Thanks for understanding, > > Sure. I look forward to hearing back from you guys in a couple of weeks. Now that GCC 9.1 has been released, would you please merge these two patches? -Alex
Re: [PATCH 0/2] Future warnings not treated as errors
On Tue, Mar 19, 2019 at 11:03 AM Martin Liška wrote: > > What's Joseph telling is that right now the patch can't be approved (and > installed). > It's due to fact that we are close to the release in stage4: > https://www.gnu.org/software/gcc/develop.html#stage4 > > So that a proper review will happen in couple of weeks. > > Thanks for understanding, Sure. I look forward to hearing back from you guys in a couple of weeks. -Alex
[PATCH v2 1/2] PR c/65403 - Ignore -Wno-error=
From: Manuel López-Ibáñez
* opts.c: Ignore -Wno-error= except if there are
other diagnostics.
---
gcc/opts-common.c | 2 ++
gcc/opts-global.c | 10 +++---
gcc/opts.c| 5 -
gcc/opts.h| 2 ++
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index edbb3ac9b6d..36e06e2372a 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -26,6 +26,8 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic.h"
#include "spellcheck.h"
+vec ignored_wnoerror_options;
+
static void prune_options (struct cl_decoded_option **, unsigned int *);
/* An option that is undocumented, that takes a joined argument, and
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index a5e9ef0237a..bef020a4c46 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -132,12 +132,16 @@ print_ignored_options (void)
{
while (!ignored_options.is_empty ())
{
- const char *opt;
-
- opt = ignored_options.pop ();
+ const char * opt = ignored_options.pop ();
warning_at (UNKNOWN_LOCATION, 0,
"unrecognized command line option %qs", opt);
}
+ while (!ignored_wnoerror_options.is_empty ())
+{
+ const char * opt = ignored_wnoerror_options.pop ();
+ warning_at (UNKNOWN_LOCATION, 0,
+ "%<-Werror=%s%>: no option -W%s", opt, opt);
+}
}
/* Handle an unknown option DECODED, returning true if an error should
diff --git a/gcc/opts.c b/gcc/opts.c
index 02f6b4656e1..492a50ba326 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3087,7 +3087,10 @@ enable_warning_as_error (const char *arg, int value,
unsigned int lang_mask,
strcpy (new_option + 1, arg);
option_index = find_opt (new_option, lang_mask);
if (option_index == OPT_SPECIAL_unknown)
-error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option);
+if (value)
+ error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option);
+else
+ ignored_wnoerror_options.safe_push (arg);
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);
diff --git a/gcc/opts.h b/gcc/opts.h
index f14d9bcb896..b18504e0bc3 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -456,4 +456,6 @@ extern bool parse_and_check_align_values (const char *flag,
bool report_error,
location_t loc);
+extern vec ignored_wnoerror_options;
+
#endif
--
2.21.0
[PATCH v2 2/2] PR c/65403 - Add tests for -Wno-error=
---
gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++
gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++
2 files changed, 25 insertions(+)
create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c
create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c
diff --git a/gcc/testsuite/c-c++-common/pr65403-1.c
b/gcc/testsuite/c-c++-common/pr65403-1.c
new file mode 100644
index 000..fbe004a1f78
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-1.c
@@ -0,0 +1,10 @@
+/* PR c/65403 */
+/* Test an unrecognized -Wno-error option in the absence of any other
+ diagnostics. The -Wno-error option should be ignored. */
+
+/* { dg-options "-Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/pr65403-2.c
b/gcc/testsuite/c-c++-common/pr65403-2.c
new file mode 100644
index 000..128d4f694a6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-2.c
@@ -0,0 +1,15 @@
+/* PR c/65403 */
+/* Test a warning, treated as an error, that some future -Wno-error option
+ might downgrade back to a warning. The -Wno-error option should produce a
+ warning in this case. */
+
+/* { dg-options "-Wunused-variable -Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ int foo; /* { dg-error "unused variable 'foo'" } */
+ return 0;
+}
+
+/* { dg-error "no option -Wsome-future-warning" "" { target *-*-* } 0 } */
+/* { dg-message "all warnings being treated as errors" "" { target *-*-* } 0 }
*/
--
2.21.0
Re: [PATCH 1/2] PR c/65403 - Ignore -Wno-error=
On Tue, Mar 19, 2019 at 11:26 AM Martin Sebor wrote: > > Please remember to quote the command line options in the message > (same as in the error below): I'll send a new version. Thanks for the feedback! -Alex
Re: [PATCH 0/2] Future warnings not treated as errors
On Tue, Mar 19, 2019 at 3:19 AM Martin Liška wrote: > > On 3/19/19 3:46 AM, Alex Henrie wrote: > > Hello, > > > > I have received permission to contribute to GCC, but I do not have > > commit access. Could one of you please merge these patches for me? > > Hi. > > I can do it for you. But if I see correctly, Joseph M. had some concerns > and he should probably make final patch approval. If I understand correctly, Joseph wanted an error message to still be printed if other diagnostics are also printed, the same as how -Wno- works. The patches that I sent yesterday do exactly that. -Alex
[PATCH 0/2] Future warnings not treated as errors
Hello, I have received permission to contribute to GCC, but I do not have commit access. Could one of you please merge these patches for me? -Alex Alex Henrie (1): PR c/65403 - Add tests for -Wno-error= Manuel López-Ibáñez (1): PR c/65403 - Ignore -Wno-error= gcc/opts-common.c | 2 ++ gcc/opts-global.c | 10 +++--- gcc/opts.c | 5 - gcc/opts.h | 2 ++ gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++ gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++ 6 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c -- 2.21.0
[PATCH 1/2] PR c/65403 - Ignore -Wno-error=
From: Manuel López-Ibáñez
* opts.c: Ignore -Wno-error= except if there are
other diagnostics.
---
gcc/opts-common.c | 2 ++
gcc/opts-global.c | 10 +++---
gcc/opts.c| 5 -
gcc/opts.h| 2 ++
4 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index edbb3ac9b6d..36e06e2372a 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -26,6 +26,8 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic.h"
#include "spellcheck.h"
+vec ignored_wnoerror_options;
+
static void prune_options (struct cl_decoded_option **, unsigned int *);
/* An option that is undocumented, that takes a joined argument, and
diff --git a/gcc/opts-global.c b/gcc/opts-global.c
index a5e9ef0237a..0b2550a4855 100644
--- a/gcc/opts-global.c
+++ b/gcc/opts-global.c
@@ -132,12 +132,16 @@ print_ignored_options (void)
{
while (!ignored_options.is_empty ())
{
- const char *opt;
-
- opt = ignored_options.pop ();
+ const char * opt = ignored_options.pop ();
warning_at (UNKNOWN_LOCATION, 0,
"unrecognized command line option %qs", opt);
}
+ while (!ignored_wnoerror_options.is_empty ())
+{
+ const char * opt = ignored_wnoerror_options.pop ();
+ warning_at (UNKNOWN_LOCATION, 0,
+ "-Wno-error=%s: no option -W%s", opt, opt);
+}
}
/* Handle an unknown option DECODED, returning true if an error should
diff --git a/gcc/opts.c b/gcc/opts.c
index 3161e0b6753..4fa79306e30 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3079,7 +3079,10 @@ enable_warning_as_error (const char *arg, int value,
unsigned int lang_mask,
strcpy (new_option + 1, arg);
option_index = find_opt (new_option, lang_mask);
if (option_index == OPT_SPECIAL_unknown)
-error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option);
+if (value)
+ error_at (loc, "%<-Werror=%s%>: no option -%s", arg, new_option);
+else
+ ignored_wnoerror_options.safe_push (arg);
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);
diff --git a/gcc/opts.h b/gcc/opts.h
index f14d9bcb896..b18504e0bc3 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -456,4 +456,6 @@ extern bool parse_and_check_align_values (const char *flag,
bool report_error,
location_t loc);
+extern vec ignored_wnoerror_options;
+
#endif
--
2.21.0
[PATCH 2/2] PR c/65403 - Add tests for -Wno-error=
---
gcc/testsuite/c-c++-common/pr65403-1.c | 10 ++
gcc/testsuite/c-c++-common/pr65403-2.c | 15 +++
2 files changed, 25 insertions(+)
create mode 100644 gcc/testsuite/c-c++-common/pr65403-1.c
create mode 100644 gcc/testsuite/c-c++-common/pr65403-2.c
diff --git a/gcc/testsuite/c-c++-common/pr65403-1.c
b/gcc/testsuite/c-c++-common/pr65403-1.c
new file mode 100644
index 000..fbe004a1f78
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-1.c
@@ -0,0 +1,10 @@
+/* PR c/65403 */
+/* Test an unrecognized -Wno-error option in the absence of any other
+ diagnostics. The -Wno-error option should be ignored. */
+
+/* { dg-options "-Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/pr65403-2.c
b/gcc/testsuite/c-c++-common/pr65403-2.c
new file mode 100644
index 000..128d4f694a6
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr65403-2.c
@@ -0,0 +1,15 @@
+/* PR c/65403 */
+/* Test a warning, treated as an error, that some future -Wno-error option
+ might downgrade back to a warning. The -Wno-error option should produce a
+ warning in this case. */
+
+/* { dg-options "-Wunused-variable -Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ int foo; /* { dg-error "unused variable 'foo'" } */
+ return 0;
+}
+
+/* { dg-error "no option -Wsome-future-warning" "" { target *-*-* } 0 } */
+/* { dg-message "all warnings being treated as errors" "" { target *-*-* } 0 }
*/
--
2.21.0
[PATCH] PR c/89524 - Ignore -Wno-error=
* opts.c: Ignore -Wno-error=.
---
gcc/opts.c | 5 -
gcc/testsuite/c-c++-common/pr89524.c | 7 +++
2 files changed, 11 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/c-c++-common/pr89524.c
diff --git a/gcc/opts.c b/gcc/opts.c
index 468abb16334..f9a490cc7c2 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -3079,7 +3079,10 @@ enable_warning_as_error (const char *arg, int value,
unsigned int lang_mask,
strcpy (new_option + 1, arg);
option_index = find_opt (new_option, lang_mask);
if (option_index == OPT_SPECIAL_unknown)
-error_at (loc, "-Werror=%s: no option -%s", arg, new_option);
+{
+ if (value)
+ error_at (loc, "-Werror=%s: no option -%s", arg, 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);
diff --git a/gcc/testsuite/c-c++-common/pr89524.c
b/gcc/testsuite/c-c++-common/pr89524.c
new file mode 100644
index 000..71bf745f57d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr89524.c
@@ -0,0 +1,7 @@
+/* PR c/89524 */
+/* { dg-options "-Werror -Wno-error=some-future-warning" } */
+
+int main(int argc, char **argv)
+{
+ return 0;
+}
--
2.21.0
