aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:1860
+def NSErrorDomain : Attr {
+ let Spellings = [GNU<"ns_error_domain">];
+ let Args = [IdentifierArgument<"ErrorDomain">];
----------------
gribozavr2 wrote:
> aaron.ballman wrote:
> > MForster wrote:
> > > gribozavr2 wrote:
> > > > Could we try to add a list of subjects here? It seems like it is a
> > > > type-only attribute, and most likely enum-only.
> > > >
> > > > let Subjects = SubjectList<[Enum]>;
> > > @milseman, could you comment on this?
> > >
> > > In the meantime I've added the restriction. Obviously this makes the
> > > tests fail. I will also test this change against the Swift unit tests.
> > FWIW, this is not a attribute; it's a declaration attribute.
> >
> > Is there a reason it's not inheritable?
> >
> > I assume it's not getting a Clang spelling because Objective-C isn't
> > tracking C2x yet? (Though that spelling still seems useful to Objective-C++
> > users in general for these NS attributes.)
> > FWIW, this is not a attribute; it's a declaration attribute.
>
> Sorry, yes, of course I meant to say "declaration attribute".
>
> > Is there a reason it's not inheritable?
>
> Good observation, I think it should be.
>
> > I assume it's not getting a Clang spelling because Objective-C isn't
> > tracking C2x yet?
>
> Cocoa users are expected to use the `NS_*` macros instead of using the
> attribute directly, so even if a C2x spelling was an option (IDK if it is),
> there would be very limited use for it.
> Cocoa users are expected to use the NS_* macros instead of using the
> attribute directly, so even if a C2x spelling was an option (IDK if it is),
> there would be very limited use for it.
Okay, that's reasonable, thanks!
================
Comment at: clang/test/Analysis/ns_error_enum.m:1
+// RUN: %clang_cc1 -verify %s
+
----------------
gribozavr2 wrote:
> aaron.ballman wrote:
> > MForster wrote:
> > > gribozavr2 wrote:
> > > > This file is a `.m` -- any specific reason? I'd call it `.c` and run
> > > > the test in C, Objective-C, and C++ modes (enums might work slightly
> > > > differently, the name lookup functionality might work differently).
> > > The test doesn't compile in C or C++ (`non-defining declaration of
> > > enumeration with a fixed underlying type is only permitted as a
> > > standalone declaration; missing list of enumerators?`). Not sure if it's
> > > worth adapting.
> > Enums with fixed underlying types exist in C++ and C, so I was expecting
> > the attribute to work there. If the attribute isn't supported in these
> > languages, should the attribute be tied to a language mode?
> There are Apple SDK headers that parse in all language modes (C, Objective-C,
> C++, Objective-C++), so I think it is quite important to test this feature in
> all modes. I suspect the reason for the error is that different language
> modes require a slightly different macro definition.
> There are Apple SDK headers that parse in all language modes (C, Objective-C,
> C++, Objective-C++), so I think it is quite important to test this feature in
> all modes.
In that case, I definitely agree. This should have multiple RUN lines to test
the various language modes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84005/new/
https://reviews.llvm.org/D84005
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
