[cfe-dev] Diagnosing initialization of deprecated data member?

[Stephan Bergmann via cfe-dev]

On 11/05/2020 06:55, Richard Smith wrote:
> On Thu, 7 May 2020 at 01:52, Stephan Bergmann via cfe-dev 
> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> 
>     For
> 
>      > $ cat test.cc
>      > struct S {
>      >     int x;
>      >     [[deprecated]] int y;
>      > };
>      > int main() {
>      >     S s{0, 0};
>      > }
> 
>     Clang emits a diagnostic
> 
>      > $ clang++ test.cc
>      > test.cc:6:12: warning: 'y' is deprecated [-Wdeprecated-declarations]
>      >     S s{0, 0};
>      >            ^
>      > test.cc:3:7: note: 'y' has been explicitly marked deprecated here
>      >     [[deprecated]] int y;
>      >       ^
>      > 1 warning generated.
> 
>     while e.g. GCC and MSVC do not (as checked with godbolt.org
>     <http://godbolt.org>).
> 
>     Is this deliberate behavior on Clang's part, or just a consequence of
>     how the code happens to work?  Are there opinions on whether or not the
>     behavior is useful?
> 
> 
> I think it's probably a bug. The C++11 attribute is documented as 
> deprecating the name, not the existence of the entity, so diagnosing a 
> use that doesn't use the name seems somewhat questionable. (It's not 
> obviously a bad choice, though, since the intent is very likely to catch 
> cases that would break if the field is removed, not only if it's renamed.)
> 
> There are a few other warnings channeled through the same path 
> (`DiagnoseUseOfDecl`), such as "availability" warnings, and it's quite 
> likely that some of those *should* still be issued for such cases.
> 
>     (I came across this issue when seeing a commit in LibreOffice where some
> 
>      > #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> 
>     were added apparently to silence exactly the above situation in code
>     initializing a PyTypeObject struct from
>     /usr/include/python3.8/cpython/object.h, not even realizing that that
>     pragma would only be needed by Clang and not by GCC.
> 
>     FWIW, the standard's recommended practice is "to produce a diagnostic
>     message in case the program refers to a name or entity other than to
>     declare it", [dcl.attr.deprecated]/4.)
> 
> 
> What was the intent in the LibreOffice case? Would they want a warning 
> on "S s{.x = 0, .y = 0};" but not on "S s{0, 0};"?

That Python/LibreOffice case is a bit complex, the commit message of 
<https://git.libreoffice.org/core/+/23d9966751566028c50ca95ca203d20f36c64f30%5E%21> 
"Fix initialization of Python-3.8--only at-end tp_print member" has the 
details.  The intend on the Python side, where the deprecation attribute 
got added, is apparently to warn about "initialization by assignment" 
cases like

   X.tp_print = 0;

It presumably did not intend to cause warnings in list-initialization 
scenarios as used in the LibreOffice code---I guess the re-addition of 
the dummy tp_print member at the end didn't even take into account the 
-Wmissing-field-initializers for client code using list-initialization, 
which causes client code to silence that warning by adding an 
initializer for the dummy member, but which in turn causes 
-Wdeprecated-declarations.

For designated initializer scenarios, the warning is apparently also 
useful (and likely intended by the Python authors) in C code, where it 
would warn about legacy code like

   ..., .tp_dealloc=..., .tp_print=..., .tp_getattr=..., ...

that assumes tp_print at its original position in PyTypeObject.  (For 
C++20, such code would cause an error anyway because tp_print has been 
moved relative to the other members.)

So, in short, for the particular scenario at hand, it looks like not 
emitting -Wdeprecated-declarations in the non-designated 
initializer-list scenario would be the most useful behavior.