<div dir="ltr">Great, thanks! <br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jan 13, 2020 at 4:04 PM Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jan 13, 2020 at 2:25 PM Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:<br>
><br>
> This breaks tests on Windows: <a href="http://45.33.8.238/win/5636/step_8.txt" rel="noreferrer" target="_blank">http://45.33.8.238/win/5636/step_8.txt</a><br>
><br>
> Likely the usual "doesn't work with delayed template parsing" thing.<br>
><br>
> Can you take a look, and if it takes a while to fix, revert while you investigate?<br>
><br>
> (I wouldn't said this on the phab issue, but couldn't find one.)<br>
<br>
Thank you for pointing this out, it's fixed in<br>
c1b13a1b17719aebace1b3be7a6ac7f90b1901a6<br>
<br>
~Aaron<br>
<br>
><br>
> On Mon, Jan 13, 2020 at 1:29 PM Aaron Ballman via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>><br>
>> Author: Nathan James<br>
>> Date: 2020-01-13T13:28:55-05:00<br>
>> New Revision: fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4<br>
>><br>
>> URL: <a href="https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4</a><br>
>> DIFF: <a href="https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/fb79ef524171c96a9f3df025ac7a8a3e00fdc0b4.diff</a><br>
>><br>
>> LOG: Fix readability-identifier-naming missing member variables<br>
>><br>
>> Fixes PR41122 (missing fixes for member variables in a destructor) and<br>
>> PR29005 (does not rename class members in all locations).<br>
>><br>
>> Added:<br>
>> clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp<br>
>><br>
>> Modified:<br>
>> clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp<br>
>><br>
>> Removed:<br>
>><br>
>><br>
>><br>
>> ################################################################################<br>
>> diff --git a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp<br>
>> index bd736743ae1c..8146cb36cf21 100644<br>
>> --- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp<br>
>> +++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp<br>
>> @@ -237,10 +237,26 @@ void IdentifierNamingCheck::registerMatchers(MatchFinder *Finder) {<br>
>> Finder->addMatcher(namedDecl().bind("decl"), this);<br>
>> Finder->addMatcher(usingDecl().bind("using"), this);<br>
>> Finder->addMatcher(declRefExpr().bind("declRef"), this);<br>
>> - Finder->addMatcher(cxxConstructorDecl().bind("classRef"), this);<br>
>> - Finder->addMatcher(cxxDestructorDecl().bind("classRef"), this);<br>
>> + Finder->addMatcher(cxxConstructorDecl(unless(isImplicit())).bind("classRef"),<br>
>> + this);<br>
>> + Finder->addMatcher(cxxDestructorDecl(unless(isImplicit())).bind("classRef"),<br>
>> + this);<br>
>> Finder->addMatcher(typeLoc().bind("typeLoc"), this);<br>
>> Finder->addMatcher(nestedNameSpecifierLoc().bind("nestedNameLoc"), this);<br>
>> + Finder->addMatcher(<br>
>> + functionDecl(unless(cxxMethodDecl(isImplicit())),<br>
>> + hasBody(forEachDescendant(memberExpr().bind("memberExpr")))),<br>
>> + this);<br>
>> + Finder->addMatcher(<br>
>> + cxxConstructorDecl(<br>
>> + unless(isImplicit()),<br>
>> + forEachConstructorInitializer(<br>
>> + allOf(isWritten(), withInitializer(forEachDescendant(<br>
>> + memberExpr().bind("memberExpr")))))),<br>
>> + this);<br>
>> + Finder->addMatcher(fieldDecl(hasInClassInitializer(<br>
>> + forEachDescendant(memberExpr().bind("memberExpr")))),<br>
>> + this);<br>
>> }<br>
>><br>
>> void IdentifierNamingCheck::registerPPCallbacks(<br>
>> @@ -710,8 +726,6 @@ static void addUsage(IdentifierNamingCheck::NamingCheckFailureMap &Failures,<br>
>> void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {<br>
>> if (const auto *Decl =<br>
>> Result.Nodes.getNodeAs<CXXConstructorDecl>("classRef")) {<br>
>> - if (Decl->isImplicit())<br>
>> - return;<br>
>><br>
>> addUsage(NamingCheckFailures, Decl->getParent(),<br>
>> Decl->getNameInfo().getSourceRange());<br>
>> @@ -730,8 +744,6 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {<br>
>><br>
>> if (const auto *Decl =<br>
>> Result.Nodes.getNodeAs<CXXDestructorDecl>("classRef")) {<br>
>> - if (Decl->isImplicit())<br>
>> - return;<br>
>><br>
>> SourceRange Range = Decl->getNameInfo().getSourceRange();<br>
>> if (Range.getBegin().isInvalid())<br>
>> @@ -806,6 +818,14 @@ void IdentifierNamingCheck::check(const MatchFinder::MatchResult &Result) {<br>
>> return;<br>
>> }<br>
>><br>
>> + if (const auto *MemberRef =<br>
>> + Result.Nodes.getNodeAs<MemberExpr>("memberExpr")) {<br>
>> + SourceRange Range = MemberRef->getMemberNameInfo().getSourceRange();<br>
>> + addUsage(NamingCheckFailures, MemberRef->getMemberDecl(), Range,<br>
>> + Result.SourceManager);<br>
>> + return;<br>
>> + }<br>
>> +<br>
>> if (const auto *Decl = Result.Nodes.getNodeAs<NamedDecl>("decl")) {<br>
>> if (!Decl->getIdentifier() || Decl->getName().empty() || Decl->isImplicit())<br>
>> return;<br>
>><br>
>> 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<br>
>> new file mode 100644<br>
>> index 000000000000..caffc3283ca2<br>
>> --- /dev/null<br>
>> +++ b/clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp<br>
>> @@ -0,0 +1,137 @@<br>
>> +// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \<br>
>> +// RUN: -config='{CheckOptions: [ \<br>
>> +// RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \<br>
>> +// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase} \<br>
>> +// RUN: ]}'<br>
>> +<br>
>> +int set_up(int);<br>
>> +int clear(int);<br>
>> +<br>
>> +class Foo {<br>
>> +public:<br>
>> + const int bar_baz; // comment-0<br>
>> + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for member 'bar_baz'<br>
>> + // CHECK-FIXES: {{^}} const int BarBaz; // comment-0<br>
>> +<br>
>> + Foo(int Val) : bar_baz(Val) { // comment-1<br>
>> + // CHECK-FIXES: {{^}} Foo(int Val) : BarBaz(Val) { // comment-1<br>
>> + set_up(bar_baz); // comment-2<br>
>> + // CHECK-FIXES: {{^}} set_up(BarBaz); // comment-2<br>
>> + }<br>
>> +<br>
>> + Foo() : Foo(0) {}<br>
>> +<br>
>> + ~Foo() {<br>
>> + clear(bar_baz); // comment-3<br>
>> + // CHECK-FIXES: {{^}} clear(BarBaz); // comment-3<br>
>> + }<br>
>> +<br>
>> + int getBar() const { return bar_baz; } // comment-4<br>
>> + // CHECK-FIXES: {{^}} int getBar() const { return BarBaz; } // comment-4<br>
>> +};<br>
>> +<br>
>> +class FooBar : public Foo {<br>
>> +public:<br>
>> + int getFancyBar() const {<br>
>> + return this->bar_baz; // comment-5<br>
>> + // CHECK-FIXES: {{^}} return this->BarBaz; // comment-5<br>
>> + }<br>
>> +};<br>
>> +<br>
>> +int getBar(const Foo &Foo) {<br>
>> + return Foo.bar_baz; // comment-6<br>
>> + // CHECK-FIXES: {{^}} return Foo.BarBaz; // comment-6<br>
>> +}<br>
>> +<br>
>> +int getBar(const FooBar &Foobar) {<br>
>> + return Foobar.bar_baz; // comment-7<br>
>> + // CHECK-FIXES: {{^}} return Foobar.BarBaz; // comment-7<br>
>> +}<br>
>> +<br>
>> +int getFancyBar(const FooBar &Foobar) {<br>
>> + return Foobar.getFancyBar();<br>
>> +}<br>
>> +<br>
>> +template <typename Dummy><br>
>> +class TempTest : public Foo {<br>
>> +public:<br>
>> + TempTest() = default;<br>
>> + TempTest(int Val) : Foo(Val) {}<br>
>> + int getBar() const { return Foo::bar_baz; } // comment-8<br>
>> + // CHECK-FIXES: {{^}} int getBar() const { return Foo::BarBaz; } // comment-8<br>
>> + int getBar2() const { return this->bar_baz; } // comment-9<br>
>> + // CHECK-FIXES: {{^}} int getBar2() const { return this->BarBaz; } // comment-9<br>
>> +};<br>
>> +<br>
>> +TempTest<int> x; //force an instantiation<br>
>> +<br>
>> +int blah() {<br>
>> + return x.getBar2(); // gotta have a reference to the getBar2 so that the<br>
>> + // compiler will generate the function and resolve<br>
>> + // the DependantScopeMemberExpr<br>
>> +}<br>
>> +<br>
>> +namespace Bug41122 {<br>
>> +namespace std {<br>
>> +<br>
>> +// for this example we aren't bothered about how std::vector is treated<br>
>> +template <typename T> //NOLINT<br>
>> +class vector { //NOLINT<br>
>> +public:<br>
>> + void push_back(bool); //NOLINT<br>
>> + void pop_back(); //NOLINT<br>
>> +}; //NOLINT<br>
>> +}; // namespace std<br>
>> +<br>
>> +class Foo {<br>
>> + std::vector<bool> &stack;<br>
>> + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for member 'stack' [readability-identifier-naming]<br>
>> +public:<br>
>> + Foo(std::vector<bool> &stack)<br>
>> + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming]<br>
>> + // CHECK-FIXES: {{^}} Foo(std::vector<bool> &Stack)<br>
>> + : stack(stack) {<br>
>> + // CHECK-FIXES: {{^}} : Stack(Stack) {<br>
>> + stack.push_back(true);<br>
>> + // CHECK-FIXES: {{^}} Stack.push_back(true);<br>
>> + }<br>
>> + ~Foo() {<br>
>> + stack.pop_back();<br>
>> + // CHECK-FIXES: {{^}} Stack.pop_back();<br>
>> + }<br>
>> +};<br>
>> +}; // namespace Bug41122<br>
>> +<br>
>> +namespace Bug29005 {<br>
>> +class Foo {<br>
>> +public:<br>
>> + int a_member_of_foo = 0;<br>
>> + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'a_member_of_foo'<br>
>> + // CHECK-FIXES: {{^}} int AMemberOfFoo = 0;<br>
>> +};<br>
>> +<br>
>> +int main() {<br>
>> + Foo foo;<br>
>> + return foo.a_member_of_foo;<br>
>> + // CHECK-FIXES: {{^}} return foo.AMemberOfFoo;<br>
>> +}<br>
>> +}; // namespace Bug29005<br>
>> +<br>
>> +namespace CtorInits {<br>
>> +template <typename T, unsigned N><br>
>> +class Container {<br>
>> + T storage[N];<br>
>> + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for member 'storage'<br>
>> + // CHECK-FIXES: {{^}} T Storage[N];<br>
>> + T *pointer = &storage[0];<br>
>> + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for member 'pointer'<br>
>> + // CHECK-FIXES: {{^}} T *Pointer = &Storage[0];<br>
>> +public:<br>
>> + Container() : pointer(&storage[0]) {}<br>
>> + // CHECK-FIXES: {{^}} Container() : Pointer(&Storage[0]) {}<br>
>> +};<br>
>> +<br>
>> +void foo() {<br>
>> + Container<int, 5> container;<br>
>> +}<br>
>> +}; // namespace CtorInits<br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
>> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>