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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits