[clang] e421a74 - [ASTImporter] Fix import of ObjCPropertyDecl that share the same name

[Raphael Isemann via cfe-commits]

Author: Raphael Isemann
Date: 2021-03-22T18:05:50+01:00
New Revision: e421a74108ee86afec133c77258470d3ed7dcc90

URL: https://github.com/llvm/llvm-project/commit/e421a74108ee86afec133c77258470d3ed7dcc90
DIFF: https://github.com/llvm/llvm-project/commit/e421a74108ee86afec133c77258470d3ed7dcc90.diff

LOG: [ASTImporter] Fix import of ObjCPropertyDecl that share the same name

Objective-C apparently allows name conflicts between instance and class
properties, so this is valid code:

```
@protocol DupProp
@property (class, readonly) int prop;
@property (readonly) int prop;
@end
```

The ASTImporter however isn't aware of this and will consider the two properties
as if they are the same property because it just compares their name and types.
This causes that when importing both properties we only end up with one property
(whatever is imported first from what I can see).

Beside generating a different AST this also leads to a bunch of asserts and
crashes as we still correctly import the two different getters for both
properties (the import code for methods does the correct check where it
differentiated between instance and class methods). As one of the setters will
not have its associated ObjCPropertyDecl imported, any call to
`ObjCMethodDecl::findPropertyDecl` will just lead to an assert or crash.

Fixes rdar://74322659

Reviewed By: shafik, kastiglione

Differential Revision: https://reviews.llvm.org/D99077

Added: 
    

Modified: 
    clang/lib/AST/ASTImporter.cpp
    clang/unittests/AST/ASTImporterTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 182a57c16abaf..f9b1910552ee0 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5066,6 +5066,11 @@ ExpectedDecl ASTNodeImporter::VisitObjCPropertyDecl(ObjCPropertyDecl *D) {
   auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
   for (auto *FoundDecl : FoundDecls) {
     if (auto *FoundProp = dyn_cast<ObjCPropertyDecl>(FoundDecl)) {
+      // Instance and class properties can share the same name but are 
diff erent
+      // declarations.
+      if (FoundProp->isInstanceProperty() != D->isInstanceProperty())
+        continue;
+
       // Check property types.
       if (!Importer.IsStructurallyEquivalent(D->getType(),
                                              FoundProp->getType())) {

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 8c4b982ec6d50..9458fc2265800 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -5639,6 +5639,35 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImplicitlyDeclareSelf) {
   EXPECT_TRUE(ToMethod->getSelfDecl() != nullptr);
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ObjPropertyNameConflict) {
+  // Tests that properties that share the same name are correctly imported.
+  // This is only possible with one instance and one class property.
+  Decl *FromTU = getTuDecl(R"(
+                           @interface DupProp{}
+                           @property (class) int prop;
+                           @property int prop;
+                           @end
+                           )",
+                           Lang_OBJCXX, "input.mm");
+  auto *FromClass = FirstDeclMatcher<ObjCInterfaceDecl>().match(
+      FromTU, namedDecl(hasName("DupProp")));
+  auto ToClass = Import(FromClass, Lang_OBJCXX);
+  ASSERT_TRUE(ToClass);
+  // We should have one class and one instance property.
+  ASSERT_EQ(
+      1, std::distance(ToClass->classprop_begin(), ToClass->classprop_end()));
+  ASSERT_EQ(1,
+            std::distance(ToClass->instprop_begin(), ToClass->instprop_end()));
+  for (clang::ObjCPropertyDecl *prop : ToClass->properties()) {
+    // All properties should have a getter and a setter.
+    ASSERT_TRUE(prop->getGetterMethodDecl());
+    ASSERT_TRUE(prop->getSetterMethodDecl());
+    // The getters/setters should be able to find the right associated property.
+    ASSERT_EQ(prop->getGetterMethodDecl()->findPropertyDecl(), prop);
+    ASSERT_EQ(prop->getSetterMethodDecl()->findPropertyDecl(), prop);
+  }
+}
+
 struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ImportAutoFunctions, ReturnWithTypedefDeclaredInside) {