aaron.ballman added inline comments.
================ Comment at: lib/Sema/SemaDeclAttr.cpp:5305 + + if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { + S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0; ---------------- apazos wrote: > aaron.ballman wrote: > > I would have assumed this would be: `!hasFunctionProto(D) || > > getFunctionOrMethodNumParams(D) != 0`, but it depends on whether you want > > to support K&R C functions. > hasFunctionProto also returns true for a function definition like this one > __attribute__((interrupt)) void foo1(int) {}. That's correct, which is why I recommended the predicate the way I did. If the function has no prototype, we want to diagnose it. If the function has a prototype, we want to see how many parameters are listed and if there are more than zero parameters, we want to diagnose it. ================ Comment at: test/Sema/riscv-interrupt-attr.c:23 + // expected-note {{repeated RISC-V 'interrupt' attribute is here}} +__attribute__((interrupt("user"))) void foo8() {} +__attribute__((interrupt("supervisor"))) void foo9() {} ---------------- apazos wrote: > aaron.ballman wrote: > > aaron.ballman wrote: > > > Do you intend for functions without a prototype to be accepted? foo8() > > > can be passed an arbitrary number of arguments, which is a bit different > > > than what I thought you wanted the semantic check to be. > > This question remains outstanding. > The checks are validating both function definitions and function prototypes > like these: > _attribute__((interrupt)) void foo1() {} > __attribute__((interrupt)) void foo(void); > Not sure what the confusion is. Ah, now I see where the confusion is. In C, an empty parameter list declares a function without a prototype; functions without prototypes can accept any number of arguments. To declare a function that accepts no arguments, you must have a prototype for the function and the parameter list is void. In C++, all functions are prototyped and an empty parameter list is equivalent to a parameter list of void. The word "prototype" doesn't mean "forward declaration". e.g., ``` // C code void foo1(); // Declaration; no prototype; accepts any number of arguments. void foo2() {} // Definition; no prototype; accepts any number of arguments. void foo3(void); // Declaration; prototype; accepts no arguments. void foo4(void) {} // Definition; prototype; accepts no arguments. foo2(1, 2, 3); // ok foo4(1, 2, 3); // error ``` Because a function without a prototype can accept any number of arguments, I think you want to diagnose such a function signature. https://reviews.llvm.org/D48412 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits