aaron.ballman added inline comments.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2504
@@ -2493,3 +2503,3 @@
 
 // Availability attribute
 def warn_availability_unknown_platform : Warning<
----------------
>> It would be good to model these new diagnostics after the MIPS interrupt 
>> diagnostics.
>> 
>> def warn_mips_interrupt_attribute : Warning<
>>    "MIPS 'interrupt' attribute only applies to functions that have "
>>    "%select{no parameters|a 'void' return type}0">,
>>    InGroup<IgnoredAttributes>;

> Ok, will do

This does not appear to have been completed yet.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4556
@@ +4555,3 @@
+  // e) The 2nd argument (if any) must be an unsigned integer.
+  if (!isFunctionOrMethod(D) || !hasFunctionProto(D) ||
+      !D->getDeclContext()->isFileContext()) {
----------------
>> 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).

I don't see a test using a named namespace with an interrupt function (though I 
do see one disallowing a static member function). Also, I am still wondering 
why there is a restriction against static member functions. (I looked at GCC's 
docs and they do not claim to support this attribute for these targets, so I'm 
not certain what this design is based on.)

================
Comment at: test/SemaCXX/attr-x86-interrupt.cpp:51
@@ +50,3 @@
+template <typename T>
+void bar(T *a) {
+  foo9(a); // expected-error {{interrupt service routine can't be called 
directly}}
----------------
Ah, I'm sorry, I wasn't very clear with my template request. I was looking for 
something like (in addition to what you've added):
```
template <typename Fn>
void bar(Fn F) {
  F(nullptr); // Should this diagnose? I expect not.
}

__attribute__((interrupt)) void foo(int *) {}
void f() {
  bar(foo);
}
```


http://reviews.llvm.org/D15709



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to