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

2017-05-22 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303562: [Sema][ObjC] Fix a bug where 
-Wunguarded-availability was emitted at the wrong… (authored by epilk).

Changed prior to commit:
  https://reviews.llvm.org/D33250?vs=99605=99772#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33250

Files:
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/SemaObjC/unguarded-availability.m


Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -7258,6 +7258,12 @@
 
   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(
@@ -7387,6 +7393,9 @@
   const Type *TyPtr = Ty.getTypePtr();
   SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
 
+  if (Range.isInvalid())
+return true;
+
   if (const TagType *TT = dyn_cast(TyPtr)) {
 TagDecl *TD = TT->getDecl();
 DiagnoseDeclAvailability(TD, Range);
Index: cfe/trunk/test/SemaObjC/unguarded-availability.m
===
--- cfe/trunk/test/SemaObjC/unguarded-availability.m
+++ cfe/trunk/test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,26 @@
 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;


Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -7258,6 +7258,12 @@
 
   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(
@@ -7387,6 +7393,9 @@
   const Type *TyPtr = Ty.getTypePtr();
   SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
 
+  if (Range.isInvalid())
+return true;
+
   if (const TagType *TT = dyn_cast(TyPtr)) {
 TagDecl *TD = TT->getDecl();
 DiagnoseDeclAvailability(TD, Range);
Index: cfe/trunk/test/SemaObjC/unguarded-availability.m
===
--- cfe/trunk/test/SemaObjC/unguarded-availability.m
+++ cfe/trunk/test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,26 @@
 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;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

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

2017-05-22 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM with one change below:




Comment at: lib/Sema/SemaDeclAttr.cpp:7256
+  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (ObjCInterfaceDecl *ID = PRE->getClassReceiver())
+  DiagnoseDeclAvailability(ID, PRE->getReceiverLocation());

`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());
```



https://reviews.llvm.org/D33250



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-05-19 Thread Erik Pilkington via Phabricator via cfe-commits 
erik.pilkington updated this revision to Diff 99605.
erik.pilkington added a comment.

> 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


https://reviews.llvm.org/D33250

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/SemaObjC/unguarded-availability.m


Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,26 @@
 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;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7252,6 +7252,12 @@
 
   bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
 
+  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (ObjCInterfaceDecl *ID = PRE->getClassReceiver())
+  DiagnoseDeclAvailability(ID, PRE->getReceiverLocation());
+return true;
+  }
+
   bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
 if (ObjCMethodDecl *D = Msg->getMethodDecl())
   DiagnoseDeclAvailability(
@@ -7381,6 +7387,9 @@
   const Type *TyPtr = Ty.getTypePtr();
   SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
 
+  if (Range.isInvalid())
+return true;
+
   if (const TagType *TT = dyn_cast(TyPtr)) {
 TagDecl *TD = TT->getDecl();
 DiagnoseDeclAvailability(TD, Range);


Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,26 @@
 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;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7252,6 +7252,12 @@
 
   bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
 
+  bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (ObjCInterfaceDecl *ID = PRE->getClassReceiver())
+  DiagnoseDeclAvailability(ID, PRE->getReceiverLocation());
+return true;
+  }
+
   bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
 if (ObjCMethodDecl *D = Msg->getMethodDecl())
   DiagnoseDeclAvailability(
@@ -7381,6 +7387,9 @@
   const Type *TyPtr = Ty.getTypePtr();
   SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
 
+  if (Range.isInvalid())
+return true;
+
   if (const TagType *TT = dyn_cast(TyPtr)) {
 TagDecl *TD = TT->getDecl();
 DiagnoseDeclAvailability(TD, Range);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-05-17 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added inline comments.



Comment at: test/SemaObjC/unguarded-availability.m:141
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property int x;
+@end

Should this be `@property(class)`?


https://reviews.llvm.org/D33250



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-05-17 Thread Alex Lorenz via Phabricator via cfe-commits 
arphaman added a comment.

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 `ObjCPropertyRefExpr`s with a class receiver?


https://reviews.llvm.org/D33250



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-05-16 Thread Erik Pilkington via Phabricator via cfe-commits 
erik.pilkington updated this revision to Diff 99235.
erik.pilkington added a comment.

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


https://reviews.llvm.org/D33250

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/SemaObjC/unguarded-availability.m


Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,17 @@
 func_10_12();
 };
 
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property int x;
+@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}}
+}
+
 #ifdef OBJCPP
 
 int f(char) AVAILABLE_10_12;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7370,7 +7370,8 @@
 
 bool DiagnoseUnguardedAvailability::VisitTypeLoc(TypeLoc Ty) {
   const Type *TyPtr = Ty.getTypePtr();
-  SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+  SourceRange Range(StmtStack.back()->getLocStart(),
+StmtStack.back()->getLocEnd());
 
   if (const TagType *TT = dyn_cast(TyPtr)) {
 TagDecl *TD = TT->getDecl();


Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,17 @@
 func_10_12();
 };
 
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property int x;
+@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}}
+}
+
 #ifdef OBJCPP
 
 int f(char) AVAILABLE_10_12;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7370,7 +7370,8 @@
 
 bool DiagnoseUnguardedAvailability::VisitTypeLoc(TypeLoc Ty) {
   const Type *TyPtr = Ty.getTypePtr();
-  SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+  SourceRange Range(StmtStack.back()->getLocStart(),
+StmtStack.back()->getLocEnd());
 
   if (const TagType *TT = dyn_cast(TyPtr)) {
 TagDecl *TD = TT->getDecl();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-05-16 Thread Erik Pilkington via Phabricator via cfe-commits 
erik.pilkington created this revision.

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


https://reviews.llvm.org/D33250

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/SemaObjC/unguarded-availability.m


Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,17 @@
 func_10_12();
 };
 
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property int x;
+@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}}
+}
+
 #ifdef OBJCPP
 
 int f(char) AVAILABLE_10_12;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7221,6 +7221,8 @@
   SmallVector AvailabilityStack;
   SmallVector StmtStack;
 
