sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:136 tooling::CompileCommand getFallbackCommand(PathRef File) const override; + /// Note that project info is gathered purely from the inner compilation + /// database. This is to ensure consistency, as a translation unit with an ---------------- nit: drop "note that" for readability. ================ Comment at: clang-tools-extra/clangd/GlobalCompilationDatabase.h:137 + /// Note that project info is gathered purely from the inner compilation + /// database. This is to ensure consistency, as a translation unit with an + /// overriden command might yield a different project compared to headers ---------------- I find this comment confusing, you say "might" when I think you mean "might otherwise" (i.e. given the old behavior). However the claim is true anyway for another reason: headers *may* simply be from a different CDB. Maybe enough for the header is: // It wouldn't make much sense to treat files with overridden commands specially when // we can't do the same for the (unknown) local headers they include. Even so I think this comment might belong rather in the implementation file... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83934/new/ https://reviews.llvm.org/D83934 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
