aaron.ballman added inline comments.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:5158
+  case llvm::Triple::avr:
+    handleAVRInterruptAttr(S, D, Attr);
+    break;
----------------
aaron.ballman wrote:
> Just call `handleSimpleAttribute()` instead.
Since this is no longer truly a simple attribute (it requires some custom 
checking), I think my earlier advice was wrong -- you should split this into a 
`handleAVRInterruptAttr()` method. I forgot that you need the custom checking 
because the attribute has custom parsing.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5721
+  case AttributeList::AT_AVRSignal:
+    handleAVRSignalAttr(S, D, Attr);
+    break;
----------------
aaron.ballman wrote:
> ...same here.
... same here.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5145
+    if (!isFunctionOrMethod(D)) {
+      S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+          << "'interrupt'" << ExpectedFunctionOrMethod;
----------------
dylanmckay wrote:
> I'm pretty sure that this check shouldn't be necessary, because we define 
> `Subjects = [Function]` in TableGen.
> 
> Without it though, the warning doesn't appear. Do you know why that is 
> @aaron.ballman?
It's because it requires custom parsing.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5146
+      S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
+          << "'interrupt'" << ExpectedFunctionOrMethod;
+      return;
----------------
The diagnostic and the predicate for it don't match your `Subjects` line. 
Should this appertain to ObjC methods? If not, you want to use 
`ExpectedFunction` instead. If so, you want to add `ObjCMethod` to the 
`Subjects` line.


https://reviews.llvm.org/D28451



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to