+  SourceRange CurrentRange;
+
   void DiagnoseDeclAvailability(NamedDecl *D, SourceRange Range);
 
 public:
@@ -7234,7 +7236,10 @@
 if (!S)
   return true;
 StmtStack.push_back(S);
+SourceRange LastRange(S->getLocStart(), S->getLocEnd());
+std::swap(LastRange, CurrentRange);
 bool Result = Base::TraverseStmt(S);
+std::swap(LastRange, CurrentRange);
 StmtStack.pop_back();
 return Result;
   }
@@ -7370,21 +7375,18 @@
 
 bool DiagnoseUnguardedAvailability::VisitTypeLoc(TypeLoc Ty) {
   const Type *TyPtr = Ty.getTypePtr();
-  SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
-
   if (const TagType *TT = dyn_cast(TyPtr)) {
 TagDecl *TD = TT->getDecl();
-DiagnoseDeclAvailability(TD, Range);
+DiagnoseDeclAvailability(TD, CurrentRange);
 
   } else if (const TypedefType *TD = dyn_cast(TyPtr)) {
 TypedefNameDecl *D = TD->getDecl();
-DiagnoseDeclAvailability(D, Range);
+DiagnoseDeclAvailability(D, CurrentRange);
 
   } else if (const auto *ObjCO = dyn_cast(TyPtr)) {
 if (NamedDecl *D = ObjCO->getInterface())
-  DiagnoseDeclAvailability(D, Range);
+  DiagnoseDeclAvailability(D, CurrentRange);
   }
-
   return true;
 }
 


Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,17 @@
 func_10_12();
 };
 
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property int x;
+@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}}
+}
+
 #ifdef OBJCPP
 
 int f(char) AVAILABLE_10_12;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7221,6 +7221,8 @@
   SmallVector AvailabilityStack;
   SmallVector StmtStack;
 
+  SourceRange CurrentRange;
+
   void DiagnoseDeclAvailability(NamedDecl *D, SourceRange Range);
 
 public:
@@ -7234,7 +7236,10 @@
 if (!S)
   return true;
 StmtStack.push_back(S);
+SourceRange LastRange(S->getLocStart(), S->getLocEnd());
+std::swap(LastRange, CurrentRange);
 bool Result = Base::TraverseStmt(S);
+std::swap(LastRange, CurrentRange);
 StmtStack.pop_back();
 return Result;
   }
@@ -7370,21 +7375,18 @@
 
 bool DiagnoseUnguardedAvailability::VisitTypeLoc(TypeLoc Ty) {
   const Type *TyPtr = Ty.getTypePtr();
-  SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
-
   if (const TagType *TT = dyn_cast(TyPtr)) {
 TagDecl *TD = TT->getDecl();
-DiagnoseDeclAvailability(TD, Range);
+DiagnoseDeclAvailability(TD, CurrentRange);
 
   } else if (const TypedefType *TD = dyn_cast(TyPtr)) {
 TypedefNameDecl *D = TD->getDecl();
-DiagnoseDeclAvailability(D, Range);
+DiagnoseDeclAvailability(D, CurrentRange);
 
   } else if (const auto *ObjCO = dyn_cast(TyPtr)) {
 if (NamedDecl *D = ObjCO->getInterface())
-  DiagnoseDeclAvailability(D, Range);
+  DiagnoseDeclAvailability(D, CurrentRange);
   }
-
   return true;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org