AW: AW: Wonly-top-basic-asm

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

On 11.2.2016, David Wohlferd wrote:
>
> Since no one expressed any objections, I have renamed the option from
> -Wonly-top-basic-asm to -Wbasic-asm-in-function.  This more clearly
> conveys what the option does (give a warning if you find basic asm in a
> function).
> 

why not simply -Wbasic-asm ?

> I believe the attached patch addresses all the other outstanding comments.
> 

Indentation wrong here. The whole block must be indented by 2 spaces.

>   if (c_parser_next_token_is (parser, CPP_CLOSE_PAREN) && !is_goto)
>+  {
>+    /* Warn on basic asm used inside of functions,
>+       EXCEPT when in naked functions.  Also allow asm (""). */

Comments should end with dot space space */

>+    if (warn_basic_asm_in_function && TREE_STRING_LENGTH (str) != 1)
>+      if (lookup_attribute ("naked",
>+			    DECL_ATTRIBUTES (current_function_decl))

the DECL_ATTRIBUTES should be at the same column as the "naked".

>+	  == NULL_TREE)
>+	warning_at (asm_loc, OPT_Wbasic_asm_in_function,
>+		    "asm statement in function does not use extended syntax");
>+
>     goto done_asm;
>+  }

>@@ -18199,6 +18201,17 @@
> 	  /* If the extended syntax was not used, mark the ASM_EXPR.  */
> 	  if (!extended_p)
> 	    {
>+	      /* Warn on basic asm used inside of functions,
>+		 EXCEPT when in naked functions.  Also allow asm (""). */

Comments should end with dot space space */

>+	      if (warn_basic_asm_in_function
>+		  && TREE_STRING_LENGTH (string) != 1)
>+		if (lookup_attribute ("naked",
>+				     DECL_ATTRIBUTES (current_function_decl))

the DECL_ATTRIBUTES should be at the same column as the "naked".

> ChangeLog:
> 2016-02-10  David Wohlferd  <dw@LimeGreenSocks.com>
> 
>     * doc/extend.texi: Doc basic asm behavior and new
>     -Wbasic-asm-in-function option.
>      * doc/invoke.texi: Doc new -Wbasic-asm-in-function option.
>      * c-family/c.opt: Define -Wbasic-asm-in-function.
>      * c/c-parser.c: Implement -Wbasic-asm-in-function for C.
>      * cp/parser.c: Implement -Wbasic-asm-in-function for c++.

C++, isn't it always upper case?

>      * testsuite/c-c++-common/Wbasic-asm-in-function.c: New tests for
>      -Wbasic-asm-in-function.
>      * testsuite/c-c++-common/Wbasic-asm-in-function-2.c: Ditto.
> 

ChangeLog lines begin with TAB.

Please split the ChangeLog, there are separate ChangeLogs
at gcc/ChangeLog (doc changes go in there)
gcc/c/ChangeLog, gcc/cp/ChangeLog, gcc/c-family/ChangLog
and gcc/testsuite/ChangeLog, the respective ChangeLog entries
use relative file names.

Please add the function name where you changed in brackets.

For instance:
    * c-parser.c (cp_parser_asm_definition): Implement -Wbasic-asm-in-function.


Thanks
Bernd.

> While I have a release on file with FSF, I don't have write access to SVN.
> 
> dw