GCC/Clang attributes guiding warnings about unused entities

GCC/Clang attributes guiding warnings about unused entities

[Stephan Bergmann]

[cross-posting this to both the GCC and Clang communities]

I have two specific issues on the subject I would like to discuss below. 
  But first a quick overview:

C++ has two attributes dealing with warnings about unused entities:

* In one direction (making the compiler suppress warnings it would 
otherwise emit), [[maybe_unused]] (which can be used on lots of 
different kinds of entities) is meant to suppress warnings about 
otherwise unused entities.

* In the other direction (making the compiler emit more warnings than it 
could otherwise infer), [[nodiscard]] (which can be used on certain 
types, functions, and constructors) is meant to cause warnings about 
discarded results of function calls or constructor invocations.  (For a 
[[nodiscard]] type, it warns about discarded results of calls to 
functions returning that type.  For a [[nodiscard]] function, it warns 
about discarded results of calls to that function.  For a [[nodiscard]] 
constructor, it warns about discarded objects initialized via that 
constructor.)

GCC and (through its GCC compatibility) Clang have three additional 
non-standard attributes:

* __attribute__((unused)) behaves mostly the same as [[maybe_unused]].

The one difference is that __attribute__((unused)) applied to a type 
does not warn about that type being unused, but rather warns about 
unreferenced variables of that type.  And it appears that both GCC and 
Clang implement [[maybe_unused]] with the same semantics as 
__attribute__((unused)), and cause [[maybe_unused]] applied to a type to 
warn about unreferenced variables of that type.  The mailing list thread 
starting at <https://lists.isocpp.org/std-discussion/2023/04/2158.php> 
"[[maybe_unused]] classes" discussed this, and argues that those special 
semantics of __attribute__((unused)) and [[maybe_unused]] applied to a 
type are not actually useful:  The GCC documentation cites as a use case 
"lock or thread classes, which are usually defined and then not 
referenced, but contain constructors and destructors that have 
nontrivial bookkeeping functions".  But the presence of those 
non-trivial con-/destructors will already prevent the compiler from 
emitting warnings about seemingly unused variables of such types.  So 
applying __attribute__((unused)) to such types looks like it does not 
bring any additional value.  Or what do other people think?

* __attribute__((warn_unused_result)) (which can only be applied to 
functions) behaves the same as [[nodiscard]] applied to functions.

* __attribute__((warn_unused)) (which can be applied to class types) is 
meant to allow warnings about unreferenced variables of the given type, 
where the compiler could otherwise not infer that those variables are 
truely unused (because the type has non-trivial con-/destructors). 
(This attribute does not have a standard counterpart.)

Similarly to how [[nodiscard]] can be applied to individual 
constructors, it looks beneficial to me to allow 
__attribute__((warn_unused)) to be applied to individual constructors, 
too.  One example use case is a RAII class that has one constructor that 
does not acquire a resource (often the default constructor) and another 
constructor that does acquire a resource.  So the class itself cannot be 
marked __attribute__((warn_unused)).  But if the non-acquiring 
constructor could individually be marked __attribute__((warn_unused)), 
the compiler could warn about unreferenced variables that are 
initialized via that constructor.  <https://reviews.llvm.org/D148505> 
"Allow `__attribute__((warn_unused))` on individual constructors" would 
implement that for Clang---but was met with some reservation for now, 
due to the already somewhat confusing landscape of standard and 
GCC/Clang-specific attributes guiding warnings about unused entities as 
outlined in this post.  What do other people think about it?  Would it 
be something that GCC would also want to implement?

Re: GCC/Clang attributes guiding warnings about unused entities

[Florian Weimer]

* Stephan Bergmann via Gcc:

> [cross-posting this to both the GCC and Clang communities]

I don't see your post here:

  <https://discourse.llvm.org/search?expanded=true&q=unused%20after%3A2023-04-20>

I don't think this is expected to work from a Discourse point of view.

> * __attribute__((unused)) behaves mostly the same as [[maybe_unused]].
>
> The one difference is that __attribute__((unused)) applied to a type
> does not warn about that type being unused, but rather warns about
> unreferenced variables of that type.  And it appears that both GCC and
> Clang implement [[maybe_unused]] with the same semantics as
> __attribute__((unused)), and cause [[maybe_unused]] applied to a type
> to warn about unreferenced variables of that type.  The mailing list
> thread starting at
> <https://lists.isocpp.org/std-discussion/2023/04/2158.php>
> "[[maybe_unused]] classes" discussed this, and argues that those
> special semantics of __attribute__((unused)) and [[maybe_unused]]
> applied to a type are not actually useful:  The GCC documentation
> cites as a use case "lock or thread classes, which are usually defined
> and then not referenced, but contain constructors and destructors that
> have nontrivial bookkeeping functions".  But the presence of those
> non-trivial con-/destructors will already prevent the compiler from
> emitting warnings about seemingly unused variables of such types.  So
> applying __attribute__((unused)) to such types looks like it does not
> bring any additional value.  Or what do other people think?

Not sure if I quite understand this.  If the attribute cannot be used to
mark (indirectly) variables of type (say) std::string as worthy of
warnings if they are unused, I think these special cases are not useful.

The assumption might be that compilers need to suppress warnings for
RAII guard variables, but GCC does not warn for them anyway.  This is
discussed occasionally, and the assumption is that such warnings would
not be useful.

> * __attribute__((warn_unused_result)) (which can only be applied to
>   functions) behaves the same as [[nodiscard]] applied to functions.

This is not entirely accurate because warn_unused_result is much harder
to suppress at the call site (at least in GCC).

> * __attribute__((warn_unused)) (which can be applied to class types)
>   is meant to allow warnings about unreferenced variables of the given
>   type, where the compiler could otherwise not infer that those
>   variables are truely unused (because the type has non-trivial
>   con-/destructors). (This attribute does not have a standard
>  counterpart.)
>
> Similarly to how [[nodiscard]] can be applied to individual
> constructors, it looks beneficial to me to allow
> __attribute__((warn_unused)) to be applied to individual constructors,
> too.  One example use case is a RAII class that has one constructor
> that does not acquire a resource (often the default constructor) and
> another constructor that does acquire a resource.  So the class itself
> cannot be marked __attribute__((warn_unused)).  But if the
> non-acquiring constructor could individually be marked
> __attribute__((warn_unused)), the compiler could warn about
> unreferenced variables that are initialized via that constructor.
> <https://reviews.llvm.org/D148505> "Allow
> `__attribute__((warn_unused))` on individual constructors" would
> implement that for Clang---but was met with some reservation for now,
> due to the already somewhat confusing landscape of standard and
> GCC/Clang-specific attributes guiding warnings about unused entities
> as outlined in this post.  What do other people think about it?  Would
> it be something that GCC would also want to implement?

How does this interact with deriving warn_unused for std::vector<T> if T
is warn_unused?  That seems like a useful feature, and appears to
require that this is a type property, not a constructor property.

And maybe there is a trend to use constructor functions for these guard
variables and auto?  So

  auto g = make_guard(obj);

instead of:

  guard<decl_type(obj)> g(obj);

Thanks,
Florian

Re: GCC/Clang attributes guiding warnings about unused entities

[Stephan Bergmann]

On 4/28/23 11:55, Florian Weimer wrote:
> * Stephan Bergmann via Gcc:
> 
>> [cross-posting this to both the GCC and Clang communities]
> 
> I don't see your post here:
> 
>    <https://discourse.llvm.org/search?expanded=true&q=unused%20after%3A2023-04-20>
> 
> I don't think this is expected to work from a Discourse point of view.

Yeah, that experiment apparently failed miserably, unfortunately.

>> * __attribute__((unused)) behaves mostly the same as [[maybe_unused]].
>>
>> The one difference is that __attribute__((unused)) applied to a type
>> does not warn about that type being unused, but rather warns about
>> unreferenced variables of that type.  And it appears that both GCC and
>> Clang implement [[maybe_unused]] with the same semantics as
>> __attribute__((unused)), and cause [[maybe_unused]] applied to a type
>> to warn about unreferenced variables of that type.  The mailing list
>> thread starting at
>> <https://lists.isocpp.org/std-discussion/2023/04/2158.php>
>> "[[maybe_unused]] classes" discussed this, and argues that those
>> special semantics of __attribute__((unused)) and [[maybe_unused]]
>> applied to a type are not actually useful:  The GCC documentation
>> cites as a use case "lock or thread classes, which are usually defined
>> and then not referenced, but contain constructors and destructors that
>> have nontrivial bookkeeping functions".  But the presence of those
>> non-trivial con-/destructors will already prevent the compiler from
>> emitting warnings about seemingly unused variables of such types.  So
>> applying __attribute__((unused)) to such types looks like it does not
>> bring any additional value.  Or what do other people think?
> 
> Not sure if I quite understand this.  If the attribute cannot be used to
> mark (indirectly) variables of type (say) std::string as worthy of
> warnings if they are unused, I think these special cases are not useful.

Not sure if I in turn understand you here.  __attribute__((unused)) and 
[[maybe_unused]] are not meant to "mark [...] variables [...] as worthy 
of warnings if they are unused" anyway?

>> * __attribute__((warn_unused)) (which can be applied to class types)
>>    is meant to allow warnings about unreferenced variables of the given
>>    type, where the compiler could otherwise not infer that those
>>    variables are truely unused (because the type has non-trivial
>>    con-/destructors). (This attribute does not have a standard
>>   counterpart.)
>>
>> Similarly to how [[nodiscard]] can be applied to individual
>> constructors, it looks beneficial to me to allow
>> __attribute__((warn_unused)) to be applied to individual constructors,
>> too.  One example use case is a RAII class that has one constructor
>> that does not acquire a resource (often the default constructor) and
>> another constructor that does acquire a resource.  So the class itself
>> cannot be marked __attribute__((warn_unused)).  But if the
>> non-acquiring constructor could individually be marked
>> __attribute__((warn_unused)), the compiler could warn about
>> unreferenced variables that are initialized via that constructor.
>> <https://reviews.llvm.org/D148505> "Allow
>> `__attribute__((warn_unused))` on individual constructors" would
>> implement that for Clang---but was met with some reservation for now,
>> due to the already somewhat confusing landscape of standard and
>> GCC/Clang-specific attributes guiding warnings about unused entities
>> as outlined in this post.  What do other people think about it?  Would
>> it be something that GCC would also want to implement?
> 
> How does this interact with deriving warn_unused for std::vector<T> if T
> is warn_unused?  That seems like a useful feature, and appears to
> require that this is a type property, not a constructor property.

Applying __attribute__((warn_unused)) to types would still be possible. 
The only change would be that, in addition, also individual constructors 
can be marked __attribute__((warn_unused)) (to be used in cases where 
the whole class type can't be).

So this would be largely orthogonal to deriving warn_unused for 
std::vector<T> from T.  (Do we even have such a feature?)  (One could, 
of course, envision an additional feature where warn_unused for the 
construction of a std::vector<T> variable could be derived from whether 
the T constructors used to initialize all the individual vector elements 
are warn_unused.)

> And maybe there is a trend to use constructor functions for these guard
> variables and auto?  So
> 
>    auto g = make_guard(obj);
> 
> instead of:
> 
>    guard<decl_type(obj)> g(obj);

I'm not sure I understand what you want to bring up with this example. 
If guard<T>(T) were a constructor that could benefit from being marked 
__attribute__((warn_unused)) (which it usually wouldn't be, though), 
then, yes, wrapping that in a make_guard function would generally stop 
the compiler from being able to warn about the unused variable g.  (As 
the knowledge which constructor had been used inside make_guard is 
generally lost at the point where the variable g is initialized; and not 
to mention intervening copy/move constructor invocations).

(Maybe, to make things more clear:  An example where my proposed 
per-constructor __attribute__((warn_unused)) was helpful to find an 
actual error in code is 
<https://git.libreoffice.org/core/+/7cdbe504ff3b59d3aec1b1e86caf7f24223dce72%5E!> 
"Fix what looks like copy/paste typos":

>      uno::Reference<beans::XPropertySet> xSect1(xSections->getByIndex(1), uno::UNO_QUERY);
>      uno::Sequence<sal_Int8> const key1(getProperty<uno::Sequence<sal_Int8>>(xSect1, "ProtectionKey"));
>      CPPUNIT_ASSERT(SvPasswordHelper::CompareHashPassword(key1, password));
>      uno::Reference<beans::XPropertySet> xSect2(xSections->getByIndex(2), uno::UNO_QUERY);
> -    uno::Sequence<sal_Int8> const key2(getProperty<uno::Sequence<sal_Int8>>(xSect1, "ProtectionKey"));
> +    uno::Sequence<sal_Int8> const key2(getProperty<uno::Sequence<sal_Int8>>(xSect2, "ProtectionKey"));
>      CPPUNIT_ASSERT(SvPasswordHelper::CompareHashPassword(key2, password));
>      uno::Reference<beans::XPropertySet> xSect3(xSections->getByIndex(3), uno::UNO_QUERY);
> -    uno::Sequence<sal_Int8> const key3(getProperty<uno::Sequence<sal_Int8>>(xSect1, "ProtectionKey"));
> +    uno::Sequence<sal_Int8> const key3(getProperty<uno::Sequence<sal_Int8>>(xSect3, "ProtectionKey"));
>      CPPUNIT_ASSERT(SvPasswordHelper::CompareHashPassword(key3, password));

where the variables xSect2 and xSect3 had been identified as unused (as 
their intended uses had erroneously used xSect1 instead), as they 
happened to be initialized by a constructor that could be marked 
__attribute__((warn_unused)).)