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

[jahanian]

On Sep 29, 2012, at 11:01 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> 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…

All should be in r164957. Thanks for feedback.
- fariborz

> 
> 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>*/