sammccall added a comment. Sorry, forgot to address the style comments. Will send a followup patch.
================ Comment at: clang-tools-extra/clangd/Preamble.cpp:87 + + const SourceManager &SM = CI.getSourceManager(); + const FileEntry *MainFE = SM.getFileEntryForID(SM.getMainFileID()); ---------------- kadircet wrote: > nit: maybe do this at the top and keep the early exit? that makes sense if ParsedCallback is the "main line" case that we're bailing out of (which is how it was written before), but I don't really see it that way. ================ Comment at: clang-tools-extra/clangd/Preamble.cpp:400 + std::move(StatCache), CapturedInfo.takeCanonicalIncludes()); + Result->MainIsIncludeGuarded = CapturedInfo.isMainFileIncludeGuarded(); + return Result; ---------------- kadircet wrote: > any reason for not making this part of the constructor ? I think this pattern of constructor (long list of args, forwards them into public fields) isn't terribly useful, but I should have been consistent here. Will send a cleanup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106203/new/ https://reviews.llvm.org/D106203 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
