[PATCH] D35726: unguarded availability: add a fixit for the "annotate '...' with an availability attribute to silence" note

Erik Pilkington via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 21 09:39:13 PDT 2017


erik.pilkington added a comment.

Thanks for working on this! This looks like it would be very useful.



================
Comment at: lib/Sema/SemaDeclAttr.cpp:7230
+        return;
+      for (const auto &M : S.getPreprocessor().macros()) {
+        if (M.first->getName() != "API_AVAILABLE")
----------------
Its unfortunate to loop over every macro. Can we use Preprocessor::getMacroDefinition()?


================
Comment at: lib/Sema/SemaDeclAttr.cpp:7231
+      for (const auto &M : S.getPreprocessor().macros()) {
+        if (M.first->getName() != "API_AVAILABLE")
+          continue;
----------------
It would be nice if we could recommend using this macro even if it isn't defined, as users might not have included the <os/availability.h> header. Maybe we can do that on apple platforms, noting that the the macro is declared in os/availability.h if it isn't already defined?


================
Comment at: lib/Sema/SemaDeclAttr.cpp:7238
+        std::string Introduced =
+            ReferringDecl->getVersionIntroduced().getAsString();
+        FixitNoteDiag << FixItHint::CreateInsertion(
----------------
OffendingDecl, right? Thats the one with the offending availability attribute


================
Comment at: lib/Sema/SemaDeclAttr.cpp:7240
+        FixitNoteDiag << FixItHint::CreateInsertion(
+            Enclosing->getLocStart(), (llvm::Twine("API_AVAILABLE(") +
+                                       PlatformName + "(" + Introduced + "))\n")
----------------
I was somewhat uncertain about adding a fixit for this because its difficult to determine where exactly the availability attribute should go, it looks like this doesn't emit an attribute at the right place for this, for example. (It should go after the meth)
```
@interface X
-(new_int)meth;
@end
```
Maybe we can do better if we look at what Enclosing actually declares? It's not worth it to emit an incorrect fixit.


Repository:
  rL LLVM

https://reviews.llvm.org/D35726





More information about the cfe-commits mailing list