[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 13:08:54 PST 2020


Great, thanks!

On Mon, Jan 13, 2020 at 4:04 PM Aaron Ballman <aaron at aaronballman.com>
wrote:

> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200113/581c59e8/attachment-0001.html>


More information about the cfe-commits mailing list