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;
----------------
sammccall wrote:
> SureYeaah wrote:
> > Nit: same parameter order for replaceWithVar and insertDeclaration.
> I think this *is* the same parameter order semantically: this is roughly
> (thing, definition) in both cases.
> The fact that the *types* match but in the opposite order is just a
> coincidence I think?
Sorry yes, makes sense.
================
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.
----------------
sammccall wrote:
> SureYeaah wrote:
> > 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.
> >
> >
> I'm not quite sure what you mean here.
>
> Annotating the operators, for `1 +a [[2 +b 3 *c 4]] +d 5`:
> - N is `*c`
> - RHS is `4`
> - LHS is `1 +a 2 +b 3 *c 4`.
>
> The point is that RHS can never be an operator of the same type, because if
> we replaced RHS with `x +e y` then N would be that `+e` instead (since `+` is
> left-associative).
>
But isn't the tree something like
+
/ \
+ 5
/ \
+ *
/ \ / \
1 2 3 4
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
