VitaNuo marked an inline comment as done. VitaNuo added a comment. Respond to comments.
================ Comment at: clang-tools-extra/clangd/refactor/tweaks/OrganizeImports.cpp:56 + return false; + Range PreambleRange; + PreambleRange.start = ---------------- kadircet wrote: > VitaNuo wrote: > > kadircet wrote: > > > kadircet wrote: > > > > relying on `MainFileIncludes` for preamble range is not correct in > > > > general. we can have includes way down inside the main file that'll be > > > > part of this vector, but not preamble (also it's not sorted explicitly). > > > > > > > > we should instead use `ComputePreambleBounds` (nit: we can also store > > > > it in ParsedAST, instead of relexing the file one more time with each > > > > CodeAction request) > > > Having a comment for reason would also be helpful, something like `To > > > accommodate clients without knowledge of source actions, we trigger > > > without checking code action kinds when inside the preamble region`. > > > nit: we can also store it in ParsedAST > > > > Seems like the data is already there, I just need to expose it. > > > nit: we can also store it in ParsedAST > > Seems like the data is already there, I just need to expose it. > > not really, we calculate bounds when building a `ParsedAST` but we don't > store it anywhere. I was referring to https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/PrecompiledPreamble.cpp#L580. We store the `PreambleData` and that seems to store the `PrecompiledPreamble` which seems to know its bounds Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153769/new/ https://reviews.llvm.org/D153769 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
