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

Yan Zhang via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 22 18:03:10 PDT 2018


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


More information about the cfe-commits mailing list