rsmith added a comment.
Should we also warn about `inline` variables in anonymous namespaces?
================
Comment at: clang/lib/Sema/SemaDecl.cpp:7127
<< FixItHint::CreateRemoval(D.getDeclSpec().getInlineSpecLoc());
+ } else if (SC == SC_Static && DC->isFileContext() &&
+ getLangOpts().CPlusPlus17) {
----------------
serberoth wrote:
> 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.
This diagnostic should be independent of the ext / compat diagnostic below.
This patch currently removes the `-Wc++14-compat` warning for `static inline
int n;`, which would be a behavior regression for that warning.
Moving this check inside the `else` block a few lines below, and removing the
check for `CPlusPlus17`, would seem reasonable. We accept `inline` variables in
earlier language modes as an extension, and we should still warn in that case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102663/new/
https://reviews.llvm.org/D102663
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
