[PATCH] D136554: Implement CWG2631

Jordan Rupprecht via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 3 17:38:26 PST 2023


rupprecht accepted this revision.
rupprecht added a comment.

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

  // a.h
  static const std::pair<double, double>& GetFakePair() {
    static constexpr std::pair<double, double> kFakePair = {123.0, 456.0};
    return kFakePair;
  }
  
  // b.h
  #include "a.h"
  
  class X {
    void Foo(..., const std::pair<double, double>& x = GetFakePair()) const;
  };
  
  // main.cc, compiled with -Wunused-function
  #include "b.h"
  int main() { return 0; }

Yields this error:

  In file included from main.cc:9:
  In file included from ./b.h:16:
  ./a.h:40:41: error: 'static' function 'GetFakePair' declared in header file should be declared 'static inline' [-Werror,-Wunneeded-internal-declaration]
  static const std::pair<double, double>& GetFakePair() {

Unless I'm missing something, it seems that, in regards to this warning, this change is just enabling an existing warning to have more coverage, and therefore this change looks ready to ship. Thanks for letting me provide all my repros before landing this one again :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554



More information about the cfe-commits mailing list