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

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington accepted this revision.
erik.pilkington added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for working on this!



================
Comment at: lib/Sema/SemaDeclAttr.cpp:7230
+        return;
+      for (const auto &M : S.getPreprocessor().macros()) {
+        if (M.first->getName() != "API_AVAILABLE")
----------------
arphaman wrote:
> erik.pilkington wrote:
> > Its unfortunate to loop over every macro. Can we use Preprocessor::getMacroDefinition()?
> Good call.
Looks like there is a convenience function: isMacroDefined(StringRef)


================
Comment at: lib/Sema/SemaDeclAttr.cpp:7231
+      for (const auto &M : S.getPreprocessor().macros()) {
+        if (M.first->getName() != "API_AVAILABLE")
+          continue;
----------------
arphaman wrote:
> erik.pilkington wrote:
> > 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?
> I think we either have to go the full way (i.e. have an `#include <os/availability.h>` fixit inserted as well) or just avoid any fixits. Note that these fixits will be used pretty much only in Xcode which doesn't show any notes that don't have fixits, so an additional note wouldn't make sense. We could potentially change the message of `note_partial_availability_silence` but I doubt it's that useful.
Ok, I think this is fine.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:7063
+  }
+  return AttributeInsertion::createInsertionBefore(D);
+}
----------------
TagDecls also have a special rule: `struct __attribute__(...) X;`.


Repository:
  rL LLVM

https://reviews.llvm.org/D35726