kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land.
Thanks, LGTM! In D116502#3217270 <https://reviews.llvm.org/D116502#3217270>, @sammccall wrote: > In D116502#3217084 <https://reviews.llvm.org/D116502#3217084>, @kadircet > wrote: > >> We have some logic in AddUsing tweak to determine insertion point based on >> AST. i think it makes sense to migrate it to these helpers too. > > Thanks, I'd forgotten about those. I might tackle those next? SGTM. >> We've got some helpers in SourceCode.h to determine insertion point in the >> absence of ASTs. Concepts here and there around an "insertion point" seems >> to be quite different (it's just a sourcelocation here and a set of >> locations + a namespace in SourceCode.h). >> I suppose those two are somewhat hard to merge and serve different purposes, >> so It's better to keep them separate. > > For sure ast-based and pseudoparsing-based cases are going to be different > APIs, but I would like to move those into this header too. Makes sense. > The problem I hit was they share private infrastructure (in SourceCode.cpp) > with other functionality (the namespace pseudoparsing for completion, I > think). So it's a bit of work to extract. Ah I see :/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116502/new/ https://reviews.llvm.org/D116502 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
