Great, thanks! On Mon, Jan 13, 2020 at 4:04 PM Aaron Ballman <aa...@aaronballman.com> wrote:
> On Mon, Jan 13, 2020 at 2:25 PM Nico Weber <tha...@chromium.org> wrote: > > > > 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.) > > Thank you for pointing this out, it's fixed in > c1b13a1b17719aebace1b3be7a6ac7f90b1901a6 > > ~Aaron > > > > > On Mon, Jan 13, 2020 at 1:29 PM Aaron Ballman via cfe-commits < > cfe-commits@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@lists.llvm.org > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
