sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land.
Thanks, and sorry for the long delay. ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:282 + Anchor::Above}}; + llvm::SmallVector<std::pair<std::string, Edit>> Edits; + ---------------- Not sure I see the point of using a SmallVector here instead of writing directly into Effect.ApplyEdits? ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:295 + + if (Impl) { + auto ImplReplacement = insertDecl( ---------------- Maybe a comment like: ``` // If we see the class implementation, add the initializer there too. // FIXME: merging the edits is awkward, do this elsewhere ``` (I don't think we should fix it in this patch, but we should at some point) ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:301 + + if (!SM.isWrittenInSameFile(Interface->getLocation(), + Impl->getLocation())) { ---------------- nit: I'd invert the condition here, to avoid the "else" block being a double-negative Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116385/new/ https://reviews.llvm.org/D116385 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits