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

[Dmitri Gribenko]

On Monday, October 1, 2012, jahanian wrote:

>
> On Sep 29, 2012, at 11:01 AM, Dmitri Gribenko <gribozavr at gmail.com<javascript:;>>
> wrote:
>
> > On Sat, Sep 29, 2012 at 8:54 PM, Fariborz Jahanian <fjahanian at apple.com<javascript:;>>
> 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 <javascript:;>> 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


Hi Fariborz,

You forgot to add the schema test to the C file that actually runs tests.

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