kadircet accepted this revision. kadircet added a comment. This revision is now accepted and ready to land.
thanks, lgtm! ================ Comment at: clang-tools-extra/clangd/ParsedAST.cpp:441 + if (Preamble) + Marks = Preamble->Marks; + Clang->getPreprocessor().addPPCallbacks( ---------------- dgoldman wrote: > kadircet wrote: > > We are not patching marks for stale preambles, which I believe is fine but > > worth a FIXME. If you are interested please take a look at > > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.cpp#L427, > > it should be fairly easy to handle movements of `#pragma mark`s in > > preamble section. > Added a FIXME. Does something similar need to be done for MainFileMacros? we already handle macros today in https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.cpp#L208. we just need to handle pragma mark in the callbacks and generate the relevant patch. ================ Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:1100 + Children(AllOf(WithName("bar"), + Children(AllOf(WithName( + "NotTopDecl")))))))), ---------------- nit: drop `AllOf` ================ Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:1102 + "NotTopDecl")))))))), + AllOf(WithName("bar")))))); +} ---------------- nit: drop `AllOf` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105904/new/ https://reviews.llvm.org/D105904 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
