ayzhao added inline comments.
================
Comment at: clang/lib/AST/Expr.cpp:2193
SmallString<256> Path(PLoc.getFilename());
Ctx.getLangOpts().remapPathPrefix(Path);
+ if (Ctx.getTargetInfo().getTriple().isOSWindows()) {
----------------
hans wrote:
> I was going to say perhaps we should put a comment about why Windows needs
> different treatment, but as we'd need to put the comment in three different
> places, maybe this (remapPathPrefix() and make_preferred()) should be
> factored out to a utility function?
Extracting these lines into a separate function (say, for example,
`handle_file_macro(...)`?) makes sense. However, I'm not sure which file this
function would belong:
* The function would need to accept a `LangOpts` and a `TargetInfo` object.
Both of these classes are Clang specific.
* It shouldn't belong in `llvm::sys::path::Style` because the file declaring
that namespace exists in the llvm directory, which shouldn't depend on things
in Clang.
* We could put it in `LangOpts`, but `TargetInfo.h` includes `LangOpts.h`, so
this would create a circular dependency.
* `TargetInfo` doesn't make much sense as it doesn't have any code generation /
language specific logic.
WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122766/new/
https://reviews.llvm.org/D122766
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
