subject:"Re\: \[clang\-tools\-extra\] r330559 \- update test to use ivar in implementation instead of class extension"

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Yan Zhang via cfe-commits

Did not see any failure related to https://reviews.llvm.org/D45936 yet after 
submission.

Best regards
Yan Zhang

> On Apr 22, 2018, at 18:13, Chandler Carruth  wrote:
> 
> I won't be back at a computer for a while and I really don't know anything 
> about objective-c... But if you don't feel confident submitting fixes with 
> post commit review, you should probably just revert If the fix is trivial 
> and/or you can find ways to test it a similar suggested in my earlier email, 
> then I'd suggest landing it and watching the build bots to ensure they are 
> happy. But if you can't figure out how to restore the bots you'll end up 
> needing to revert the whole series and get some help from someone with access 
> to another platform.
> 
>> On Sun, Apr 22, 2018, 18:03 Yan Zhang via cfe-commits 
>>  wrote:
>> Need a accept for that revision. Can you accept it?
>> 
>>> On Sun, Apr 22, 2018 at 6:02 PM Chandler Carruth  
>>> wrote:
>>> See my other email -- you can compile code targeting other platforms 
>>> regardless of the platform you develop on. Not exactly as good as 
>>> reproducing it with a bot, but about the best you have.
>>> 
 On Sun, Apr 22, 2018 at 6:01 PM Chandler Carruth  
 wrote:
 I don't know anything about objective-c, or anything about OSX.
 
 However, I guarantee these bots aren't using 32-bit OSX. ;] Look at the 
 bot names. They're running Linux in various flavors: ppc64be, ppc64le, 
 armv8, etc.
 
 My suspicion is that this is a linux-specific issue.
 
 But you can reproduce this yourself. Just run clang (or clang-tidy) over 
 this test file with --target= for various linux triples. It 
 doesn't include any headers or anything else, so you should be able to see 
 all these failures.
 
 Anyways, please land that revision or revert until you have a reviewer for 
 it (many maybe not around this weekend). But please don't leave the bots 
 broken.
 
> On Sun, Apr 22, 2018 at 5:55 PM Yan Zhang  wrote:
 
