SureYeaah added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:50
// Generate Replacement for declaring the selected Expr as a new variable
- tooling::Replacement insertDeclaration(llvm::StringRef VarName) const;
+ tooling::Replacement insertDeclaration(llvm::StringRef VarName,
+ SourceRange InitChars) const;
----------------
Nit: same parameter order for replaceWithVar and insertDeclaration.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:245
+ assert(E && "callee and args should be Exprs!");
+ if (E == Op->getArg(0) || E == Op->getArg(1))
+ SelectedOperands.push_back(Child);
----------------
Why do we need to check this for CXXOperatorCallExpr and not BinaryOperator?
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:305
+ const SelectionTree::Node *Start = Op.SelectedOperands.front(); // LHS
+ const SelectionTree::Node *End = Op.SelectedOperands.back(); // RHS
+ // End is already correct, Start needs to be pushed down to the right spot.
----------------
For 1 + [[2 + 3 * 4]] + 5, we probably don't get an invalid range from this
function. If so, we might want to add one more check where we parse End and
compare it's operator.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:314
+ }
+ // Op.LHS (partially) selected, so descend into it.
+ Start = Op.SelectedOperands.front();
----------------
Nit: For [[a + b + c]],
+
/ \
+ c
/ \
a b
For the second + as Op, a would be completely selected. So Op.LHS can be
partially or completely selected.
================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:448
+
+ // Binary subexpressions
+ {R"cpp(void f() {
----------------
Can we have some tests with macros as well?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65139/new/
https://reviews.llvm.org/D65139
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
