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

Sun Apr 22 18:14:43 PDT 2018

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 <chandlerc at gmail.com>
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 at 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 <chandlerc at gmail.com>
>> 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 <chandlerc at gmail.com>
>>> 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=<triple> 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 <ynzhang at google.com> 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 <chandlerc at gmail.com>
>>>>> 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 at 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&view=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&r1=330558&r2=330559&view=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<llvm::Optional<IdentifierNamingCheck::NamingStyle>>
>>>>>>>          &NamingStyles) {
>>>>>>> +  if (isa<ObjCIvarDecl>(D) && NamingStyles[SK_ObjcIvar])
>>>>>>> +    return SK_ObjcIvar;
>>>>>>> +
>>>>>>>    if (isa<TypedefDecl>(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&view=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: --
>>>>>>> +
>>>>>>> + at interface Foo
>>>>>>> + at end
>>>>>>> +
>>>>>>> + at 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;
>>>>>>> +}
>>>>>>> + at end
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> cfe-commits mailing list
>>>>>>> cfe-commits at 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
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>

-- 
Best regards
Yan Zhang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180423/7903dcad/attachment.html>