<div dir="ltr"><div dir="ltr">On Tue, 12 May 2020 at 00:48, Stephan Bergmann via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 11/05/2020 06:55, Richard Smith wrote:<br>
> On Thu, 7 May 2020 at 01:52, Stephan Bergmann via cfe-dev <br>
> <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a> <mailto:<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>>> wrote:<br>
> <br>
> For<br>
> <br>
> > $ cat test.cc<br>
> > struct S {<br>
> > int x;<br>
> > [[deprecated]] int y;<br>
> > };<br>
> > int main() {<br>
> > S s{0, 0};<br>
> > }<br>
> <br>
> Clang emits a diagnostic<br>
> <br>
> > $ clang++ test.cc<br>
> > test.cc:6:12: warning: 'y' is deprecated [-Wdeprecated-declarations]<br>
> > S s{0, 0};<br>
> > ^<br>
> > test.cc:3:7: note: 'y' has been explicitly marked deprecated here<br>
> > [[deprecated]] int y;<br>
> > ^<br>
> > 1 warning generated.<br>
> <br>
> while e.g. GCC and MSVC do not (as checked with <a href="http://godbolt.org" rel="noreferrer" target="_blank">godbolt.org</a><br>
> <<a href="http://godbolt.org" rel="noreferrer" target="_blank">http://godbolt.org</a>>).<br>
> <br>
> Is this deliberate behavior on Clang's part, or just a consequence of<br>
> how the code happens to work? Are there opinions on whether or not the<br>
> behavior is useful?<br>
> <br>
> <br>
> I think it's probably a bug. The C++11 attribute is documented as <br>
> deprecating the name, not the existence of the entity, so diagnosing a <br>
> use that doesn't use the name seems somewhat questionable. (It's not <br>
> obviously a bad choice, though, since the intent is very likely to catch <br>
> cases that would break if the field is removed, not only if it's renamed.)<br>
> <br>
> There are a few other warnings channeled through the same path <br>
> (`DiagnoseUseOfDecl`), such as "availability" warnings, and it's quite <br>
> likely that some of those *should* still be issued for such cases.<br>
> <br>
> (I came across this issue when seeing a commit in LibreOffice where some<br>
> <br>
> > #pragma GCC diagnostic ignored "-Wdeprecated-declarations"<br>
> <br>
> were added apparently to silence exactly the above situation in code<br>
> initializing a PyTypeObject struct from<br>
> /usr/include/python3.8/cpython/object.h, not even realizing that that<br>
> pragma would only be needed by Clang and not by GCC.<br>
> <br>
> FWIW, the standard's recommended practice is "to produce a diagnostic<br>
> message in case the program refers to a name or entity other than to<br>
> declare it", [dcl.attr.deprecated]/4.)<br>
> <br>
> <br>
> What was the intent in the LibreOffice case? Would they want a warning <br>
> on "S s{.x = 0, .y = 0};" but not on "S s{0, 0};"?<br>
<br>
That Python/LibreOffice case is a bit complex, the commit message of <br>
<<a href="https://git.libreoffice.org/core/+/23d9966751566028c50ca95ca203d20f36c64f30%5E%21" rel="noreferrer" target="_blank">https://git.libreoffice.org/core/+/23d9966751566028c50ca95ca203d20f36c64f30%5E%21</a>> <br>
"Fix initialization of Python-3.8--only at-end tp_print member" has the <br>
details. The intend on the Python side, where the deprecation attribute <br>
got added, is apparently to warn about "initialization by assignment" <br>
cases like<br>
<br>
X.tp_print = 0;<br>
<br>
It presumably did not intend to cause warnings in list-initialization <br>
scenarios as used in the LibreOffice code---I guess the re-addition of <br>
the dummy tp_print member at the end didn't even take into account the <br>
-Wmissing-field-initializers for client code using list-initialization, <br>
which causes client code to silence that warning by adding an <br>
initializer for the dummy member, but which in turn causes <br>
-Wdeprecated-declarations.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>The deprecated field is followed by a #ifdef'd extra field:</div><div><br></div><div>> /* these must be last and never explicitly initialized */<br>> Py_ssize_t tp_allocs;<br></div><div><br></div><div>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.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
For designated initializer scenarios, the warning is apparently also <br>
useful (and likely intended by the Python authors) in C code, where it <br>
would warn about legacy code like<br>
<br>
..., .tp_dealloc=..., .tp_print=..., .tp_getattr=..., ...<br>
<br>
that assumes tp_print at its original position in PyTypeObject. (For <br>
C++20, such code would cause an error anyway because tp_print has been <br>
moved relative to the other members.)<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
So, in short, for the particular scenario at hand, it looks like not <br>
emitting -Wdeprecated-declarations in the non-designated <br>
initializer-list scenario would be the most useful behavior.<br></blockquote><div><br></div><div>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.</div></div></div>