kadircet added a comment.
mostly LG, a few last nits.
================
Comment at: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp:51
+ // FIXME: we should rather propagate the current directory into
+ // ExpandResponseFiles as well in addition to FS and someone can take a
+ // look at it later on.
----------------
nit: drop `and someone can take a look at it later on.`
================
Comment at: clang/unittests/Tooling/CompilationDatabaseTest.cpp:865
+ void SetUp() override {
+ FS = new llvm::vfs::InMemoryFileSystem;
+ ASSERT_TRUE(addFile(path(StringRef("rsp1.rsp")), "-Dflag"));
----------------
make this part of the constructor instead of having a SetUp
================
Comment at: clang/unittests/Tooling/CompilationDatabaseTest.cpp:866
+ FS = new llvm::vfs::InMemoryFileSystem;
+ ASSERT_TRUE(addFile(path(StringRef("rsp1.rsp")), "-Dflag"));
+ }
----------------
move this to the test
================
Comment at: clang/unittests/Tooling/CompilationDatabaseTest.cpp:870
+ bool addFile(StringRef File, StringRef Context) {
+ return FS->addFile(File, 0, llvm::MemoryBuffer::getMemBufferCopy(Context));
+ }
----------------
nit: I would rather keep ASSERT_TRUE in here
================
Comment at: clang/unittests/Tooling/CompilationDatabaseTest.cpp:877
+ ->getCompileCommands(path(F));
+ if (Results.empty()) {
+ return "none";
----------------
nit: no need for braces
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70222/new/
https://reviews.llvm.org/D70222
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
