Re: [clang-tools-extra] r310559 - [clang-tidy] Minor documentation improvement

2017-08-10 Thread Alexander Kornienko via cfe-commits 
There's also `C::x; // 2` below, so I'd add this comment back at least for
consistency.

On Thu, Aug 10, 2017 at 5:53 PM, Gábor Horváth  wrote:

>
>
> On 10 August 2017 at 17:40, Alexander Kornienko  wrote:
>
>>
>>
>> On Thu, Aug 10, 2017 at 11:13 AM, Gabor Horvath via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: xazax
>>> Date: Thu Aug 10 02:13:26 2017
>>> New Revision: 310559
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=310559=rev
>>> Log:
>>> [clang-tidy] Minor documentation improvement
>>>
>>> Patch by: Lilla Barancsuk
>>>
>>> Modified:
>>> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-s
>>> tatic-accessed-through-instance.rst
>>> clang-tools-extra/trunk/test/clang-tidy/readability-static-a
>>> ccessed-through-instance.cpp
>>>
>>> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-s
>>> tatic-accessed-through-instance.rst
>>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>>> docs/clang-tidy/checks/readability-static-accessed-through-i
>>> nstance.rst?rev=310559=310558=310559=diff
>>> 
>>> ==
>>> --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-s
>>> tatic-accessed-through-instance.rst (original)
>>> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-s
>>> tatic-accessed-through-instance.rst Thu Aug 10 02:13:26 2017
>>> @@ -25,6 +25,7 @@ is changed to:
>>>
>>>  .. code-block:: c++
>>>
>>> +  C *c1 = new C();
>>>C::foo();
>>>C::x;
>>>
>>>
>>> Modified: clang-tools-extra/trunk/test/clang-tidy/readability-static-a
>>> ccessed-through-instance.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>>> test/clang-tidy/readability-static-accessed-through-instance
>>> .cpp?rev=310559=310558=310559=diff
>>> 
>>> ==
>>> --- 
>>> clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
>>> (original)
>>> +++ 
>>> clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
>>> Thu Aug 10 02:13:26 2017
>>> @@ -116,7 +116,7 @@ using E = D;
>>>
>>>  template  void f(T t, C c) {
>>>t.x; // OK, t is a template parameter.
>>> -  c.x; // 1
>>> +  c.x;
>>>
>>
>> This comment was there to make the CHECK-FIXES line unique so that it
>> doesn't match wrong line in the test. Any specific reason to remove it?
>>
>
> There was no specific reason, this seemed to be redundant, and reducing
> readability. But I just noticed there is a C::x; line far above, so it
> looks like this was not redundant at all. Should I add this back or you are
> fine with leaving it as is for now?
>
>
>>
>>
>>>// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
>>>// CHECK-FIXES: {{^}}  C::x; // 1{{$}}
>>>  }
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r310559 - [clang-tidy] Minor documentation improvement

2017-08-10 Thread Gábor Horváth via cfe-commits 
On 10 August 2017 at 17:40, Alexander Kornienko  wrote:

>
>
> On Thu, Aug 10, 2017 at 11:13 AM, Gabor Horvath via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: xazax
>> Date: Thu Aug 10 02:13:26 2017
>> New Revision: 310559
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=310559=rev
>> Log:
>> [clang-tidy] Minor documentation improvement
>>
>> Patch by: Lilla Barancsuk
>>
>> Modified:
>> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-s
>> tatic-accessed-through-instance.rst
>> clang-tools-extra/trunk/test/clang-tidy/readability-static-a
>> ccessed-through-instance.cpp
>>
>> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-s
>> tatic-accessed-through-instance.rst
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> docs/clang-tidy/checks/readability-static-accessed-through-
>> instance.rst?rev=310559=310558=310559=diff
>> 
>> ==
>> --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-s
>> tatic-accessed-through-instance.rst (original)
>> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-s
>> tatic-accessed-through-instance.rst Thu Aug 10 02:13:26 2017
>> @@ -25,6 +25,7 @@ is changed to:
>>
>>  .. code-block:: c++
>>
>> +  C *c1 = new C();
>>C::foo();
>>C::x;
>>
>>
>> Modified: clang-tools-extra/trunk/test/clang-tidy/readability-static-a
>> ccessed-through-instance.cpp
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/
>> test/clang-tidy/readability-static-accessed-through-
>> instance.cpp?rev=310559=310558=310559=diff
>> 
>> ==
>> --- 
>> clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
>> (original)
>> +++ 
>> clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
>> Thu Aug 10 02:13:26 2017
>> @@ -116,7 +116,7 @@ using E = D;
>>
>>  template  void f(T t, C c) {
>>t.x; // OK, t is a template parameter.
>> -  c.x; // 1
>> +  c.x;
>>
>
> This comment was there to make the CHECK-FIXES line unique so that it
> doesn't match wrong line in the test. Any specific reason to remove it?
>

There was no specific reason, this seemed to be redundant, and reducing
readability. But I just noticed there is a C::x; line far above, so it
looks like this was not redundant at all. Should I add this back or you are
fine with leaving it as is for now?


>
>
>>// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
>>// CHECK-FIXES: {{^}}  C::x; // 1{{$}}
>>  }
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r310559 - [clang-tidy] Minor documentation improvement

2017-08-10 Thread Alexander Kornienko via cfe-commits 
On Thu, Aug 10, 2017 at 11:13 AM, Gabor Horvath via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: xazax
> Date: Thu Aug 10 02:13:26 2017
> New Revision: 310559
>
> URL: http://llvm.org/viewvc/llvm-project?rev=310559=rev
> Log:
> [clang-tidy] Minor documentation improvement
>
> Patch by: Lilla Barancsuk
>
> Modified:
> clang-tools-extra/trunk/docs/clang-tidy/checks/readability-
> static-accessed-through-instance.rst
> clang-tools-extra/trunk/test/clang-tidy/readability-static-
> accessed-through-instance.cpp
>
> Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/readability-
> static-accessed-through-instance.rst
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/docs/clang-tidy/checks/readability-static-accessed-
> through-instance.rst?rev=310559=310558=310559=diff
> 
> ==
> --- clang-tools-extra/trunk/docs/clang-tidy/checks/readability-
> static-accessed-through-instance.rst (original)
> +++ clang-tools-extra/trunk/docs/clang-tidy/checks/readability-
> static-accessed-through-instance.rst Thu Aug 10 02:13:26 2017
> @@ -25,6 +25,7 @@ is changed to:
>
>  .. code-block:: c++
>
> +  C *c1 = new C();
>C::foo();
>C::x;
>
>
> Modified: clang-tools-extra/trunk/test/clang-tidy/readability-static-
> accessed-through-instance.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
> trunk/test/clang-tidy/readability-static-accessed-
> through-instance.cpp?rev=310559=310558=310559=diff
> 
> ==
> --- 
> clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
> (original)
> +++ 
> clang-tools-extra/trunk/test/clang-tidy/readability-static-accessed-through-instance.cpp
> Thu Aug 10 02:13:26 2017
> @@ -116,7 +116,7 @@ using E = D;
>
>  template  void f(T t, C c) {
>t.x; // OK, t is a template parameter.
> -  c.x; // 1
> +  c.x;
>

This comment was there to make the CHECK-FIXES line unique so that it
doesn't match wrong line in the test. Any specific reason to remove it?


>// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
>// CHECK-FIXES: {{^}}  C::x; // 1{{$}}
>  }
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits