diff mbox series

[doc] correct/improve -Wmissing-attributes and -Wattribute-alias

Message ID a558b0ce-c21e-657e-b19c-0e07c842eea9@gmail.com
State New
Headers show
Series [doc] correct/improve -Wmissing-attributes and -Wattribute-alias | expand

Commit Message

Martin Sebor Feb. 6, 2019, 4:16 p.m. UTC
The manual documents the -Wno-missing-attributes form of the option
as if it was enabled by default, even though it's enabled by -Wall
(I can't get this -Wno- convention straight in my head).  I also
got private comments on the documentation of the option suggesting
to add cross-references, and to list the attributes
-Wattribute-alias considers (the same ones as -Wmissing-attributes).

The attached patch makes these changes.

Martin

Comments

Sandra Loosemore Feb. 16, 2019, 1:57 a.m. UTC | #1
On 2/6/19 9:16 AM, Martin Sebor wrote:
> The manual documents the -Wno-missing-attributes form of the option
> as if it was enabled by default, even though it's enabled by -Wall
> (I can't get this -Wno- convention straight in my head).  I also
> got private comments on the documentation of the option suggesting
> to add cross-references, and to list the attributes
> -Wattribute-alias considers (the same ones as -Wmissing-attributes).
> 
> The attached patch makes these changes.

I found the discussion of both options incomprehensible even with this 
patch.  :-(  The defaults are incorrect, there are typos, awkward 
wording and confusing paragraph organization, etc.  So I consulted the 
sources and came up with the attached alternative patch.  Can you review 
this for correctness and generally making sense?

-Sandra
2019-02-15  Sandra Loosemore  <sandra@codesourcery.com>
	    Martin Sebor  <msebor@gmail.com>

	gcc/
	* c-family/c.opt (Wmissing-attributes): Clean up doc string.
	* common.opt (Wattribute-alias): Likewise.
	* doc/invoke.texi (Option Summary): List general form of
	-Wattribute-alias=.  List positive form of -Wmissing-attributes.
	(-Wmissing-attributes): Invert entry, rewrite and correct default.
	Add cross-references.
	(-Wattribute-alias): Rewrite and correct default.  Mention
	considered attributes (same as for -Wmissing-attributes).
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 268948)
+++ gcc/c-family/c.opt	(working copy)
@@ -818,7 +818,7 @@ Warn on primary template declaration.
 Wmissing-attributes
 C ObjC C++ ObjC++ Var(warn_missing_attributes) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about declarations of entities that may be missing attributes
-that related entities have been declared with it.
+that related entities have been declared with.
 
 Wmissing-format-attribute
 C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format)
Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 268948)
+++ gcc/common.opt	(working copy)
@@ -552,11 +552,11 @@ Warn about inappropriate attribute usage
 
 Wattribute-alias
 Common Alias(Wattribute_alias=, 1, 0) Warning
-Warn about type safety and similar errors and mismatches in attribute alias and related.
+Warn about type safety and similar errors and mismatches in declarations with alias attributes.
 
 Wattribute-alias=
 Common Joined RejectNegative UInteger Var(warn_attribute_alias) Init(1) Warning IntegerRange(0, 2)
-Warn about type safety and similar errors and mismatches in attribute alias and related.
+Warn about type safety and similar errors and mismatches in declarations with alias attributes.
 
 Wcannot-profile
 Common Var(warn_cannot_profile) Init(1) Warning
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 268948)
+++ gcc/doc/invoke.texi	(working copy)
@@ -288,7 +288,7 @@ Objective-C and Objective-C++ Dialects}.
 -Walloc-zero  -Walloc-size-larger-than=@var{byte-size} @gol
 -Walloca  -Walloca-larger-than=@var{byte-size} @gol
 -Wno-aggressive-loop-optimizations  -Warray-bounds  -Warray-bounds=@var{n} @gol
--Wno-attributes  -Wno-attribute-alias @gol
+-Wno-attributes  -Wattribute-alias=@var{n}  @gol
 -Wbool-compare  -Wbool-operation @gol
 -Wno-builtin-declaration-mismatch @gol
 -Wno-builtin-macro-redefined  -Wc90-c99-compat  -Wc99-c11-compat @gol
@@ -322,7 +322,7 @@ Objective-C and Objective-C++ Dialects}.
 -Winvalid-pch  -Wlarger-than=@var{byte-size} @gol
 -Wlogical-op  -Wlogical-not-parentheses  -Wlong-long @gol
 -Wmain  -Wmaybe-uninitialized  -Wmemset-elt-size  -Wmemset-transposed-args @gol
--Wmisleading-indentation  -Wno-missing-attributes  -Wmissing-braces @gol
+-Wmisleading-indentation  -Wmissing-attributes  -Wmissing-braces @gol
 -Wmissing-field-initializers  -Wmissing-format-attribute @gol
 -Wmissing-include-dirs  -Wmissing-noreturn  -Wmissing-profile @gol
 -Wno-multichar  -Wmultistatement-macros  -Wnonnull  -Wnonnull-compare @gol