> I am running tests locally with "ninja check-clang-tools" and I am sure 
> it is running this test because I could get error message when I debug it.
> 
> The problem (according to the error message) is all caused by different 
> architecture. It seems a lot of ObjC features are not supported in old 
> 32-bit OSX (which I believe the test bots are using). I have another 
> revision sent out to see if it can help. Can you take a quick look? 
> https://reviews.llvm.org/D45936
> 
>> On Sun, Apr 22, 2018 at 5:51 PM Chandler Carruth  
>> wrote:
>> The commit log here no longer reflects the commit. This is not just 
>> updating the test, this is a complete re-application of the original 
>> patch in r330492. =[
>> 
>> Also, the bots are still complaining:
>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
>> http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
>> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659
>> 
>> I'm not sure how you're running your tests that you don't see these 
>> issues, but they seem to reproduce on many build bots and the error 
>> message doesn't seem to be architecture specific at all...
>> 
>> I suspect something about how you are trying to run tests isn't actually 
>> running this test if you aren't able to locally reproduce.
>> 
>>> On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits 
>>>  wrote:
>>> Author: wizard
>>> Date: Sun Apr 22 17:15:15 2018
>>> New Revision: 330559
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
>>> Log:
>>> update test to use ivar in implementation instead of class extension
>>> 
>>> Summary: using ivar in class extension is not supported in 32-bit 
>>> architecture of MacOS.
>>> 
>>> Reviewers: alexfh, hokein
>>> 
>>> Reviewed By: alexfh
>>> 
>>> Subscribers: klimek, cfe-commits
>>> 
>>> Differential Revision: https://reviews.llvm.org/D45912
>>> 
>>> Added:
>>> 
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> Modified:
>>> 
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> 
>>> Modified: 
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> URL: 
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff
>>> ==
>>> --- 
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>>

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Yan Zhang via cfe-commits

Sure. Will do. The change I am trying is pretty trivial and only limited to
the test itself.

On Sun, Apr 22, 2018 at 6:13 PM Chandler Carruth 
wrote:

> I won't be back at a computer for a while and I really don't know anything
> about objective-c... But if you don't feel confident submitting fixes with
> post commit review, you should probably just revert If the fix is
> trivial and/or you can find ways to test it a similar suggested in my
> earlier email, then I'd suggest landing it and watching the build bots to
> ensure they are happy. But if you can't figure out how to restore the bots
> you'll end up needing to revert the whole series and get some help from
> someone with access to another platform.
>
> On Sun, Apr 22, 2018, 18:03 Yan Zhang via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Need a accept for that revision. Can you accept it?
>>
>> On Sun, Apr 22, 2018 at 6:02 PM Chandler Carruth 
>> wrote:
>>
>>> See my other email -- you can compile code targeting other platforms
>>> regardless of the platform you develop on. Not exactly as good as
>>> reproducing it with a bot, but about the best you have.
>>>
>>> On Sun, Apr 22, 2018 at 6:01 PM Chandler Carruth 
>>> wrote:
>>>
 I don't know anything about objective-c, or anything about OSX.

 However, I guarantee these bots aren't using 32-bit OSX. ;] Look at the
 bot names. They're running Linux in various flavors: ppc64be, ppc64le,
 armv8, etc.

 My suspicion is that this is a linux-specific issue.

 But you can reproduce this yourself. Just run clang (or clang-tidy)
 over this test file with --target= for various linux triples. It
 doesn't include any headers or anything else, so you should be able to see
 all these failures.

 Anyways, please land that revision or revert until you have a reviewer
 for it (many maybe not around this weekend). But please don't leave the
 bots broken.

 On Sun, Apr 22, 2018 at 5:55 PM Yan Zhang  wrote:

> I am running tests locally with "ninja check-clang-tools" and I am
> sure it is running this test because I could get error message when I 
> debug
> it.
>
> The problem (according to the error message) is all caused by
> different architecture. It seems a lot of ObjC features are not supported
> in old 32-bit OSX (which I believe the test bots are using). I have 
> another
> revision sent out to see if it can help. Can you take a quick look?
> https://reviews.llvm.org/D45936
>
> On Sun, Apr 22, 2018 at 5:51 PM Chandler Carruth 
> wrote:
>
>> The commit log here no longer reflects the commit. This is not just
>> updating the test, this is a complete re-application of the original 
>> patch
>> in r330492. =[
>>
>> Also, the bots are still complaining:
>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
>> http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
>> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659
>>
>> I'm not sure how you're running your tests that you don't see these
>> issues, but they seem to reproduce on many build bots and the error 
>> message
>> doesn't seem to be architecture specific at all...
>>
>> I suspect something about how you are trying to run tests isn't
>> actually running this test if you aren't able to locally reproduce.
>>
>> On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: wizard
>>> Date: Sun Apr 22 17:15:15 2018
>>> New Revision: 330559
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
>>> Log:
>>> update test to use ivar in implementation instead of class extension
>>>
>>> Summary: using ivar in class extension is not supported in 32-bit
>>> architecture of MacOS.
>>>
>>> Reviewers: alexfh, hokein
>>>
>>> Reviewed By: alexfh
>>>
>>> Subscribers: klimek, cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D45912
>>>
>>> Added:
>>>
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> Modified:
>>>
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff
>>>
>>> ==
>>> ---
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> (original)
>>> +++
>>>

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Chandler Carruth via cfe-commits

I won't be back at a computer for a while and I really don't know anything
about objective-c... But if you don't feel confident submitting fixes with
post commit review, you should probably just revert If the fix is
trivial and/or you can find ways to test it a similar suggested in my
earlier email, then I'd suggest landing it and watching the build bots to
ensure they are happy. But if you can't figure out how to restore the bots
you'll end up needing to revert the whole series and get some help from
someone with access to another platform.

On Sun, Apr 22, 2018, 18:03 Yan Zhang via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Need a accept for that revision. Can you accept it?
>
> On Sun, Apr 22, 2018 at 6:02 PM Chandler Carruth 
> wrote:
>
>> See my other email -- you can compile code targeting other platforms
>> regardless of the platform you develop on. Not exactly as good as
>> reproducing it with a bot, but about the best you have.
>>
>> On Sun, Apr 22, 2018 at 6:01 PM Chandler Carruth 
>> wrote:
>>
>>> I don't know anything about objective-c, or anything about OSX.
>>>
>>> However, I guarantee these bots aren't using 32-bit OSX. ;] Look at the
>>> bot names. They're running Linux in various flavors: ppc64be, ppc64le,
>>> armv8, etc.
>>>
>>> My suspicion is that this is a linux-specific issue.
>>>
>>> But you can reproduce this yourself. Just run clang (or clang-tidy) over
>>> this test file with --target= for various linux triples. It doesn't
>>> include any headers or anything else, so you should be able to see all
>>> these failures.
>>>
>>> Anyways, please land that revision or revert until you have a reviewer
>>> for it (many maybe not around this weekend). But please don't leave the
>>> bots broken.
>>>
>>> On Sun, Apr 22, 2018 at 5:55 PM Yan Zhang  wrote:
>>>
 I am running tests locally with "ninja check-clang-tools" and I am
 sure it is running this test because I could get error message when I debug
 it.

 The problem (according to the error message) is all caused by different
 architecture. It seems a lot of ObjC features are not supported in old
 32-bit OSX (which I believe the test bots are using). I have another
 revision sent out to see if it can help. Can you take a quick look?
 https://reviews.llvm.org/D45936

 On Sun, Apr 22, 2018 at 5:51 PM Chandler Carruth 
 wrote:

> The commit log here no longer reflects the commit. This is not just
> updating the test, this is a complete re-application of the original patch
> in r330492. =[
>
> Also, the bots are still complaining:
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
> http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659
>
> I'm not sure how you're running your tests that you don't see these
> issues, but they seem to reproduce on many build bots and the error 
> message
> doesn't seem to be architecture specific at all...
>
> I suspect something about how you are trying to run tests isn't
> actually running this test if you aren't able to locally reproduce.
>
> On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: wizard
>> Date: Sun Apr 22 17:15:15 2018
>> New Revision: 330559
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
>> Log:
>> update test to use ivar in implementation instead of class extension
>>
>> Summary: using ivar in class extension is not supported in 32-bit
>> architecture of MacOS.
>>
>> Reviewers: alexfh, hokein
>>
>> Reviewed By: alexfh
>>
>> Subscribers: klimek, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D45912
>>
>> Added:
>>
>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>> Modified:
>>
>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff
>>
>> ==
>> ---
>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>> (original)
>> +++
>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>> Sun Apr 22 17:15:15 2018
>> @@ -109,6 +109,7 @@ namespace readability {
>>  m(TemplateParameter) \
>>  m(TypeAlias) \
>>  m(MacroDefinition) \
>> +m(ObjcIvar) \
>>
>>  enum StyleKind {
>>  #define

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Yan Zhang via cfe-commits

Need a accept for that revision. Can you accept it?

On Sun, Apr 22, 2018 at 6:02 PM Chandler Carruth 
wrote:

> See my other email -- you can compile code targeting other platforms
> regardless of the platform you develop on. Not exactly as good as
> reproducing it with a bot, but about the best you have.
>
> On Sun, Apr 22, 2018 at 6:01 PM Chandler Carruth 
> wrote:
>
>> I don't know anything about objective-c, or anything about OSX.
>>
>> However, I guarantee these bots aren't using 32-bit OSX. ;] Look at the
>> bot names. They're running Linux in various flavors: ppc64be, ppc64le,
>> armv8, etc.
>>
>> My suspicion is that this is a linux-specific issue.
>>
>> But you can reproduce this yourself. Just run clang (or clang-tidy) over
>> this test file with --target= for various linux triples. It doesn't
>> include any headers or anything else, so you should be able to see all
>> these failures.
>>
>> Anyways, please land that revision or revert until you have a reviewer
>> for it (many maybe not around this weekend). But please don't leave the
>> bots broken.
>>
>> On Sun, Apr 22, 2018 at 5:55 PM Yan Zhang  wrote:
>>
>>> I am running tests locally with "ninja check-clang-tools" and I am sure
>>> it is running this test because I could get error message when I debug it.
>>>
>>> The problem (according to the error message) is all caused by different
>>> architecture. It seems a lot of ObjC features are not supported in old
>>> 32-bit OSX (which I believe the test bots are using). I have another
>>> revision sent out to see if it can help. Can you take a quick look?
>>> https://reviews.llvm.org/D45936
>>>
>>> On Sun, Apr 22, 2018 at 5:51 PM Chandler Carruth 
>>> wrote:
>>>
 The commit log here no longer reflects the commit. This is not just
 updating the test, this is a complete re-application of the original patch
 in r330492. =[

 Also, the bots are still complaining:
 http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
 http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
 http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659

 I'm not sure how you're running your tests that you don't see these
 issues, but they seem to reproduce on many build bots and the error message
 doesn't seem to be architecture specific at all...

 I suspect something about how you are trying to run tests isn't
 actually running this test if you aren't able to locally reproduce.

 On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Author: wizard
> Date: Sun Apr 22 17:15:15 2018
> New Revision: 330559
>
> URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
> Log:
> update test to use ivar in implementation instead of class extension
>
> Summary: using ivar in class extension is not supported in 32-bit
> architecture of MacOS.
>
> Reviewers: alexfh, hokein
>
> Reviewed By: alexfh
>
> Subscribers: klimek, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D45912
>
> Added:
>
> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
> Modified:
>
> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff
>
> ==
> ---
> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
> (original)
> +++
> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
> Sun Apr 22 17:15:15 2018
> @@ -109,6 +109,7 @@ namespace readability {
>  m(TemplateParameter) \
>  m(TypeAlias) \
>  m(MacroDefinition) \
> +m(ObjcIvar) \
>
>  enum StyleKind {
>  #define ENUMERATE(v) SK_ ## v,
> @@ -384,6 +385,9 @@ static StyleKind findStyleKind(
>  const NamedDecl *D,
>  const
> std::vector
>  ) {
> +  if (isa(D) && NamingStyles[SK_ObjcIvar])
> +return SK_ObjcIvar;
> +
>if (isa(D) && NamingStyles[SK_Typedef])
>  return SK_Typedef;
>
>
> Added:
> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m?rev=330559=auto
>
> ==
> ---
>

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Chandler Carruth via cfe-commits

See my other email -- you can compile code targeting other platforms
regardless of the platform you develop on. Not exactly as good as
reproducing it with a bot, but about the best you have.

On Sun, Apr 22, 2018 at 6:01 PM Chandler Carruth 
wrote:

> I don't know anything about objective-c, or anything about OSX.
>
> However, I guarantee these bots aren't using 32-bit OSX. ;] Look at the
> bot names. They're running Linux in various flavors: ppc64be, ppc64le,
> armv8, etc.
>
> My suspicion is that this is a linux-specific issue.
>
> But you can reproduce this yourself. Just run clang (or clang-tidy) over
> this test file with --target= for various linux triples. It doesn't
> include any headers or anything else, so you should be able to see all
> these failures.
>
> Anyways, please land that revision or revert until you have a reviewer for
> it (many maybe not around this weekend). But please don't leave the bots
> broken.
>
> On Sun, Apr 22, 2018 at 5:55 PM Yan Zhang  wrote:
>
>> I am running tests locally with "ninja check-clang-tools" and I am sure
>> it is running this test because I could get error message when I debug it.
>>
>> The problem (according to the error message) is all caused by different
>> architecture. It seems a lot of ObjC features are not supported in old
>> 32-bit OSX (which I believe the test bots are using). I have another
>> revision sent out to see if it can help. Can you take a quick look?
>> https://reviews.llvm.org/D45936
>>
>> On Sun, Apr 22, 2018 at 5:51 PM Chandler Carruth 
>> wrote:
>>
>>> The commit log here no longer reflects the commit. This is not just
>>> updating the test, this is a complete re-application of the original patch
>>> in r330492. =[
>>>
>>> Also, the bots are still complaining:
>>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
>>> http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
>>> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659
>>>
>>> I'm not sure how you're running your tests that you don't see these
>>> issues, but they seem to reproduce on many build bots and the error message
>>> doesn't seem to be architecture specific at all...
>>>
>>> I suspect something about how you are trying to run tests isn't actually
>>> running this test if you aren't able to locally reproduce.
>>>
>>> On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: wizard
 Date: Sun Apr 22 17:15:15 2018
 New Revision: 330559

 URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
 Log:
 update test to use ivar in implementation instead of class extension

 Summary: using ivar in class extension is not supported in 32-bit
 architecture of MacOS.

 Reviewers: alexfh, hokein

 Reviewed By: alexfh

 Subscribers: klimek, cfe-commits

 Differential Revision: https://reviews.llvm.org/D45912

 Added:

 clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
 Modified:

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

 Modified:
 clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff

 ==
 ---
 clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
 (original)
 +++
 clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
 Sun Apr 22 17:15:15 2018
 @@ -109,6 +109,7 @@ namespace readability {
  m(TemplateParameter) \
  m(TypeAlias) \
  m(MacroDefinition) \
 +m(ObjcIvar) \

  enum StyleKind {
  #define ENUMERATE(v) SK_ ## v,
 @@ -384,6 +385,9 @@ static StyleKind findStyleKind(
  const NamedDecl *D,
  const
 std::vector
  ) {
 +  if (isa(D) && NamingStyles[SK_ObjcIvar])
 +return SK_ObjcIvar;
 +
if (isa(D) && NamingStyles[SK_Typedef])
  return SK_Typedef;


 Added:
 clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
 URL:
 http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m?rev=330559=auto

 ==
 ---
 clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
 (added)
 +++
 clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
 Sun Apr 22 17:15:15 2018
 @@ -0,0 +1,15 @@
 +// RUN: %check_clang_tidy %s readability-identifier-naming %t \
 +// RUN:

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Chandler Carruth via cfe-commits

I don't know anything about objective-c, or anything about OSX.

However, I guarantee these bots aren't using 32-bit OSX. ;] Look at the bot
names. They're running Linux in various flavors: ppc64be, ppc64le, armv8,
etc.

My suspicion is that this is a linux-specific issue.

But you can reproduce this yourself. Just run clang (or clang-tidy) over
this test file with --target= for various linux triples. It doesn't
include any headers or anything else, so you should be able to see all
these failures.

Anyways, please land that revision or revert until you have a reviewer for
it (many maybe not around this weekend). But please don't leave the bots
broken.

On Sun, Apr 22, 2018 at 5:55 PM Yan Zhang  wrote:

> I am running tests locally with "ninja check-clang-tools" and I am sure
> it is running this test because I could get error message when I debug it.
>
> The problem (according to the error message) is all caused by different
> architecture. It seems a lot of ObjC features are not supported in old
> 32-bit OSX (which I believe the test bots are using). I have another
> revision sent out to see if it can help. Can you take a quick look?
> https://reviews.llvm.org/D45936
>
> On Sun, Apr 22, 2018 at 5:51 PM Chandler Carruth 
> wrote:
>
>> The commit log here no longer reflects the commit. This is not just
>> updating the test, this is a complete re-application of the original patch
>> in r330492. =[
>>
>> Also, the bots are still complaining:
>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
>> http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
>> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659
>>
>> I'm not sure how you're running your tests that you don't see these
>> issues, but they seem to reproduce on many build bots and the error message
>> doesn't seem to be architecture specific at all...
>>
>> I suspect something about how you are trying to run tests isn't actually
>> running this test if you aren't able to locally reproduce.
>>
>> On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: wizard
>>> Date: Sun Apr 22 17:15:15 2018
>>> New Revision: 330559
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
>>> Log:
>>> update test to use ivar in implementation instead of class extension
>>>
>>> Summary: using ivar in class extension is not supported in 32-bit
>>> architecture of MacOS.
>>>
>>> Reviewers: alexfh, hokein
>>>
>>> Reviewed By: alexfh
>>>
>>> Subscribers: klimek, cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D45912
>>>
>>> Added:
>>>
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> Modified:
>>>
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff
>>>
>>> ==
>>> ---
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> (original)
>>> +++
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> Sun Apr 22 17:15:15 2018
>>> @@ -109,6 +109,7 @@ namespace readability {
>>>  m(TemplateParameter) \
>>>  m(TypeAlias) \
>>>  m(MacroDefinition) \
>>> +m(ObjcIvar) \
>>>
>>>  enum StyleKind {
>>>  #define ENUMERATE(v) SK_ ## v,
>>> @@ -384,6 +385,9 @@ static StyleKind findStyleKind(
>>>  const NamedDecl *D,
>>>  const
>>> std::vector
>>>  ) {
>>> +  if (isa(D) && NamingStyles[SK_ObjcIvar])
>>> +return SK_ObjcIvar;
>>> +
>>>if (isa(D) && NamingStyles[SK_Typedef])
>>>  return SK_Typedef;
>>>
>>>
>>> Added:
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m?rev=330559=auto
>>>
>>> ==
>>> ---
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> (added)
>>> +++
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> Sun Apr 22 17:15:15 2018
>>> @@ -0,0 +1,15 @@
>>> +// RUN: %check_clang_tidy %s readability-identifier-naming %t \
>>> +// RUN: -config='{CheckOptions: \
>>> +// RUN:  [{key: readability-identifier-naming.ObjcIvarPrefix, value:
>>> '_'}]}' \
>>> +// RUN: --
>>> +
>>> +@interface Foo
>>> +@end
>>> +
>>> +@implementation Foo {
>>> +int _bar;
>>> +int barWithoutPrefix;
>>> +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for
>>> objc ivar 'barWithoutPrefix' [readability-identifier-naming]
>>> +//

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Yan Zhang via cfe-commits

btw due to the lack of way to testing it on bot environments, I am not sure
if specify fobjc-abo-version=2 could solve the problem (could it break
builds on 32-bit machines or just skip them). So I played safe to use the
old way of declaring ivars in the revision.

On Sun, Apr 22, 2018 at 5:55 PM Yan Zhang  wrote:

> I am running tests locally with "ninja check-clang-tools" and I am sure
> it is running this test because I could get error message when I debug it.
>
> The problem (according to the error message) is all caused by different
> architecture. It seems a lot of ObjC features are not supported in old
> 32-bit OSX (which I believe the test bots are using). I have another
> revision sent out to see if it can help. Can you take a quick look?
> https://reviews.llvm.org/D45936
>
> On Sun, Apr 22, 2018 at 5:51 PM Chandler Carruth 
> wrote:
>
>> The commit log here no longer reflects the commit. This is not just
>> updating the test, this is a complete re-application of the original patch
>> in r330492. =[
>>
>> Also, the bots are still complaining:
>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
>> http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
>> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659
>>
>> I'm not sure how you're running your tests that you don't see these
>> issues, but they seem to reproduce on many build bots and the error message
>> doesn't seem to be architecture specific at all...
>>
>> I suspect something about how you are trying to run tests isn't actually
>> running this test if you aren't able to locally reproduce.
>>
>> On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: wizard
>>> Date: Sun Apr 22 17:15:15 2018
>>> New Revision: 330559
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
>>> Log:
>>> update test to use ivar in implementation instead of class extension
>>>
>>> Summary: using ivar in class extension is not supported in 32-bit
>>> architecture of MacOS.
>>>
>>> Reviewers: alexfh, hokein
>>>
>>> Reviewed By: alexfh
>>>
>>> Subscribers: klimek, cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D45912
>>>
>>> Added:
>>>
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> Modified:
>>>
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff
>>>
>>> ==
>>> ---
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> (original)
>>> +++
>>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>> Sun Apr 22 17:15:15 2018
>>> @@ -109,6 +109,7 @@ namespace readability {
>>>  m(TemplateParameter) \
>>>  m(TypeAlias) \
>>>  m(MacroDefinition) \
>>> +m(ObjcIvar) \
>>>
>>>  enum StyleKind {
>>>  #define ENUMERATE(v) SK_ ## v,
>>> @@ -384,6 +385,9 @@ static StyleKind findStyleKind(
>>>  const NamedDecl *D,
>>>  const
>>> std::vector
>>>  ) {
>>> +  if (isa(D) && NamingStyles[SK_ObjcIvar])
>>> +return SK_ObjcIvar;
>>> +
>>>if (isa(D) && NamingStyles[SK_Typedef])
>>>  return SK_Typedef;
>>>
>>>
>>> Added:
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m?rev=330559=auto
>>>
>>> ==
>>> ---
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> (added)
>>> +++
>>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>>> Sun Apr 22 17:15:15 2018
>>> @@ -0,0 +1,15 @@
>>> +// RUN: %check_clang_tidy %s readability-identifier-naming %t \
>>> +// RUN: -config='{CheckOptions: \
>>> +// RUN:  [{key: readability-identifier-naming.ObjcIvarPrefix, value:
>>> '_'}]}' \
>>> +// RUN: --
>>> +
>>> +@interface Foo
>>> +@end
>>> +
>>> +@implementation Foo {
>>> +int _bar;
>>> +int barWithoutPrefix;
>>> +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for
>>> objc ivar 'barWithoutPrefix' [readability-identifier-naming]
>>> +// CHECK-FIXES: int _barWithoutPrefix;
>>> +}
>>> +@end
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>
> --
> Best regards
> Yan Zhang
>


-- 
Best regards
Yan Zhang
___
cfe-commits mailing list

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Yan Zhang via cfe-commits

I am running tests locally with "ninja check-clang-tools" and I am sure it
is running this test because I could get error message when I debug it.

The problem (according to the error message) is all caused by different
architecture. It seems a lot of ObjC features are not supported in old
32-bit OSX (which I believe the test bots are using). I have another
revision sent out to see if it can help. Can you take a quick look?
https://reviews.llvm.org/D45936

On Sun, Apr 22, 2018 at 5:51 PM Chandler Carruth 
wrote:

> The commit log here no longer reflects the commit. This is not just
> updating the test, this is a complete re-application of the original patch
> in r330492. =[
>
> Also, the bots are still complaining:
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
> http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
> http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659
>
> I'm not sure how you're running your tests that you don't see these
> issues, but they seem to reproduce on many build bots and the error message
> doesn't seem to be architecture specific at all...
>
> I suspect something about how you are trying to run tests isn't actually
> running this test if you aren't able to locally reproduce.
>
> On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: wizard
>> Date: Sun Apr 22 17:15:15 2018
>> New Revision: 330559
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
>> Log:
>> update test to use ivar in implementation instead of class extension
>>
>> Summary: using ivar in class extension is not supported in 32-bit
>> architecture of MacOS.
>>
>> Reviewers: alexfh, hokein
>>
>> Reviewed By: alexfh
>>
>> Subscribers: klimek, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D45912
>>
>> Added:
>>
>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>> Modified:
>>
>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>>
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff
>>
>> ==
>> ---
>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>> (original)
>> +++
>> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>> Sun Apr 22 17:15:15 2018
>> @@ -109,6 +109,7 @@ namespace readability {
>>  m(TemplateParameter) \
>>  m(TypeAlias) \
>>  m(MacroDefinition) \
>> +m(ObjcIvar) \
>>
>>  enum StyleKind {
>>  #define ENUMERATE(v) SK_ ## v,
>> @@ -384,6 +385,9 @@ static StyleKind findStyleKind(
>>  const NamedDecl *D,
>>  const std::vector
>>  ) {
>> +  if (isa(D) && NamingStyles[SK_ObjcIvar])
>> +return SK_ObjcIvar;
>> +
>>if (isa(D) && NamingStyles[SK_Typedef])
>>  return SK_Typedef;
>>
>>
>> Added:
>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m?rev=330559=auto
>>
>> ==
>> ---
>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>> (added)
>> +++
>> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
>> Sun Apr 22 17:15:15 2018
>> @@ -0,0 +1,15 @@
>> +// RUN: %check_clang_tidy %s readability-identifier-naming %t \
>> +// RUN: -config='{CheckOptions: \
>> +// RUN:  [{key: readability-identifier-naming.ObjcIvarPrefix, value:
>> '_'}]}' \
>> +// RUN: --
>> +
>> +@interface Foo
>> +@end
>> +
>> +@implementation Foo {
>> +int _bar;
>> +int barWithoutPrefix;
>> +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for
>> objc ivar 'barWithoutPrefix' [readability-identifier-naming]
>> +// CHECK-FIXES: int _barWithoutPrefix;
>> +}
>> +@end
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>

-- 
Best regards
Yan Zhang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

2018-04-22 Thread Chandler Carruth via cfe-commits

The commit log here no longer reflects the commit. This is not just
updating the test, this is a complete re-application of the original patch
in r330492. =[

Also, the bots are still complaining:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830
http://lab.llvm.org:8011/builders/clang-cmake-armv8-quick/builds/1979
http://lab.llvm.org:8011/builders/clang-ppc64le-linux-lnt/builds/11659

I'm not sure how you're running your tests that you don't see these issues,
but they seem to reproduce on many build bots and the error message doesn't
seem to be architecture specific at all...

I suspect something about how you are trying to run tests isn't actually
running this test if you aren't able to locally reproduce.

On Sun, Apr 22, 2018 at 5:19 PM Yan Zhang via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: wizard
> Date: Sun Apr 22 17:15:15 2018
> New Revision: 330559
>
> URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev
> Log:
> update test to use ivar in implementation instead of class extension
>
> Summary: using ivar in class extension is not supported in 32-bit
> architecture of MacOS.
>
> Reviewers: alexfh, hokein
>
> Reviewed By: alexfh
>
> Subscribers: klimek, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D45912
>
> Added:
>
> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
> Modified:
>
> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp?rev=330559=330558=330559=diff
>
> ==
> ---
> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
> (original)
> +++
> clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
> Sun Apr 22 17:15:15 2018
> @@ -109,6 +109,7 @@ namespace readability {
>  m(TemplateParameter) \
>  m(TypeAlias) \
>  m(MacroDefinition) \
> +m(ObjcIvar) \
>
>  enum StyleKind {
>  #define ENUMERATE(v) SK_ ## v,
> @@ -384,6 +385,9 @@ static StyleKind findStyleKind(
>  const NamedDecl *D,
>  const std::vector
>  ) {
> +  if (isa(D) && NamingStyles[SK_ObjcIvar])
> +return SK_ObjcIvar;
> +
>if (isa(D) && NamingStyles[SK_Typedef])
>  return SK_Typedef;
>
>
> Added:
> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m?rev=330559=auto
>
> ==
> ---
> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
> (added)
> +++
> clang-tools-extra/trunk/test/clang-tidy/readability-identifier-naming-objc.m
> Sun Apr 22 17:15:15 2018
> @@ -0,0 +1,15 @@
> +// RUN: %check_clang_tidy %s readability-identifier-naming %t \
> +// RUN: -config='{CheckOptions: \
> +// RUN:  [{key: readability-identifier-naming.ObjcIvarPrefix, value:
> '_'}]}' \
> +// RUN: --
> +
> +@interface Foo
> +@end
> +
> +@implementation Foo {
> +int _bar;
> +int barWithoutPrefix;
> +// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for
> objc ivar 'barWithoutPrefix' [readability-identifier-naming]
> +// CHECK-FIXES: int _barWithoutPrefix;
> +}
> +@end
>
>
> ___
> 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] r330559 - update test to use ivar in implementation instead of class extension

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

9 matches

Site Navigation

Mail list logo

Footer information