aaron.ballman added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:255
@@ -254,1 +254,3 @@
def err_parameter_name_omitted : Error<"parameter name omitted">;
+def err_excess_arguments : Error<"function %0 has too many arguments">;
+def err_wrong_return_type : Error<"function %0 does not have the \'void\'
return type">;
----------------
I would combine these, and name the result so that it is obvious it relates to
the MIPS interrupt attribute. This can then become: "MIPS 'interrupt' attribute
only applies to functions %select{with no parameters|a 'void' return type}0"
Generally speaking, we make these warnings instead of errors (though the choice
is obviously yours), and add them to the IgnoredAttributes group (so you warn,
and then never attach the attribute to the declaration).
================
Comment at: lib/Sema/SemaDeclAttr.cpp:4441
@@ +4440,3 @@
+
+ DeclarationName Name = D->getAsFunction()->getNameInfo().getName();
+
----------------
No need for this; you can pass any NamedDecl * into Diag() and it will get
quoted and displayed properly. Since you've already checked that it's a
function or method, you can simply use cast<NamedDecl>(D) when needed.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:4448
@@ +4447,3 @@
+
+ if (!(getFunctionOrMethodResultType(D)->isVoidType())) {
+ S.Diag(D->getLocation(), diag::err_wrong_return_type) << Name;
----------------
No need for the extra set of parens.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:4453
@@ +4452,3 @@
+
+ if (D->hasAttr<Mips16Attr>()) {
+ S.Diag(Attr.getLoc(), diag::err_attributes_are_not_compatible)
----------------
Please use checkAttrMutualExclusion() instead. Also, you need to add a similar
check to the Mips16 attribute handler to ensure the mutual exclusion is
properly checked. Should have semantic test cases for both orderings:
```
__attribute__((mips16)) __attribute__((interrupt)) void f();
__attribute__((interrupt)) __attribute__((mips16)) void g();
```
What about the nomips16 attribute? Should this be mutually exclusive as well?
http://reviews.llvm.org/D10802
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