@@ -5056,7 +5056,7 @@ about the layout of the file that the di
 
 This warning is enabled by @option{-Wall} in C and C++.
 
-@item -Wno-missing-attributes
+@item -Wmissing-attributes
 @opindex Wmissing-attributes
 @opindex Wno-missing-attributes
 Warn when a declaration of a function is missing one or more attributes
@@ -5064,10 +5064,10 @@ that a related function is declared with
 affect the correctness or efficiency of generated code.  For example,
 the warning is issued for declarations of aliases that use attributes
 to specify less restrictive requirements than those of their targets.
-This typically represents a potential optimization oportunity rather
-than a hidden bug.  The @option{-Wattribute-alias} option controls warnings
-issued for mismatches between declarations of aliases and their targets
-that might be indicative of code generation bugs.
+This typically represents a potential optimization opportunity.
+By contrast, the @option{-Wattribute-alias=2} option controls warnings
+issued when the alias is more restrictive than the target, which could
+lead to incorrect code generation.
 Attributes considered include @code{alloc_align}, @code{alloc_size},
 @code{cold}, @code{const}, @code{hot}, @code{leaf}, @code{malloc},
 @code{nonnull}, @code{noreturn}, @code{nothrow}, @code{pure},
@@ -5080,6 +5080,13 @@ or @code{nonnull} is declared without it
 @code{error}, and @code{warning} suppress the warning.
 (@pxref{Function Attributes}).
 
+You can use the @code{copy} attribute to apply the same
+set of attributes to a declaration as that on another declaration without
+explicitly enumerating the attributes. This attribute can be applied
+to declarations of functions (@pxref{Common Function Attributes}),
+variables (@pxref{Common Variable Attributes}), or types
+(@pxref{Common Type Attributes}).
+
 @option{-Wmissing-attributes} is enabled by @option{-Wall}.
 
 For example, since the declaration of the primary function template
@@ -6104,7 +6111,6 @@ false positives and is deactivated by de
 Warn about declarations using the @code{alias} and similar attributes whose
 target is incompatible with the type of the alias.
 @xref{Function Attributes,,Declaring Attributes of Functions}.
-The @option{-Wattribute-alias=1} is  enabled by @option{-Wall}.
 
 @table @gcctabopt
 @item -Wattribute-alias=1
@@ -6113,13 +6119,26 @@ incompatibilities between the type of th
 target.  Such incompatibilities are typically indicative of bugs.
 
 @item -Wattribute-alias=2
-At this level @option{-Wattribute-alias} also diagnoses mismatches between
-the set of attributes of the alias declaration and the attributes applied
-to its target.  Although in some cases such mismatches may indicate bugs,
-in other cases they may be benign and could be resolved simply by adding
-the missing attribute to the target.
+
+At this level @option{-Wattribute-alias} also diagnoses cases where
+the attributes of the alias declaration are more restrictive than the
+attributes applied to its target.  These mismatches can potentially
+result in incorrect code generation.  In other cases they may be
+benign and could be resolved simply by adding the missing attribute to
+the target.  For comparison, see the @option{-Wmissing-attributes}
+option, which controls diagnostics when the alias declaration is less
+restrictive than the target, rather than more restrictive.
+
+Attributes considered include @code{alloc_align}, @code{alloc_size},
+@code{cold}, @code{const}, @code{hot}, @code{leaf}, @code{malloc},
+@code{nonnull}, @code{noreturn}, @code{nothrow}, @code{pure},
+@code{returns_nonnull}, and @code{returns_twice}.
 @end table
 
+@option{-Wattribute-alias} is equivalent to @option{-Wattribute-alias=1}.
+This is the default.  You can disable these warnings with either
+@option{-Wno-attribute-alias} or @option{-Wattribute-alias=0}.
+
 @item -Wbool-compare
 @opindex Wno-bool-compare
 @opindex Wbool-compare
