[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

[Alan Zhao via Phabricator via cfe-commits]

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