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

Sun Apr 22 18:13:06 PDT 2018

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180423/cbb667b8/attachment-0001.html>