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
