[PATCH] D28451: [AVR] Add support for the 'interrupt' and 'naked' attributes
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 23 07:31:29 PST 2017
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
More information about the cfe-commits
mailing list