[PATCH] D10802: [mips] Interrupt attribute support.

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:256
@@ +255,3 @@
+def warn_mips_interrupt_attribute : Warning<
+   "function %0 must %select{take no arguments|have the \'void\' return type}1"
+   " for the 'interrupt' attribute for MIPS">,
----------------
No need for \'; you can use ' directly.

I think the wording is still a bit awkward. Function declarations generally don't "take arguments", but have parameters, and "the void return type" reads weird.  I think:

"MIPS 'interrupt' attribute only applies to functions with %select{no parameters|a 'void' return type}0"

would be an improvement.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4439
@@ +4438,3 @@
+    S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+        << "\'interrupt\'" << ExpectedFunctionOrMethod;
+    return;
----------------
No need for \', you can use ' directly.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4462
@@ +4461,3 @@
+    S.Diag(Attr.getLoc(), diag::warn_attribute_type_not_supported)
+        << Attr.getName() << "\'" + std::string(Str) + "\'";
+    return;
----------------
No need for \', can use ' directly.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4484
@@ +4483,3 @@
+static void handleMips16Attribute(Sema &S, Decl *D, const AttributeList &Attr) {
+  if (checkAttrMutualExclusion<NoMips16Attr>(S, D, Attr.getRange(), Attr.getName()))
+    return;
----------------
This is a good change, but should be as part of a separate patch since it has nothing to do with MIPS interrupt.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4495
@@ +4494,3 @@
+static void handleNoMips16Attribute(Sema &S, Decl *D, const AttributeList &Attr) {
+  if (checkAttrMutualExclusion<Mips16Attr>(S, D, Attr.getRange(), Attr.getName()))
+    return;
----------------
Same with this.

================
Comment at: test/Sema/mips16_attr_allowed.c:22
@@ -21,1 +21,3 @@
 
+void __attribute__((mips16, nomips16)) foo16c(); // expected-error {{'mips16' and 'nomips16' attributes are not compatible}} \
+                                                 // expected-note {{conflicting attribute is here}}
----------------
This should be part of a separate patch.


http://reviews.llvm.org/D10802