[PATCH] D56663: [MSP430] Improve support of 'interrupt' attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:277-280
+def warn_msp430_interrupt_attribute : Warning<
+   "MSP430 'interrupt' attribute only applies to functions that have "
+   "%select{no parameters|a 'void' return type}0">,
+   InGroup<IgnoredAttributes>;
----------------
I'd fold this in to the preceding diagnostics using `%select{}`. Perhaps the new diagnostic name should be `warn_interrupt_attribute_invalid_subject` or some such?


================
Comment at: lib/CodeGen/TargetInfo.cpp:6777
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
-    if (const MSP430InterruptAttr *attr = FD->getAttr<MSP430InterruptAttr>()) {
-      // Handle 'interrupt' attribute:
-      llvm::Function *F = cast<llvm::Function>(GV);
+    const MSP430InterruptAttr *Attr = FD->getAttr<MSP430InterruptAttr>();
+    if (!Attr)
----------------
Might as well switch this to using `const auto *`; also, the variable should be renamed to not conflict with the name of a type.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:5400
+
+  // The attribute takes one integer argument
   if (!checkAttributeNumArgs(S, AL, 1))
----------------
Missing a full stop here as well.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56663/new/

https://reviews.llvm.org/D56663