[PATCH] D65139: [clangd] Support extraction of binary "subexpressions" like a + [[b + c]].

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 25 01:11:24 PDT 2019


sammccall marked 2 inline comments as done.
sammccall added a comment.

This is ready for another round, I think.



================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:393
   std::string VarName = "dummy";
+  SourceRange Range = Target->getExtractionChars();
   // insert new variable declaration
----------------
SureYeaah wrote:
> Why not just compute this inside ExtractionContext?
Using ExtractionContext to cache values that are used in several places but not everywhere seems confusing and to obfuscate the data flow.

In particular, when is it going to be initialized? Not in the constructor, because we don't want to do this during compare. On first access? That seems much less clear than explicitly computing it and passing it around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65139





More information about the cfe-commits mailing list