Diagnosing initialization of deprecated data member?

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).

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 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.)

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).

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^!> "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.

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.

The interaction between this flag and -Wmissing-field-initializers seems unfortunate. I wonder whether it would be reasonable to suppress that warning for the case where the field is deprecated.

The deprecated field is followed by a #ifdef’d extra field:

/* these must be last and never explicitly initialized */
Py_ssize_t tp_allocs;

I think that might be OK: assuming you don’t provide an initializer for that field, you’d still get a -Wmissing-field-initializers warning in the case where the field exists, but this struct is just fundamentally incompatible with -Wmissing-field-initializers in the case where that field exists.

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.)

I suspect the Python authors do not intend for the tp_print field to be explicitly initialized, whether named or not. And given the comment on the tp_allocs field, I suspect they do not intend to support use of that header with -Wmissing-field-initializers.

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.

I don’t find this particular case to be strongly persuasive one way or the other. But I’m slightly leaning towards it being desirable to warn in this case: providing an explicit initializer (with a /tp_print/ comment, no less) seems fundamentally no different from providing a designated .tp_print= initializer, which seems fundamentally no different from providing an x.tp_print= assignment.

Indeed, not emitting -Wmissing-field-initializers for deprecated members at the end of a struct would look like the best solution in this particular case. But would it be a good choice in general, and is "deprecated members a the end of a struct" a common enough scenario to even bother? After all, maybe it is best to just leave things as they are now...