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

[Hans Wennborg via Phabricator via llvm-commits]

hans added a comment.

> we should have a flag that controls which slash direction to use on windows triples, since different people will want different things.
> And since most people who use clang-cl run it on Windows, the default for that flag should imho stay a backslash (but projects can add the forward direction flag if they want).

I guess this is the part that's not entirely clear to me. Do people really *want* the backslash? Would anything break if we just use the forward slash?

This is MSVC's current behaviour:

  C:\src\tmp>type a.cc foo\a.h
  
  a.cc
  
  
  #include "foo/a.h"
  
  int main() { f(); return 0; }
  
  foo\a.h
  
  
  #include <stdio.h>
  
  inline void f() { printf("%s\n", __FILE__); }
  
  C:\src\tmp>cl /nologo a.cc && a.exe
  a.cc
  C:\src\tmp\foo/a.h

I think

  C:\src\tmp\foo/a.h

looks pretty weird, and canonicalizing on slashes would be an improvement even if it diverges from MSVC.

> (I don't think the argument about '\' in include lines applies to this patch (?))

It kinda does in that developers, even on Windows, already use forward slashes to refer to included files:

  #include "foo/bar.h"

and so having the compiler prefer forward slashes for the whole filename is in line with that.

While this would make clang-cl diverge from MSVC, maybe this is one of those cases where we want to diverge because clang-cl's way is better. Unless it breaks something.



================
Comment at: clang/lib/AST/Expr.cpp:2193
     SmallString<256> Path(PLoc.getFilename());
     Ctx.getLangOpts().remapPathPrefix(Path);
+    if (Ctx.getTargetInfo().getTriple().isOSWindows()) {
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766