[PATCH] D15709: [X86] Support 'interrupt' attribute for x86

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: include/clang/Basic/Attr.td:1531
@@ -1529,1 +1530,3 @@
 
+def IAInterrupt : InheritableAttr, TargetSpecificAttr<TargetAnyX86> {
+  // NOTE: If you add any additional spellings, ARMInterrupt's,
----------------
Maybe this (and IAInterruptDocs) should be named AnyX86Interrupt as well.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4556
@@ +4555,3 @@
+  if (!isFunctionOrMethod(D) || !hasFunctionProto(D) ||
+      !D->getDeclContext()->isFileContext()) {
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
----------------
> Yes, we allow any non-member functions.

Then we should have a test that shows this working with a named namespace interrupt function. Also, it's a bit odd to me that members of named namespaces are fine, but static member functions are not. This disallows possibly-sensible code (like a private static member function of a class for better encapsulation).

================
Comment at: test/Sema/attr-x86-interrupt.c:54
@@ +53,3 @@
+  foo8((int *)argv);       // expected-error {{interrupt service routine can't be used directly}}
+  return 0;
+}
----------------
I'd like to see a test like:
```
__attribute__((interrupt)) void foo8(int *a) {}

void g(void (*fp)(int *));

void f() {
  g(foo8);
}
```
as I think this is supposed to diagnose. It might also be good to have a similar test using templates in C++.


http://reviews.llvm.org/D15709