[PATCH] D124874: [clang] add -fmodule-file-home-is-cwd

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.


================
Comment at: clang/lib/Serialization/ASTWriter.cpp:1231-1236
+    SmallString<128> BaseDir(CWD->getName());
+    cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
+    BaseDirectory.assign(BaseDir.begin(), BaseDir.end());
+  } else if (WritingModule && WritingModule->Directory) {
     SmallString<128> BaseDir(WritingModule->Directory->getName());
     cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
----------------
rmaz wrote:
> urnathan wrote:
> > This looks unfortunately duplicatey.  Any less-worse way of avoiding that?
> It could be moved into the `else` clause at the cost of complicating the `if` statement with another case and adding another nested `if`. I thought it was simpler like this, but can change if you prefer.
IMHO it would be simpler to remove the duplication.  It'd make it clearer one's essentially doing the same thing with CWD as with Directory, (modulo the conditional bit just below).

code bloat make compiler slow :) 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124874/new/

https://reviews.llvm.org/D124874