[clang-tools-extra] fb79ef5 - Fix readability-identifier-naming missing member variables
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 13 11:25:41 PST 2020
This breaks tests on Windows: http://45.33.8.238/win/5636/step_8.txt
Likely the usual "doesn't work with delayed template parsing" thing.
Can you take a look, and if it takes a while to fix, revert while you
investigate?
(I wouldn't said this on the phab issue, but couldn't find one.)
On Mon, Jan 13, 2020 at 1:29 PM Aaron Ballman via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
>
> Author: Nathan James
> Date: 2020-01-13T13:28:55-05:00
> New Revision: fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
>
> URL:
> https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
> DIFF:
> https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4.diff
>
> LOG: Fix readability-identifier-naming missing member variables
>
> Fixes PR41122 (missing fixes for member variables in a destructor) and
> PR29005 (does not rename class members in all locations).
>
> Added:
>
> clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
>
> Modified:
> clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
>
> Removed:
>
>
>
>
> ################################################################################
> diff --git
> a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> index bd736743ae1c..8146cb36cf21 100644
> --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
> @@ -237,10 +237,26 @@ void
> IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {
> Finder->addMatcher(namedDecl().bind("decl"), this);
> Finder->addMatcher(usingDecl().bind("using"), this);
> Finder->addMatcher(declRefExpr().bind("declRef"), this);
> - Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);
> - Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);
> +
> Finder->addMatcher(cxxConstructorDecl(unless(isImplicit())).bind("classRef"),
> + this);
> +
> Finder->addMatcher(cxxDestructorDecl(unless(isImplicit())).bind("classRef"),
> + this);
> Finder->addMatcher(typeLoc().bind("typeLoc"), this);
> Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"),
> this);
> + Finder->addMatcher(
> + functionDecl(unless(cxxMethodDecl(isImplicit())),
> +
> hasBody(forEachDescendant(memberExpr().bind("memberExpr")))),
> + this);
> + Finder->addMatcher(
> + cxxConstructorDecl(
> + unless(isImplicit()),
> + forEachConstructorInitializer(
> + allOf(isWritten(), withInitializer(forEachDescendant(
> + memberExpr().bind("memberExpr")))))),
> + this);
> + Finder->addMatcher(fieldDecl(hasInClassInitializer(
> +
> forEachDescendant(memberExpr().bind("memberExpr")))),
> + this);
> }
>
> void IdentifierNamingCheck::registerPPCallbacks(
> @@ -710,8 +726,6 @@ static void
> addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,
> void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result)
> {
> if (const auto *Decl =
> Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) {
> - if (Decl->isImplicit())
> - return;
>
> addUsage(NamingCheckFailures, Decl->getParent(),
> Decl->getNameInfo().getSourceRange());
> @@ -730,8 +744,6 @@ void IdentifierNamingCheck::check(const
> MatchFinder::MatchResult &Result) {
>
> if (const auto *Decl =
> Result.Nodes.getNodeAs<CXXDestructorDecl>("classRef")) {
> - if (Decl->isImplicit())
> - return;
>
> SourceRange Range = Decl->getNameInfo().getSourceRange();
> if (Range.getBegin().isInvalid())
> @@ -806,6 +818,14 @@ void IdentifierNamingCheck::check(const
> MatchFinder::MatchResult &Result) {
> return;
> }
>
> + if (const auto *MemberRef =
> + Result.Nodes.getNodeAs<MemberExpr>("memberExpr")) {
> + SourceRange Range = MemberRef->getMemberNameInfo().getSourceRange();
> + addUsage(NamingCheckFailures, MemberRef->getMemberDecl(), Range,
> + Result.SourceManager);
> + return;
> + }
> +
> if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {
> if (!Decl->getIdentifier() || Decl->getName().empty() ||
> Decl->isImplicit())
> return;
>
> diff --git
> a/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
> b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
> new file mode 100644
> index 000000000000..caffc3283ca2
> --- /dev/null
> +++
> b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
> @@ -0,0 +1,137 @@
> +// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
> +// RUN: -config='{CheckOptions: [ \
> +// RUN: {key: readability-identifier-naming.MemberCase, value:
> CamelCase}, \
> +// RUN: {key: readability-identifier-naming.ParameterCase, value:
> CamelCase} \
> +// RUN: ]}'
> +
> +int set_up(int);
> +int clear(int);
> +
> +class Foo {
> +public:
> + const int bar_baz; // comment-0
> + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for
> member 'bar_baz'
> + // CHECK-FIXES: {{^}} const int BarBaz; // comment-0
> +
> + Foo(int Val) : bar_baz(Val) { // comment-1
> + // CHECK-FIXES: {{^}} Foo(int Val) : BarBaz(Val) { // comment-1
> + set_up(bar_baz); // comment-2
> + // CHECK-FIXES: {{^}} set_up(BarBaz); // comment-2
> + }
> +
> + Foo() : Foo(0) {}
> +
> + ~Foo() {
> + clear(bar_baz); // comment-3
> + // CHECK-FIXES: {{^}} clear(BarBaz); // comment-3
> + }
> +
> + int getBar() const { return bar_baz; } // comment-4
> + // CHECK-FIXES: {{^}} int getBar() const { return BarBaz; } //
> comment-4
> +};
> +
> +class FooBar : public Foo {
> +public:
> + int getFancyBar() const {
> + return this->bar_baz; // comment-5
> + // CHECK-FIXES: {{^}} return this->BarBaz; // comment-5
> + }
> +};
> +
> +int getBar(const Foo &Foo) {
> + return Foo.bar_baz; // comment-6
> + // CHECK-FIXES: {{^}} return Foo.BarBaz; // comment-6
> +}
> +
> +int getBar(const FooBar &Foobar) {
> + return Foobar.bar_baz; // comment-7
> + // CHECK-FIXES: {{^}} return Foobar.BarBaz; // comment-7
> +}
> +
> +int getFancyBar(const FooBar &Foobar) {
> + return Foobar.getFancyBar();
> +}
> +
> +template <typename Dummy>
> +class TempTest : public Foo {
> +public:
> + TempTest() = default;
> + TempTest(int Val) : Foo(Val) {}
> + int getBar() const { return Foo::bar_baz; } // comment-8
> + // CHECK-FIXES: {{^}} int getBar() const { return Foo::BarBaz; } //
> comment-8
> + int getBar2() const { return this->bar_baz; } // comment-9
> + // CHECK-FIXES: {{^}} int getBar2() const { return this->BarBaz; } //
> comment-9
> +};
> +
> +TempTest<int> x; //force an instantiation
> +
> +int blah() {
> + return x.getBar2(); // gotta have a reference to the getBar2 so that the
> + // compiler will generate the function and resolve
> + // the DependantScopeMemberExpr
> +}
> +
> +namespace Bug41122 {
> +namespace std {
> +
> +// for this example we aren't bothered about how std::vector is treated
> +template <typename T> //NOLINT
> +class vector { //NOLINT
> +public:
> + void push_back(bool); //NOLINT
> + void pop_back(); //NOLINT
> +}; //NOLINT
> +}; // namespace std
> +
> +class Foo {
> + std::vector<bool> &stack;
> + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for
> member 'stack' [readability-identifier-naming]
> +public:
> + Foo(std::vector<bool> &stack)
> + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for
> parameter 'stack' [readability-identifier-naming]
> + // CHECK-FIXES: {{^}} Foo(std::vector<bool> &Stack)
> + : stack(stack) {
> + // CHECK-FIXES: {{^}} : Stack(Stack) {
> + stack.push_back(true);
> + // CHECK-FIXES: {{^}} Stack.push_back(true);
> + }
> + ~Foo() {
> + stack.pop_back();
> + // CHECK-FIXES: {{^}} Stack.pop_back();
> + }
> +};
> +}; // namespace Bug41122
> +
> +namespace Bug29005 {
> +class Foo {
> +public:
> + int a_member_of_foo = 0;
> + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for
> member 'a_member_of_foo'
> + // CHECK-FIXES: {{^}} int AMemberOfFoo = 0;
> +};
> +
> +int main() {
> + Foo foo;
> + return foo.a_member_of_foo;
> + // CHECK-FIXES: {{^}} return foo.AMemberOfFoo;
> +}
> +}; // namespace Bug29005
> +
> +namespace CtorInits {
> +template <typename T, unsigned N>
> +class Container {
> + T storage[N];
> + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for
> member 'storage'
> + // CHECK-FIXES: {{^}} T Storage[N];
> + T *pointer = &storage[0];
> + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for
> member 'pointer'
> + // CHECK-FIXES: {{^}} T *Pointer = &Storage[0];
> +public:
> + Container() : pointer(&storage[0]) {}
> + // CHECK-FIXES: {{^}} Container() : Pointer(&Storage[0]) {}
> +};
> +
> +void foo() {
> + Container<int, 5> container;
> +}
> +}; // namespace CtorInits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200113/69b5f7a6/attachment.html>
More information about the cfe-commits
mailing list