[PATCH] D102663: Bug 49633 - Added warning for static inline global and namespaced declarations for c++17+

Fri May 21 13:40:16 PDT 2021

serberoth added inline comments.

================
Comment at: clang/lib/Sema/SemaDecl.cpp:7127
         << FixItHint::CreateRemoval(D.getDeclSpec().getInlineSpecLoc());
+    } else if (SC == SC_Static && DC->isFileContext() &&
+               getLangOpts().CPlusPlus17) {
----------------
erichkeane wrote:
> First, what about this is C++17 specific?  The inline and static relationship is older than C++17, right?
> 
> Also, are you sure that the warning/stuff in the 'else' isn't still valuable?
Perhaps I misunderstood something in the bug write-up, the component for the ticket is C++17.  Also there is the warning that `inline variables are a C++17 extension` that appears when you use the inline keyword on a variable (regardless of the appearance of the static keyword).  I suppose that does not necessarily mean we cannot simply show both warnings, and maybe that is the right thing to do.  I felt that was not really necessary because of the other warning.

As for the other warnings in the else the one is the warning that I mentioned (which only applies when the C++17 standard is not applied, and the other is a C++14 compatibility warning which states:
"inline variables are incompatible with C++ standards before C++17"

You can see the messages for those diagnostic message here:
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticSemaKinds.td#L1467

Please let me know if I am still missing something with how this diagnostic was supposed to apply and where.  Thank you.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102663