[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D136554#4024719 <https://reviews.llvm.org/D136554#4024719>, @rupprecht wrote:

> In D136554#4024628 <https://reviews.llvm.org/D136554#4024628>, @rupprecht wrote:
>
>> I still see one behavior change (actually it was there before, but I missed it in the test results), but as far as I can tell, it's a good one? If I reduce it too much, I get the warning with the baseline toolchain, so it's not erroneous AFAICT. Although I won't pretend I know all the intricacies of `static` and `inline`.
>
> I missed another place in the unreduced repro that was referencing this method, and that was the trick. I reduced the newly-enabled warning case to this:
>
>   // a.h
>   static const std::pair<double, double>& GetFakePairRef() {
>     static constexpr std::pair<double, double> fake = {1.0, 2.0};
>     return fake;
>   }
>   
>   struct Metadata {
>     const std::pair<double, double>& data = GetFakePairRef();
>   };
>   
>   // main.cc, compiled with -Wunused-function
>   #include "a.h"
>   int main() { return 0; }
>
> Again, still LGTM.

Yes, this is a nice improvement.
It make sense. We now only mark default member initializers odr used when they are actually odr used.
default constructing an instance of MetaData silences the warning, which is expected!

I'll probably land today.
Thanks a lot for helping me find and reduce all of these corner cases, it was very useful and appreciated :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136554/new/

https://reviews.llvm.org/D136554