Sandra Loosemore Feb. 25, 2019, 3:31 a.m. UTC | #2
On 2/15/19 6:57 PM, Sandra Loosemore wrote:
> On 2/6/19 9:16 AM, Martin Sebor wrote:
>> The manual documents the -Wno-missing-attributes form of the option
>> as if it was enabled by default, even though it's enabled by -Wall
>> (I can't get this -Wno- convention straight in my head).  I also
>> got private comments on the documentation of the option suggesting
>> to add cross-references, and to list the attributes
>> -Wattribute-alias considers (the same ones as -Wmissing-attributes).
>>
>> The attached patch makes these changes.
> 
> I found the discussion of both options incomprehensible even with this 
> patch.  :-(  The defaults are incorrect, there are typos, awkward 
> wording and confusing paragraph organization, etc.  So I consulted the 
> sources and came up with the attached alternative patch.  Can you review 
> this for correctness and generally making sense?
> 
> 2019-02-15  Sandra Loosemore  <sandra@codesourcery.com>
> 	    Martin Sebor  <msebor@gmail.com>
> 
> 	gcc/
> 	* c-family/c.opt (Wmissing-attributes): Clean up doc string.
> 	* common.opt (Wattribute-alias): Likewise.
> 	* doc/invoke.texi (Option Summary): List general form of
> 	-Wattribute-alias=.  List positive form of -Wmissing-attributes.
> 	(-Wmissing-attributes): Invert entry, rewrite and correct default.
> 	Add cross-references.
> 	(-Wattribute-alias): Rewrite and correct default.  Mention
> 	considered attributes (same as for -Wmissing-attributes).

Any thoughts on this patch?  I'll go ahead and commit it if I don't hear 
any objection in the next couple of days....  it can't be any worse than 
the existing documentation.  :-P

-Sandra
Martin Sebor Feb. 25, 2019, 5:27 p.m. UTC | #3
On 2/15/19 6:57 PM, Sandra Loosemore wrote:
> On 2/6/19 9:16 AM, Martin Sebor wrote:
>> The manual documents the -Wno-missing-attributes form of the option
>> as if it was enabled by default, even though it's enabled by -Wall
>> (I can't get this -Wno- convention straight in my head).  I also
>> got private comments on the documentation of the option suggesting
>> to add cross-references, and to list the attributes
>> -Wattribute-alias considers (the same ones as -Wmissing-attributes).
>>
>> The attached patch makes these changes.
> 
> I found the discussion of both options incomprehensible even with this 
> patch.  :-(  The defaults are incorrect, there are typos, awkward 
> wording and confusing paragraph organization, etc.  So I consulted the 
> sources and came up with the attached alternative patch.  Can you review 
> this for correctness and generally making sense?

Thanks.  I think your update is fine.  There's some overlap between
the -Wattribute-alias and -Wmissing-attributes warnings, as well
as some gaps in one or both, that could stand to be reviewed and
adjusted.  I'm also don't think that diagnosing likely bugs only
at the optional level 2 and diagnosing potential optimization
oportunities at the default level 1 was the right decision.  But
that will have to wait until GCC 10.

Martin

Index: gcc/common.opt
...
@@ -552,11 +552,11 @@ Warn about inappropriate attribute usage

  Wattribute-alias
  Common Alias(Wattribute_alias=, 1, 0) Warning
-Warn about type safety and similar errors and mismatches in attribute 
alias and related.
+Warn about type safety and similar errors and mismatches in 
declarations with alias attributes.

The "and related" part refers to other attributes that are similar to
but distinct from alias.  I think the only such attribute is weakref.
It's probably not essential to make this one sentence 100% accurate
but I thought I'd mention it just for clarity.
diff mbox series

Patch

gcc/ChangeLog:

	* doc/invoke.texi (-Wmissing-attributes): Invert entry.
	Add cross-references.
	(-Wattribute-alias): Mention considered attributes (same
	as for -Wmissing-attributes).

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 268583)
+++ gcc/doc/invoke.texi	(working copy)
@@ -5055,10 +5055,10 @@  about the layout of the file that the directive re
 
 This warning is enabled by @option{-Wall} in C and C++.
 
-@item -Wno-missing-attributes
+@item -Wmissing-attributes
 @opindex Wmissing-attributes
 @opindex Wno-missing-attributes
-Warn when a declaration of a function is missing one or more attributes
+Do not warn when a declaration of a function is missing one or more attributes
 that a related function is declared with and whose absence may adversely
 affect the correctness or efficiency of generated code.  For example,
 the warning is issued for declarations of aliases that use attributes
@@ -5079,6 +5079,11 @@  or @code{nonnull} is declared without it.  Attribu
 @code{error}, and @code{warning} suppress the warning.
 (@pxref{Function Attributes}).
 
+The the @code{copy} attribute provides a mechanism for applying the same
+set of attributes to a declaration as that on another declaration without
+explicitly enumerating the attributes. (@pxref{Function Attributes}.
+@pxref{Variable Attributes}.  @pxref{Variable Attributes}.)
+
 @option{-Wmissing-attributes} is enabled by @option{-Wall}.
 
 For example, since the declaration of the primary function template
@@ -6101,9 +6106,14 @@  false positives and is deactivated by default.
 @opindex -Wattribute-alias
 @opindex -Wno-attribute-alias
 Warn about declarations using the @code{alias} and similar attributes whose
-target is incompatible with the type of the alias.
+target is incompatible with the type of the alias.  Attributes considered
+include @code{alloc_align}, @code{alloc_size}, @code{cold}, @code{const},
+@code{hot}, @code{leaf}, @code{malloc}, @code{nonnull}, @code{noreturn},
+@code{nothrow}, @code{pure}, @code{returns_nonnull}, and @code{returns_twice}.
+
 @xref{Function Attributes,,Declaring Attributes of Functions}.
-The @option{-Wattribute-alias=1} is  enabled by @option{-Wall}.
+The @option{-Wattribute-alias=1} option is enabled by @option{-Wall}.
+See also the related @option{-Wmissing-attributes} option.
 
 @table @gcctabopt
 @item -Wattribute-alias=1