Get class property selectors from property decl if it exists

[herzka]

Summary

Before this fix, trying to get or set a class property using dot syntax would always use the constructed name (x or setX:), which might not match the real selector if the getter or setter is specified via property attributes. Now, the selectors in the declaration have priority over the constructed names, which is consistent with instance properties.

[herzka]

This issue applies to getters too. I'll make this fix broader.

I think the real fix for both is to construct the ObjCPropertyRefExpr using the ObjCPropertyDecl so that it's considered an explicit property. Currently, it still gets considered implicit, which is not true. I don't know what implications changing that may have, though. This fix seems safer.

[arphaman]

Can you please post the patch with full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)?

[herzka]

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.

[compnerd]

Please add a test for this.

lib/Sema/SemaExprObjC.cpp

1989

Use auto.

[herzka]

Added test, used auto

[herzka]

Rename selectors

[compnerd]

I think Im misunderstanding something. How does the test actually test what you are changing?

[herzka]

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.

[compnerd]

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?

[herzka]

@compnerd, yes please. I don't have commit access. Thanks for the review!

[compnerd]

SVN r295683

lib/Sema/SemaExprObjC.cpp

   }
 
   // Look for the matching setter, in case it is needed.
-  Selector SetterSel =
-    SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-                                            PP.getSelectorTable(),
-                                           &propertyName);
+  Selector SetterSel;
+  if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+          &propertyName, ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+    SetterSel = PD->getSetterName();
+  } else {
+    SetterSel =
+      SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+                                             PP.getSelectorTable(),
+                                             &propertyName);
+  }
 
   ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
   if (!Setter) {