[cfe-commits] r164861 - in /cfe/trunk: bindings/xml/comment-xml-schema.rng tools/libclang/CXComment.cpp

[Dmitri Gribenko]

On Sat, Sep 29, 2012 at 8:54 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>
> On Sep 29, 2012, at 1:26 AM, Dmitri Gribenko wrote:
>
>> On Sat, Sep 29, 2012 at 1:35 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>>> +      StringRef distribution;
>>> +      if (AA->getPlatform()) {
>>> +        distribution = AA->getPlatform()->getName();
>>> +        if (distribution == "macosx")
>>> +          distribution = "OSX";
>>> +        else
>>> +          distribution = "iOS";
>>> +      }
>>
>> Do we need to do this translation here?  Why do we map everything that
>> is not "macosx" to "iOS"?
>
> Yes, translation is necessary. There are currently macosx and ios as allowed platforms in availability macros. I will probably check for "ios" and assert for anything else.

I don't know if we should assert -- do we reject the attribute in
other cases or just emit a warning?  Maybe just fold all other
platform names to something like 'Unknown'?

>> Please add tests for CXComment.cpp changes to
>> test/Index/annotate-comments.cpp.  It is a bit crowded and slow to
>> FileCheck already, so you might want to put tests for availability
>> attrs into new file, something like
>> annotate-comments-availability-attrs.cpp.
>
> I was planning to add a test. I got bogged down trying to add a workable test to annotate-comments.cpp.
> I will add one soon though.

OK.  Please consider creating a new file.  I know how hard
annotate-comments.cpp is -- adding "just one more test" tends to
change line numbers everywhere and forces a change to the
corresponding CHECK lines...

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/