[PATCH] D29967: Get class property selectors from property decl if it exists
compnerd closed this revision. compnerd added a comment. SVN r295683 https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29967: Get class property selectors from property decl if it exists
herzka added a comment. @compnerd, yes please. I don't have commit access. Thanks for the review! https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29967: Get class property selectors from property decl if it exists
compnerd accepted this revision. compnerd added a comment. This revision is now accepted and ready to land. Ah, I had missed the `-verify` option on the test. Yes, that makes sense. Ternary may have flowed the conditional code better. Do you need someone to commit this on your behalf? https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29967: Get class property selectors from property decl if it exists
herzka added a comment. Currently, `A.customGetterProperty` would be turned into `[A customGetterProperty]`, which would fail to compile because that selector isn't declared anywhere. With my fix, it does compile because it (correctly) resolves to `[A customGet]`, which does exist. Similarly, `a.customSetterProperty = 1` got turned into `[a setCustomSetterProperty:1]` (another nonexistent selector), but now becomes `[a customSet:1]`. Here are the errors I get when I run the test without my fix: error: 'error' diagnostics seen but not expected: File /Users/herzka/dev/OSS/llvm/tools/clang/test/SemaObjC/objc-class-property.m Line 45: no setter method 'setCustomSetterProperty:' for assignment to property File /Users/herzka/dev/OSS/llvm/tools/clang/test/SemaObjC/objc-class-property.m Line 46: no getter method for read from property 2 errors generated. https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29967: Get class property selectors from property decl if it exists
compnerd added a comment. I think Im misunderstanding something. How does the test actually test what you are changing? https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29967: Get class property selectors from property decl if it exists
herzka updated this revision to Diff 88652.
herzka added a comment.
Rename selectors
https://reviews.llvm.org/D29967
Files:
lib/Sema/SemaExprObjC.cpp
test/SemaObjC/objc-class-property.m
Index: test/SemaObjC/objc-class-property.m
===
--- test/SemaObjC/objc-class-property.m
+++ test/SemaObjC/objc-class-property.m
@@ -21,22 +21,31 @@
@property (class) int c2; // expected-note {{property declared here}} \
// expected-note {{property declared here}}
@property (class) int x;
+@property (class, setter=customSet:) int customSetterProperty;
+@property (class, getter=customGet) int customGetterProperty;
@end
@implementation A // expected-warning {{class property 'c2' requires method
'c2' to be defined}} \
// expected-warning {{class property 'c2' requires method
'setC2:' to be defined}}
@dynamic x; // refers to the instance property
@dynamic (class) x; // refers to the class property
@synthesize z, c2; // expected-error {{@synthesize not allowed on a class
property 'c2'}}
@dynamic c; // refers to the class property
+@dynamic customSetterProperty;
+@dynamic customGetterProperty;
@end
int test() {
A *a = [[A alloc] init];
a.c; // expected-error {{property 'c' is a class property; did you mean to
access it with class 'A'}}
return a.x + A.c;
}
+void customSelectors() {
+ A.customSetterProperty = 1;
+ (void)A.customGetterProperty;
+}
+
void message_id(id me) {
[me y];
}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
}
}
+ Selector GetterSel;
+ Selector SetterSel;
+ if (auto PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+ } else {
+GetterSel = PP.getSelectorTable().getNullarySelector();
+SetterSel =
+ SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ );
+ }
+
// Search for a declared property first.
- Selector Sel = PP.getSelectorTable().getNullarySelector();
- ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+ ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
// If this reference is in an @implementation, check for 'private' methods.
if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
if (Getter) {
// FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2013,6 @@
}
// Look for the matching setter, in case it is needed.
- Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
- );
-
ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
if (!Setter) {
// If this reference is in an @implementation, also check for 'private'
Index: test/SemaObjC/objc-class-property.m
===
--- test/SemaObjC/objc-class-property.m
+++ test/SemaObjC/objc-class-property.m
@@ -21,22 +21,31 @@
@property (class) int c2; // expected-note {{property declared here}} \
// expected-note {{property declared here}}
@property (class) int x;
+@property (class, setter=customSet:) int customSetterProperty;
+@property (class, getter=customGet) int customGetterProperty;
@end
@implementation A // expected-warning {{class property 'c2' requires method 'c2' to be defined}} \
// expected-warning {{class property 'c2' requires method 'setC2:' to be defined}}
@dynamic x; // refers to the instance property
@dynamic (class) x; // refers to the class property
@synthesize z, c2; // expected-error {{@synthesize not allowed on a class property 'c2'}}
@dynamic c; // refers to the class property
+@dynamic customSetterProperty;
+@dynamic customGetterProperty;
@end
int test() {
A *a = [[A alloc] init];
a.c; // expected-error {{property 'c' is a class property; did you mean to access it with class 'A'}}
return a.x + A.c;
}
+void customSelectors() {
+ A.customSetterProperty = 1;
+ (void)A.customGetterProperty;
+}
+
void message_id(id me) {
[me y];
}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
}
}
+ Selector GetterSel;
+ Selector SetterSel;
+ if (auto PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+ }
[PATCH] D29967: Get class property selectors from property decl if it exists
herzka updated this revision to Diff 88650.
herzka added a comment.
Added test, used auto
https://reviews.llvm.org/D29967
Files:
lib/Sema/SemaExprObjC.cpp
test/SemaObjC/objc-class-property.m
Index: test/SemaObjC/objc-class-property.m
===
--- test/SemaObjC/objc-class-property.m
+++ test/SemaObjC/objc-class-property.m
@@ -21,22 +21,31 @@
@property (class) int c2; // expected-note {{property declared here}} \
// expected-note {{property declared here}}
@property (class) int x;
+@property (class, setter=customSetA:) int customSetterProperty;
+@property (class, getter=customGetB) int customGetterProperty;
@end
@implementation A // expected-warning {{class property 'c2' requires method
'c2' to be defined}} \
// expected-warning {{class property 'c2' requires method
'setC2:' to be defined}}
@dynamic x; // refers to the instance property
@dynamic (class) x; // refers to the class property
@synthesize z, c2; // expected-error {{@synthesize not allowed on a class
property 'c2'}}
@dynamic c; // refers to the class property
+@dynamic customSetterProperty;
+@dynamic customGetterProperty;
@end
int test() {
A *a = [[A alloc] init];
a.c; // expected-error {{property 'c' is a class property; did you mean to
access it with class 'A'}}
return a.x + A.c;
}
+void customSelectors() {
+ A.customSetterProperty = 1;
+ (void)A.customGetterProperty;
+}
+
void message_id(id me) {
[me y];
}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
}
}
+ Selector GetterSel;
+ Selector SetterSel;
+ if (auto PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+ } else {
+GetterSel = PP.getSelectorTable().getNullarySelector();
+SetterSel =
+ SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ );
+ }
+
// Search for a declared property first.
- Selector Sel = PP.getSelectorTable().getNullarySelector();
- ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+ ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
// If this reference is in an @implementation, check for 'private' methods.
if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
if (Getter) {
// FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2013,6 @@
}
// Look for the matching setter, in case it is needed.
- Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
- );
-
ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
if (!Setter) {
// If this reference is in an @implementation, also check for 'private'
Index: test/SemaObjC/objc-class-property.m
===
--- test/SemaObjC/objc-class-property.m
+++ test/SemaObjC/objc-class-property.m
@@ -21,22 +21,31 @@
@property (class) int c2; // expected-note {{property declared here}} \
// expected-note {{property declared here}}
@property (class) int x;
+@property (class, setter=customSetA:) int customSetterProperty;
+@property (class, getter=customGetB) int customGetterProperty;
@end
@implementation A // expected-warning {{class property 'c2' requires method 'c2' to be defined}} \
// expected-warning {{class property 'c2' requires method 'setC2:' to be defined}}
@dynamic x; // refers to the instance property
@dynamic (class) x; // refers to the class property
@synthesize z, c2; // expected-error {{@synthesize not allowed on a class property 'c2'}}
@dynamic c; // refers to the class property
+@dynamic customSetterProperty;
+@dynamic customGetterProperty;
@end
int test() {
A *a = [[A alloc] init];
a.c; // expected-error {{property 'c' is a class property; did you mean to access it with class 'A'}}
return a.x + A.c;
}
+void customSelectors() {
+ A.customSetterProperty = 1;
+ (void)A.customGetterProperty;
+}
+
void message_id(id me) {
[me y];
}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
}
}
+ Selector GetterSel;
+ Selector SetterSel;
+ if (auto PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel =
[PATCH] D29967: Get class property selectors from property decl if it exists
compnerd requested changes to this revision.
compnerd added a comment.
This revision now requires changes to proceed.
Please add a test for this.
Comment at: lib/Sema/SemaExprObjC.cpp:1989
+ Selector SetterSel;
+ if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
Use `auto`.
https://reviews.llvm.org/D29967
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29967: Get class property selectors from property decl if it exists
herzka updated this revision to Diff 88519.
herzka edited the summary of this revision.
herzka added a comment.
Added full context. Sorry about that! This is my first contribution, and I
don't know how I completely missed that line while going through the steps.
https://reviews.llvm.org/D29967
Files:
lib/Sema/SemaExprObjC.cpp
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
}
}
+ Selector GetterSel;
+ Selector SetterSel;
+ if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+ } else {
+GetterSel = PP.getSelectorTable().getNullarySelector();
+SetterSel =
+ SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ );
+ }
+
// Search for a declared property first.
- Selector Sel = PP.getSelectorTable().getNullarySelector();
- ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+ ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
// If this reference is in an @implementation, check for 'private' methods.
if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
if (Getter) {
// FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2013,6 @@
}
// Look for the matching setter, in case it is needed.
- Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
- );
-
ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
if (!Setter) {
// If this reference is in an @implementation, also check for 'private'
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
}
}
+ Selector GetterSel;
+ Selector SetterSel;
+ if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+ } else {
+GetterSel = PP.getSelectorTable().getNullarySelector();
+SetterSel =
+ SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ );
+ }
+
// Search for a declared property first.
- Selector Sel = PP.getSelectorTable().getNullarySelector();
- ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+ ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
// If this reference is in an @implementation, check for 'private' methods.
if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
if (Getter) {
// FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2013,6 @@
}
// Look for the matching setter, in case it is needed.
- Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
- );
-
ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
if (!Setter) {
// If this reference is in an @implementation, also check for 'private'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29967: Get class property selectors from property decl if it exists
arphaman added a comment. Can you please post the patch with full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)? https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D29967: Get class property selectors from property decl if it exists
herzka updated this revision to Diff 88448.
herzka retitled this revision from "Get class property setter selector from
property decl if exists" to "Get class property selectors from property decl if
it exists".
herzka edited the summary of this revision.
https://reviews.llvm.org/D29967
Files:
lib/Sema/SemaExprObjC.cpp
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
}
}
+ Selector GetterSel;
+ Selector SetterSel;
+ if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+ } else {
+GetterSel = PP.getSelectorTable().getNullarySelector();
+SetterSel =
+ SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ );
+ }
+
// Search for a declared property first.
- Selector Sel = PP.getSelectorTable().getNullarySelector();
- ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+ ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
// If this reference is in an @implementation, check for 'private' methods.
if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
if (Getter) {
// FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2013,6 @@
}
// Look for the matching setter, in case it is needed.
- Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
- );
-
ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
if (!Setter) {
// If this reference is in an @implementation, also check for 'private'
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
}
}
+ Selector GetterSel;
+ Selector SetterSel;
+ if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+ , ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+ } else {
+GetterSel = PP.getSelectorTable().getNullarySelector();
+SetterSel =
+ SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ );
+ }
+
// Search for a declared property first.
- Selector Sel = PP.getSelectorTable().getNullarySelector();
- ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+ ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
// If this reference is in an @implementation, check for 'private' methods.
if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
if (Getter) {
// FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2013,6 @@
}
// Look for the matching setter, in case it is needed.
- Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
- );
-
ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
if (!Setter) {
// If this reference is in an @implementation, also check for 'private'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
