[Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location

[erik.pilkington]

Summary

Previously, we used TypeLocs to find the correct SourceLocations to emit -Wunguarded-availability. Unfortunately, TypeLocs can't be trusted as they sometimes have an an empty SourceLocation component. This new patch maintains the enclosing SourceLocation to be used instead.
Thanks for taking a look,
Erik

[erik.pilkington]

Just noticed this can be simplified a bit, NFC compared to the last version of the diff.

[arphaman]

Hmm, I don't like how we end with a location that points to x instead of InterWithProp. Can we ignore the TypeLocs with invalid location and instead look at ObjCPropertyRefExprs with a class receiver?

[arphaman]

test/SemaObjC/unguarded-availability.m

141 ↗

(On Diff #99235)

Should this be @property(class)?

[erik.pilkington]

Can we ignore the TypeLocs with invalid location and instead look at ObjCPropertyRefExprs with a class receiver?

Sure, good idea. This new patch does exactly that.

Thanks,
Erik

[arphaman]

LGTM with one change below:

lib/Sema/SemaDeclAttr.cpp

7256 ↗

(On Diff #99605)

getClassReceiver calls get in a PointerUnion, which will trigger an assertion when the PRE is not a class receiver. Please rewrite this if to something like:

if (PRE->isClassReceiver())
  DiagnoseDeclAvailability(PRE->getClassReceiver(), PRE->getReceiverLocation());

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

public:
   }
 
   void IssueDiagnostics(Stmt *S) { TraverseStmt(S); }
 
   bool TraverseIfStmt(IfStmt *If);
 
   bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
 
+  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+    if (PRE->isClassReceiver())
+      DiagnoseDeclAvailability(PRE->getClassReceiver(), PRE->getReceiverLocation());
+    return true;
+  }
+
   bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
     if (ObjCMethodDecl *D = Msg->getMethodDecl())
       DiagnoseDeclAvailability(
           D, SourceRange(Msg->getSelectorStartLoc(), Msg->getLocEnd()));
     return true;
   }
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
if (AvailabilityResult Result =
     FixitDiag << FixItHint::CreateInsertion(ElseInsertionLoc, FixItOS.str());
   }
 }
 
 bool DiagnoseUnguardedAvailability::VisitTypeLoc(TypeLoc Ty) {
   const Type *TyPtr = Ty.getTypePtr();
   SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
 
+  if (Range.isInvalid())
+    return true;
+
   if (const TagType *TT = dyn_cast<TagType>(TyPtr)) {
     TagDecl *TD = TT->getDecl();
     DiagnoseDeclAvailability(TD, Range);
 
   } else if (const TypedefType *TD = dyn_cast<TypedefType>(TyPtr)) {
     TypedefNameDecl *D = TD->getDecl();
     DiagnoseDeclAvailability(D, Range);
 

cfe/trunk/test/SemaObjC/unguarded-availability.m

 void test_params2(int_10_12 x) AVAILABLE_10_12; // no warn
 
 void (^topLevelBlockDecl)() = ^ {
   func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
   if (@available(macos 10.12, *))
     func_10_12();
 };
 
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property(class) int x;
++ (void) setX: (int)newX AVAILABLE_10_12; // expected-note{{marked partial here}}
+@end
+void test_property(void) {
+  int y = InterWithProp.x; // expected-warning{{'InterWithProp' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+  InterWithProp.x = y; // expected-warning{{'InterWithProp' is only available on macOS 10.12 or newer}} expected-note{{@available}} expected-warning{{'setX:' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
+__attribute__((objc_root_class))
+@interface Subscriptable
+- (id)objectAtIndexedSubscript:(int)sub AVAILABLE_10_12; // expected-note{{marked partial here}}
+@end
+
+void test_at(Subscriptable *x) {
+  id y = x[42]; // expected-warning{{'objectAtIndexedSubscript:' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
 #ifdef OBJCPP
 
 int f(char) AVAILABLE_10_12;
 int f(int);
 
 template <class T> int use_f() {
   // FIXME: We should warn here!
   return f(T());