[PATCH] D28451: [AVR] Add support for the 'interrupt' and 'naked' attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The new tests aren't really what I had in mind. The codegen tests that should be sema tests can just be rolled into your existing sema tests, by the way.



================
Comment at: lib/Sema/SemaDeclAttr.cpp:5137
+  if (!checkAttributeNumArgs(S, Attr, 0))
+    Attr.setInvalid();
+
----------------
This should simply return rather than attempt to attach an invalid attribute to the declaration (same below).


================
Comment at: test/CodeGen/avr/attributes/interrupt-no-args.c:3
+
+// CHECK: error: 'interrupt' attribute takes no arguments
+__attribute__((interrupt(12))) void foo(void) { }
----------------
This should be a test in Sema, not in CodeGen.


================
Comment at: test/CodeGen/avr/attributes/interrupt.c:3
+
+// CHECK: define void @foo() #0
+__attribute__((interrupt)) void foo(void) { }
----------------
As should this.


================
Comment at: test/CodeGen/avr/attributes/interrupt.m:1
+// RUN: %clang_cc1 -triple avr-unknown-unknown -x objective-c++ -emit-llvm -o - %s | FileCheck %s
+
----------------
Why objective-c++?


================
Comment at: test/CodeGen/avr/attributes/interrupt.m:4
+// CHECK: define void @_Z3foov() #0
+void foo() __attribute__((interrupt)) { }
+
----------------
This is not an Objective-C method decl, so it doesn't really test anything new. The test I was envisioning was something like:
```
@interface F // There's probably an expected warning here about a missing base class
-(void) foo __attribute__((interrupt));
@end

@implementation F
-(void) foo __attribute__((interrupt)) {}
@end
```


================
Comment at: test/CodeGen/avr/attributes/signal-no-args.c:4
+// CHECK: error: 'signal' attribute takes no arguments
+__attribute__((signal(12))) void foo(void) { }
----------------
This should also be a Sema test.


================
Comment at: test/CodeGen/avr/attributes/signal.m:4
+// CHECK: define void @_Z3foov() #0
+void foo() __attribute__((signal)) { }
+
----------------
Same concerns about this not testing anything new as above.


https://reviews.llvm.org/D28451