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