[clang-tools-extra] fb79ef5 - Fix readability-identifier-naming missing member variables
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 13 13:03:46 PST 2020
On Mon, Jan 13, 2020 at 2:25 PM Nico Weber <thakis at 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 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
More information about the cfe-commits
mailing list