erik.pilkington added a comment.
Hi Steven, thanks for working on this!
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2916
+ "ignoring availability attribute %select{on '+load' method|"
+ "with constructor attribute|with desctructor attribute}0">,
+ InGroup<Availability>;
----------------
typo: destructor
================
Comment at: lib/Sema/SemaDecl.cpp:9134-9151
+ // Diagnose availability attributes. Availability cannot be used on functions
+ // that are run during load/unload.
+ for (const auto& attr: NewFD->attrs()) {
+ if (!isa<AvailabilityAttr>(attr))
+ continue;
+
+ if (NewFD->hasAttr<ConstructorAttr>()) {
----------------
Shouldn't this be in ProcessDeclAttributeList in SemaDeclAttr.cpp?
================
Comment at: lib/Sema/SemaDecl.cpp:9136
+ // that are run during load/unload.
+ for (const auto& attr: NewFD->attrs()) {
+ if (!isa<AvailabilityAttr>(attr))
----------------
Can't this just be: `if (NewFD->hasAttr<AvailabilityAttr>())`?
================
Comment at: lib/Sema/SemaDeclAttr.cpp:6828
+ if (const auto *MethodD = dyn_cast<ObjCMethodDecl>(Ctx))
+ if (MethodD->isClassMethod() && MethodD->getNameAsString() == "load")
+ return true;
----------------
getNameAsString is deprecated, maybe you should get this from
MethodD->getSelector()?
================
Comment at: lib/Sema/SemaDeclObjC.cpp:4737-4738
+ // + load method cannot have availability attributes. It has to be the same
+ // as deployment target.
+ for (const auto& attr: ObjCMethod->attrs()) {
----------------
Second sentence might read better as "It get called on startup, so it has to
have the availability of the deployment target".
================
Comment at: lib/Sema/SemaDeclObjC.cpp:4739-4740
+ // as deployment target.
+ for (const auto& attr: ObjCMethod->attrs()) {
+ if (!isa<AvailabilityAttr>(attr))
+ continue;
----------------
This should also be `if (ObjCMethod->hasAttr<AvailabilityAttr>)`
================
Comment at: lib/Sema/SemaDeclObjC.cpp:4744
+ if (ObjCMethod->isClassMethod() &&
+ ObjCMethod->getNameAsString() == "load") {
+ Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
----------------
Same comment as above!
================
Comment at: test/SemaObjC/unguarded-availability.m:334
+@implementation HasStaticInitializer1
++ (void)load {
+ func_10_11(); // expected-warning{{'func_10_11' is only available on macOS
10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to
silence this warning}}
----------------
Could you add a test that `+(void)load: (int)x;` still works? It should still
accept availability attributes, right?
Repository:
rC Clang
https://reviews.llvm.org/D45699
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
