[PATCH] D65526: [Clangd] Initial prototype version of ExtractFunction
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 2 07:06:38 PDT 2019
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:211
+ // Note that DRELocType is never OUTSIDECONTEXT.
+ if (DeclLocType + 1 != DRELocType)
> sammccall wrote:
> > this line seems very suspect, what are you trying to do and why?
> This just checks whether (DeclLocType, DRELocType) is either (PRETARGET, TARGET) or (TARGET, POSTTARGET). Could explicitly write it if needed.
Definitely needed, apart from the lack of clarity the current version has UB sometimes (`DeclLocType + 1` might not be a valid value)
But by "why" i mostly mean - this looks like the place for recording the location, not the place for deciding not to record the location because some algorithm later may not need it. Seems a lot simpler to record it unconditionally and worry about it later.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits