NoQ added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:1310
+def MIGServerRoutine : InheritableAttr {
+ let Spellings = [Clang<"mig_server_routine">];
----------------
aaron.ballman wrote:
> Should this be limited to specific targets, or is this a general concept that
> applies to all targets?
I guess it shouldn't. There are [[
https://en.wikipedia.org/wiki/Mach_(kernel)#Software_based_on_Mach | a lot of
]] Darwin-inspecific forks of Mach, and restricting to a specific hardware
architecture also doesn't seem to make much sense.
================
Comment at: clang/include/clang/Basic/Attr.td:1312
+ let Spellings = [Clang<"mig_server_routine">];
+ let Subjects = SubjectList<[Function]>;
+ let Documentation = [MIGCallingConventionDocs];
----------------
aaron.ballman wrote:
> Objective-C methods as well?
Mmm, i guess it's technically possible, even if a bit annoying to support. Let
me try.
================
Comment at: clang/include/clang/Basic/Attr.td:1313
+ let Subjects = SubjectList<[Function]>;
+ let Documentation = [MIGCallingConventionDocs];
+}
----------------
aaron.ballman wrote:
> This isn't really a calling convention though, is it? It doesn't change
> codegen, impact function types, or anything like that.
That's right, but for whatever reason it's often referred to as "a" calling
convention. It makes slight sense because callee/caller-deallocated OOL
parameters are kinda similar to callee/caller-saved registers, something like
that. I guess i'll call it just "convention".
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8702
+def warn_mig_server_routine_does_not_return_kern_return_t : Warning<
+ "'%0' attribute only applies to functions that return a kernel return code">,
+ InGroup<IgnoredAttributes>;
----------------
aaron.ballman wrote:
> Will users understand "kernel return code"? Should this say `kern_return_t`
> explicitly?
>
> No need to use %0 here, just spell out the attribute name directly (unless
> you expect this to be used by multiple attributes, in which case the name of
> the diagnostic should be changed).
It should say either `kern_return_t` or `IOReturn` depending on the specific
framework that's being used (the latter is a typedef for the former). I guess i
could scan the AST to for a `typedef kern_return_t IOReturn` and display the
appropriate message, but this sort of stuff always sounds like an overkill. For
now i change the wording to an exact "a kern_return_t". I could also say "a
kern_return_t or an IOReturn", do you have any preference here?
================
Comment at: clang/test/Sema/attr-mig.c:17
+}
+
+kern_return_t bar_forward() { // no-warning
----------------
aaron.ballman wrote:
> Here's an edge case to consider:
> ```
> __attribute__((mig_server_routine)) int foo(void);
>
> typedef int kern_return_t;
>
> kern_return_t foo(void) { ... }
> ```
> Do you have to care about situations like that?
> Do you have to care about situations like that?
I hope i not :) `kern_return_t` is available pretty much everywhere and most
likely it's not a problem to update the first declaration of `foo()` with
`kern_return_t`. Ok if i add a relaxing code later if it turns out that i have
to?
================
Comment at: clang/test/Sema/attr-mig.cpp:10
+public:
+ virtual __attribute__((mig_server_routine)) IOReturn externalMethod();
+ virtual __attribute__((mig_server_routine)) void anotherMethod(); //
expected-warning{{'mig_server_routine' attribute only applies to functions that
return a kernel return code}}
----------------
aaron.ballman wrote:
> Can you use the C++ spelling for the attribute, so we have a bit of coverage
> for that?
Is there a vision that i should also provide a namespaced C++ attribute, eg.
`[[mig::server_routine]]`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58365/new/
https://reviews.llvm.org/D58365
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
