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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65526/new/ https://reviews.llvm.org/D65526 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
