[PATCH] D68108: Redeclare Objective-C property accessors inside the ObjCImplDecl in which they are synthesized.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/include/clang/AST/DeclObjC.h:2787
+  /// interface as a decl context.
+  ObjCMethodDecl *SetterMethodDecl = nullptr;
+
----------------
Are these comments right?  The DC is the implementation's interface?

Regardless, I think these comments are misleading: the right way to put this is that these are the getter and setter method *definitions* (even if they don't necessarily have a body).  And, relatedly, you should mention whether these are set even if the methods are defined explicitly or only if they're synthesized.


================
Comment at: clang/include/clang/Sema/Sema.h:8522
+  /// implementation, but only if they haven't been overridden by an
+  /// explicit definition.
+  llvm::DenseMap<ObjCContainerDecl *,
----------------
Property accessors are created in the `@implementation` if they aren't defined explicitly there.

Does this really need to be global state on Sema?  Does it theoretically need to be serialized and restored if e.g. a translation-unit prefix is cut after an `@implementation` is seen?


================
Comment at: clang/lib/AST/DeclObjC.cpp:1381
+      }
+    }
+
----------------
Can you just do this as a simple check to look through the `ObjCImplDecl` to the `ObjCInterfaceDecl` as the first thing, immediately after you initialize `Container`?


================
Comment at: clang/lib/Analysis/BodyFarm.cpp:843
+    return Val.getValue();
+  Val = nullptr;
+
----------------
Why did this logic need to change?


================
Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:1887
+        addIfExists(propImpl->getGetterMethodDecl());
+        addIfExists(propImpl->getSetterMethodDecl());
       }
----------------
This is only correct if these methods aren't defined explicitly by the user, right?  Or do this methods return null in that case?

Same question goes for the CGObjCMac code.


================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:6698
     emitMethodConstant(methodArray, MD, forProtocol);
-  }
   methodArray.finishAndAddTo(values);
 
----------------
Spurious change.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:639
+void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
                                     llvm::Function *Fn,
                                     const CGFunctionInfo &FnInfo,
----------------
Spurious change.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5063
                                  const_cast<ObjCImplementationDecl *>(D), PID);
     }
   }
----------------
Is this special treatment still necessary?  Won't we encounter the getter and setter on the normal pass over the method definitions in the `@implementation`?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68108/new/

https://reviews.llvm.org/D68108