This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Fix readability-identifier-naming missing member variables
ClosedPublic

Authored by njames93 on Jan 2 2020, 6:03 PM.

Details

Reviewers
aaron.ballman
Summary

Diff Detail

Event Timeline

njames93 created this revision.Jan 2 2020, 6:03 PM
njames93 updated this revision to Diff 236012.Jan 3 2020, 3:19 AM

More edge cases handled to do with field decls and initializers

Thank you for fixing these bugs!
Did you try running the check of clang/llvm again, to see if there are still missing cases of this?

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
571

these all seem to be an artifact of running clang-format?
Such a change can be done, but should happen in a separate patch (just formatting does not require review).

736

this change violates the coding guideline.

821

const auto * to show that this variable is a pointer.

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
89

I dont understand this comment. What do you mean?

Ahh clang format script must have run, I'll undo that. I've not tried running it against the whole of llvm as I don't know what the easy way to do that is, especially given that llvm disregards it's standard naming convention for old projects and stl like containers. The comment is just in relation to the bug report that this fix is based on, could probably go

Ahh clang format script must have run, I'll undo that. I've not tried running it against the whole of llvm as I don't know what the easy way to do that is, especially given that llvm disregards it's standard naming convention for old projects and stl like containers. The comment is just in relation to the bug report that this fix is based on, could probably go

yeah clang-format tends to do that, as devs do not consistently apply automatic formatting :/

for llvm: https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
You can use run-clang-tidy.py in the tools/ directory of clang-tidy. Then specify your custom built clang-tidy binary.

The call should like similar to this (not tested):
<path-to-llvm>/llvm-project $ run-clang-tidy.py -p <path-to-build-dir-with-compile-commands> -clang-tidy-binary <path-to-build> -checks=-*,readability-identifier-naming -fix clang/lib

That will run over clang/libs only. If you choose lib/ it will run over each lib-subdirectory of the project.

Maybe you can still try it out and see what diagnostics you get. It will be a good practice anyway, if you plan to keep developing :)

njames93 updated this revision to Diff 236157.Jan 3 2020, 5:24 PM

addressed issues. Running the clang-tidy on the whole of lib clang just resulted in errors when compiling due to its dependancy on llvm, but I kind of knew that was going to happen

njames93 marked 4 inline comments as done.Jan 3 2020, 5:44 PM
aaron.ballman added inline comments.Jan 6 2020, 11:34 AM
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
137

I think this is one more test case to add: base class specifiers. e.g.,

struct foo_bar {}; // Should be renamed

struct Baz : foo_bar {}; // Should rename foo_bar
njames93 marked 2 inline comments as done.Jan 6 2020, 5:38 PM
njames93 added inline comments.
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
137

That test case already exists in the readability-identifier-naming.cpp file

class my_derived_class : public virtual my_class {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'my_derived_class'
// CHECK-FIXES: {{^}}class CMyDerivedClass : public virtual CMyClass {};{{$}}
aaron.ballman accepted this revision.Jan 8 2020, 11:45 AM

LGTM!

clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
137

Oh, thank you for pointing that out!

This revision is now accepted and ready to land.Jan 8 2020, 11:45 AM
njames93 marked an inline comment as done.Jan 13 2020, 7:37 AM

Is anyone able to commit on my behalf?

Is anyone able to commit on my behalf?

Happy to do so. I've committed for you in fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4

Please also close PRs.

Please also close PRs.

Done