LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 41122 - [clang-tidy] readability-identifier-naming misses fixing member variables in destructor
Summary: [clang-tidy] readability-identifier-naming misses fixing member variables in ...
Status: RESOLVED FIXED
Alias: None
Product: clang-tools-extra
Classification: Unclassified
Component: clang-tidy (show other bugs)
Version: unspecified
Hardware: All All
: P enhancement
Assignee: Nathan James
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-18 02:59 PDT by MyDeveloperDay
Modified: 2020-05-11 14:11 PDT (History)
5 users (show)

See Also:
Fixed By Commit(s): rGfb79ef524171c96a9f3df025ac7a8a3e00fdc0b4


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description MyDeveloperDay 2019-03-18 02:59:44 PDT
Whilst looking at how clang-tidy might be able to handle such large scale variable renaming for something like https://reviews.llvm.org/D59251 I decided to run clang-tidy over the clang code base (starting in lib/Format)

The following code shows an example which will fail to correctly be fixed

-----------------------------------------------------
#include <vector>

class Foo {
public:
  Foo(std::vector<bool> &Stack)
      : Stack(Stack) {
    Stack.push_back(true);
  }
  ~Foo() {
    Stack.pop_back();
  }

  std::vector<bool> &Stack;
};
-----------------------------------------------------

will be converted to (notice the Stack.pop_back() in the destructor)

-----------------------------------------------------
class Foo {
public:
  Foo(std::vector<bool> &stack)
      : stack(stack) {
    stack.push_back(true);
  }
  ~Foo() {
    Stack.pop_back();
  }

  std::vector<bool> &stack;
};

-----------------------------------------------------

Conversion was done with the following .clang-tidy file

-----------------------------------------------------
Checks: '-*,readability-identifier-naming'
CheckOptions:
  - key:             readability-identifier-naming.MemberCase
    value:           camelBack
  - key:             readability-identifier-naming.PrivateMemberCase
    value:           camelBack
  - key:             readability-identifier-naming.ParameterCase
    value:           camelBack
  - key:             readability-identifier-naming.VariableCase
    value:           camelBack
  - key:             readability-identifier-naming.PointerParameterCase
    value:           camelBack
-----------------------------------------------------


c:\Repos\buildareas\tidy_tests\test3.cxx:5:26: warning: invalid case style for parameter 'Stack' [readability-identifier-naming]
  Foo(std::vector<bool> &Stack)
                         ^~~~~
                         stack
c:\Repos\buildareas\tidy_tests\test3.cxx:5:26: note: FIX-IT applied suggested code changes
c:\Repos\buildareas\tidy_tests\test3.cxx:6:15: note: FIX-IT applied suggested code changes
      : Stack(Stack) {
              ^
c:\Repos\buildareas\tidy_tests\test3.cxx:7:5: note: FIX-IT applied suggested code changes
    Stack.push_back(true);
    ^
c:\Repos\buildareas\tidy_tests\test3.cxx:13:22: warning: invalid case style for member 'Stack' [readability-identifier-naming]
  std::vector<bool> &Stack;
                     ^~~~~
                     stack
c:\Repos\buildareas\tidy_tests\test3.cxx:6:9: note: FIX-IT applied suggested code changes
      : Stack(Stack) {
        ^
c:\Repos\buildareas\tidy_tests\test3.cxx:13:22: note: FIX-IT applied suggested code changes
  std::vector<bool> &Stack;
                     ^
clang-tidy applied 5 of 5 suggested fixes.
Comment 1 Andrew Somerville 2019-04-18 21:45:18 PDT
This is actually a bigger issue than the title implies. The fact that it gets changed in the constructor is actually because of readability-identifier-naming.ParameterCase rather than readability-identifier-naming.MemberCase

For some reason clang-tidy seems to mis any object accesses to the members it renames. Static members for some reason are correctly renamed.

Similar issue reported here as well:

https://stackoverflow.com/questions/50906481/clang-tidy-readability-identifier-naming-module-does-not-seem-to-properly-handle/55756706#55756706
Comment 2 Nathan James 2020-01-14 15:21:21 PST
fixed in rGfb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
Comment 3 Andrew Somerville 2020-05-11 14:11:27 PDT
I got confused by the rG got prepended to that hash and realized it's part of the URL from phabricator. Here are the URLs if anyone else is interested:

Hash: fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4

https://reviews.llvm.org/rGfb79ef524171c96a9f3df025ac7a8a3e00fdc0b4
https://reviews.llvm.org/D72121