This is an archive of the discontinued LLVM Phabricator instance.

[Availability] Improve availability to consider functions run at load time
ClosedPublic

Authored by steven_wu on Apr 16 2018, 2:10 PM.

Details

Summary

There are some functions/methods that run when the application launches
or the library loads. Those functions will run reguardless the OS
version as long as it satifies the minimum deployment target. Annotate
them with availability attributes doesn't really make sense because they
are essentially available on all targets since minimum deployment
target.

rdar://problem/36093384

Diff Detail

Event Timeline

steven_wu created this revision.Apr 16 2018, 2:10 PM

Hi Steven, thanks for working on this!

include/clang/Basic/DiagnosticSemaKinds.td
2916

typo: destructor

lib/Sema/SemaDecl.cpp
9134–9151

Shouldn't this be in ProcessDeclAttributeList in SemaDeclAttr.cpp?

9136

Can't this just be: if (NewFD->hasAttr<AvailabilityAttr>())?

lib/Sema/SemaDeclAttr.cpp
6828

getNameAsString is deprecated, maybe you should get this from MethodD->getSelector()?

lib/Sema/SemaDeclObjC.cpp
4737–4738

Second sentence might read better as "It get called on startup, so it has to have the availability of the deployment target".

4739–4740

This should also be if (ObjCMethod->hasAttr<AvailabilityAttr>)

4744

Same comment as above!

test/SemaObjC/unguarded-availability.m
334

Could you add a test that +(void)load: (int)x; still works? It should still accept availability attributes, right?

Thanks for reviewing Erik!

lib/Sema/SemaDecl.cpp
9134–9151

This has to happen after mergeDeclAttributes, otherwise, you won't catch the case which the availability attributes and constructor attributes are on different decl that gets merged. It can't be in mergeDeclAttributes as well, then it won't catch the case they are on a single decl.

9136

The location of the diagnostics need to be on attributes and that is why it is a for loop.

steven_wu updated this revision to Diff 142713.Apr 16 2018, 3:59 PM

Address review feedback. Fix typos and comments.

steven_wu marked 5 inline comments as done.Apr 16 2018, 3:59 PM
erik.pilkington accepted this revision.Apr 16 2018, 4:18 PM

LGTM, thanks!

lib/Sema/SemaDecl.cpp
9134–9151

Ah, okay. Makes sense.

9136

Maybe if (auto *AA = NewFD->getAttr<AvailabilityAttr>()) then?

This revision is now accepted and ready to land.Apr 16 2018, 4:18 PM
steven_wu updated this revision to Diff 142717.Apr 16 2018, 4:35 PM

Address review feedback

This revision was automatically updated to reflect the committed changes.