carlosgalvezp added inline comments.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp:46-48
+// OK
+static const int v8{123};
+static constexpr int v9{123};
----------------
vingeldal wrote:
> Is it really the best behavior to allow these? If I got the rationale right
> we don't warn about this because const and constexpr have implicit internal
> linkage anyway, so static doesn't make a difference, right?
> Reading the documentation for this check I gather static would probably have
> been deprecated if it wasn't for the fact that deprecation would have broken
> compatibility with C. So, if we drop the static keyword here we still get the
> behavior we want, without a confusing keyword we would rather get rid of if
> we could, while keeping compatibility with C.
>
> I'm thinking it could be better to just discourage from using static for
> cases like this.
Thanks for the review!
Yes, that example is "bad" code, but this particular check would currently ask
users to move the variables to the anonymous namespace - which is also
incorrect.
My plan is to add one more separate checks in the "readability" section that
warn about redundant usages of 'static' (like this one), and even supports
automatic fixes (which aren't possible in this check).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139113/new/
https://reviews.llvm.org/D139113
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
