[PATCH] D25283: AvailabilityAttrs: Refactor context checking when diagnosing an availability violation

[Aaron Ballman via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added inline comments.


================
Comment at: include/clang/Sema/Sema.h:9747
+  AvailabilityResult ShouldDiagnoseAvailabilityOfDecl(NamedDecl *&D,
+                                                      std::string *Message);
 
----------------
Since you're altering the doxygen comments, it would be nice to add a comment for `Message`.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:6346
+                                                Decl *Ctx) {
+  assert(K != AR_Available);
+
----------------
Please add an explanatory string literal to the condition.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:6350
+    if (K == AR_NotYetIntroduced) {
+      if (auto *AA = getAttrForPlatform(S.Context, C))
+        if (AA->getIntroduced() >= DeclVersion)
----------------
Please do not use `auto` here; the type is not spelled out in the initialization.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:6368
+    if (const auto *CatOrImpl = dyn_cast<ObjCImplDecl>(Ctx))
+      if (const auto *Interface = CatOrImpl->getClassInterface())
+        if (IsContextGreater(Interface))
----------------
Same comment here as well.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:6372
+    // A category implicitly has the availability of the interface.
+    if (const ObjCCategoryDecl *CatD = dyn_cast<ObjCCategoryDecl>(Ctx))
+      if (const ObjCInterfaceDecl *Interface = CatD->getClassInterface())
----------------
You can use `auto` here though. :-)


================
Comment at: lib/Sema/SemaDeclAttr.cpp:6398
+  VersionTuple DeclVersion;
+  if (auto *AA = getAttrForPlatform(S.Context, D))
+    DeclVersion = AA->getIntroduced();
----------------
Please do not use `auto` here; the type is not spelled out in the initialization.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:6699
+
+    // If the context of this function is more unavailable then D, we should not
+    // emit a diagnostic.
----------------
s/then/than


https://reviews.llvm.org/D25283