arphaman marked 3 inline comments as done.
arphaman added inline comments.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:7230
+ return;
+ for (const auto &M : S.getPreprocessor().macros()) {
+ if (M.first->getName() != "API_AVAILABLE")
----------------
erik.pilkington wrote:
> Its unfortunate to loop over every macro. Can we use
> Preprocessor::getMacroDefinition()?
Good call.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:7231
+ for (const auto &M : S.getPreprocessor().macros()) {
+ if (M.first->getName() != "API_AVAILABLE")
+ continue;
----------------
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.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:7240
+ FixitNoteDiag << FixItHint::CreateInsertion(
+ Enclosing->getLocStart(), (llvm::Twine("API_AVAILABLE(") +
+ PlatformName + "(" + Introduced +
"))\n")
----------------
erik.pilkington wrote:
> 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.
Yeah, Good point.
Repository:
rL LLVM
https://reviews.llvm.org/D35726
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
