hans added inline comments.

================
Comment at: include/clang/Driver/CLCompatOptions.td:68
@@ +67,3 @@
+def _SLASH_FC : CLFlag<"FC">,
+  HelpText<"Use absolute paths in diagnostics and __FILE__">,
+  Alias<fabsolute_paths>;
----------------
thakis wrote:
> hans wrote:
> > thakis wrote:
> > > Hm, I'm not sure this should affect __FILE__. It sounds people want this 
> > > for diagnostics, but making it affect __FILE__ makes reproducible builds 
> > > harder. (There's -ffile-macro-prefix-to-remove from 
> > > http://reviews.llvm.org/D17741 for that too, but that's not in.)
> > The MSVC docs say it applies to __FILE__ though 
> > https://msdn.microsoft.com/en-us/library/027c4t2s.aspx and I worry that 
> > implementing only half of it might surprise users.
> > 
> > Chromium currently builds with MSVC and /FC, so we should already have full 
> > paths in __FILE__ there.
> Sure, but the MSVC build is not deterministic in several ways that folks 
> aren't happy about, and we're supposed to improve that :-)
> 
> As far as I know __FILE__ defaults to just the file's basename in cl.exe (is 
> that true?). So without /FC __FILE__ is useless there and people more or less 
> have to pass /FC. With clang, we emit the full relative path to the file, so 
> it's not useless already.
> 
> For diagnostics, I can kind of see how full paths are useful: You likely 
> develop on the same machine as you see the diagnostics on, so people want 
> real absolute paths so they can click on them in MSVC. For __FILE__, that's 
> less convincing, since that gets embedded into the binary that's shipped 
> elsewhere.
> As far as I know FILE defaults to just the file's basename in cl.exe (is that 
> true?).

In my testing, MSVC does include the full relative path in __FILE__ by default.

I agree that diagnostics seems to be the main motivation here. I'm just worried 
that if we implement /FC but leave out the FILE part, that's confusing for 
whoever might rely on it.

Maybe we should just do an -fabs-path-diagnostics option instead?


https://reviews.llvm.org/D23816



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to