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