[PATCH] Warn for ignored ASM labels on typdef declarations PR 85444 (v.3)

Sat May 26 18:43:00 GMT 2018

Hello everyone!

I know every member of the community is very busy, but I am following
up on this patch to conform to the 'ping' etiquette.

Please let me know what comments you have about this patch and how I
can modify it to make sure that it meets standards.

Thanks for everything that you all do to make GCC the best compiler out there.

Will

On Fri, May 18, 2018 at 5:34 PM, Will Hawkins <whh8b@virginia.edu> wrote:
> Hello again!
>
> Thanks to the feedback of Mr. Myers and those on the PR, I have
> created a version 3 of this patch. This version introduces a new
> warning flag (enabled at Wall) -Wignored-asm-name that will flag cases
> where the user specifies an ASM name that the compiler ignores.
>
> Test cases included. Results from make bootstrap and/or make -k check
> are available upon request.
>
> Please let me know what I can do to make this better and bring it up
> to the standards of the community! Thanks again for the feedback on
> this patch during the previous two revisions!
>
> Sincerely,
> Will Hawkins
>
>
>     2018-05-18 Will Hawkins <whh8b@virginia.edu>
>
>     PR c,c++/85444
>         * gcc/c/c-decl.c: Warn about ignored asm label for
>         typedef declaration
>         * gcc/cp/decl.c: Warn about ignored asm label for
>         typedef declaration
>         * gcc/testsuite/gcc.dg/asm-pr85444.c: c testcase.
>         * gcc/testsuite/g++.dg/asm-pr85444.C: c++ testcase.
>
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index c48d6dc..ab3a9af 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -595,6 +595,10 @@ Wignored-attributes
>  C C++ Var(warn_ignored_attributes) Init(1) Warning
>  Warn whenever attributes are ignored.
>
> +Wignored-asm-name
> +C C++ Var(warn_ignored_asm_name) Warning LangEnabledBy(C C++,Wall)
> +Warn whenever assembler names are specified but ignored.
> +
>  Wincompatible-pointer-types
>  C ObjC Var(warn_incompatible_pointer_types) Init(1) Warning
>  Warn when there is a conversion between pointers that have incompatible types.
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 3c4b18e..5a1ecd7 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -5177,7 +5177,11 @@ finish_decl (tree decl, location_t init_loc, tree init,
>        if (!DECL_FILE_SCOPE_P (decl)
>        && variably_modified_type_p (TREE_TYPE (decl), NULL_TREE))
>      add_stmt (build_stmt (DECL_SOURCE_LOCATION (decl), DECL_EXPR, decl));
> -
> +      if (asmspec_tree != NULL_TREE)
> +    {
> +      warning (OPT_Wignored_asm_name, "asm-specifier is ignored in "
> +           "typedef declaration");
> +    }
>        rest_of_decl_compilation (decl, DECL_FILE_SCOPE_P (decl), 0);
>      }
>
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 10e3079..4c3ee36 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -6981,6 +6981,11 @@ cp_finish_decl (tree decl, tree init, bool
> init_const_expr_p,
>    /* Take care of TYPE_DECLs up front.  */
>    if (TREE_CODE (decl) == TYPE_DECL)
>      {
> +      if (asmspec_tree != NULL_TREE)
> +    {
> +      warning (OPT_Wignored_asm_name, "asm-specifier is ignored for "
> +           "typedef declarations");
> +    }
>        if (type != error_mark_node
>        && MAYBE_CLASS_TYPE_P (type) && DECL_NAME (decl))
>      {
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index ca3772b..63f81f4 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -286,7 +286,8 @@ Objective-C and Objective-C++ Dialects}.
>  -Wformat-y2k  -Wframe-address @gol
>  -Wframe-larger-than=@var{len}  -Wno-free-nonheap-object
> -Wjump-misses-init @gol
>  -Wif-not-aligned @gol
> --Wignored-qualifiers  -Wignored-attributes  -Wincompatible-pointer-types @gol
> +-Wignored-qualifiers  -Wignored-attributes  -Wignored-asm-name @gol
> +-Wincompatible-pointer-types @gol
>  -Wimplicit  -Wimplicit-fallthrough  -Wimplicit-fallthrough=@var{n} @gol
>  -Wimplicit-function-declaration  -Wimplicit-int @gol
>  -Winit-self  -Winline  -Wno-int-conversion  -Wint-in-bool-context @gol
> @@ -4523,6 +4524,14 @@ Warn when an attribute is ignored.  This is
> different from the
>  to drop an attribute, not that the attribute is either unknown, used in a
>  wrong place, etc.  This warning is enabled by default.
>
> +@item -Wignored-asm-name @r{(C and C++ only)}
> +@opindex Wignored-asm-name
> +@opindex Wno-ignored-asm-name
> +Warn when an assembler name is given but ignored. For C and C++, this
> +happens when a @code{typdef} declaration is given an assembler name.
> +
> +This warning is also enabled by @option{-Wall}.
> +
>  @item -Wmain
>  @opindex Wmain
>  @opindex Wno-main
> diff --git a/gcc/testsuite/g++.dg/asm-pr85444.C
> b/gcc/testsuite/g++.dg/asm-pr85444.C
> new file mode 100644
> index 0000000..f1f8f61
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/asm-pr85444.C
> @@ -0,0 +1,13 @@
> +/* Fix Bugzilla 8544 -- asm specifier on typedef silently ignored.
> +   { dg-do compile }
> +   { dg-options "-Wignored-asm-name" } */
> +
> +typedef struct
> +{
> +  int a;
> +} x asm ("X"); /* { dg-warning "asm-specifier is ignored" } */
> +
> +int main()
> +{
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/asm-pr85444.c
> b/gcc/testsuite/gcc.dg/asm-pr85444.c
> new file mode 100644
> index 0000000..73ccea0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/asm-pr85444.c
> @@ -0,0 +1,14 @@
> +/* Fix Bugzilla 8544 -- asm specifier on typedef silently ignored.
> +   { dg-do compile }
> +   { dg-options "-Wignored-asm-name" } */
> +
> +typedef struct
> +{
> +  int a;
> +} x asm ("X"); /* { dg-warning "asm-specifier is ignored" } */
> +
> +int main()
> +{
> +  return 0;
> +}
> +
>
> On Mon, Apr 30, 2018 at 3:56 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Mon, 30 Apr 2018, Will Hawkins wrote:
>>
>>> I agree! It was, however, the closest of all the categories that I
>>> could find that seemed to match the warning that I am trying to emit.
>>> I will go back and review the categories and see if there is something
>>> that I missed.
>>
>> If there isn't a suitable warning option for a new warning, that means you
>> need to add (with documentation) a new warning option (which might then be
>> enabled by -Wall or -Wextra if appropriate; and, once in GCC, should have
>> release notes added to gcc-9/changes.html on the website).
>>
>> --
>> Joseph S. Myers
>> joseph@codesourcery.com