[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)
+    return;
SureYeaah wrote:
> 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



More information about the cfe-commits mailing list