Add support for -fdiagnostics-abs-path: printing absolute paths in diagnostics

[hans]

Summary

It turns out several Chromium developers rely on this on Windows.

[thakis]

include/clang/Driver/CLCompatOptions.td

68 ↗

(On Diff #69045)

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.)

[hans]

include/clang/Driver/CLCompatOptions.td

68 ↗

(On Diff #69045)

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.

[thakis]

include/clang/Driver/CLCompatOptions.td

68 ↗

(On Diff #69045)

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.

[hans]

include/clang/Driver/CLCompatOptions.td

68 ↗

(On Diff #69045)

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?

[thakis]

include/clang/Driver/CLCompatOptions.td

68 ↗

(On Diff #69045)

Works for me :-)

[hans]

Just doing -fdiagnostics-abs-path.

Please take another look.

[thakis]

I like this approach.

[rsmith]

include/clang/Driver/Options.td

994 ↗

(On Diff #69300)

How about -fdiagnostics-[use-]absolute-paths rather than -fdiagnostics-abs-path to avoid any confusion about this referring to the abs function?

lib/Frontend/TextDiagnostic.cpp

773 ↗

(On Diff #69300)

Err, setting the second parameter to true here causes it to generate incorrect paths. I wouldn't do that... If you want to remove .. components, you should use FileManager::getCanonicalName instead, which will do the appropriate symlink resolution here.

[hans]

include/clang/Driver/Options.td

994 ↗

(On Diff #69300)

Sounds good to me.

[hans]

Addressing Richard's comments.

[rsmith]

lib/Frontend/TextDiagnostic.cpp

770–777 ↗

(On Diff #69304)

Why split off the filename and rejoin it here? Is this to better handle cases where the directory exists but the file doesn't, or an attempt to avoid resolving file symlinks? A comment in the code explaining this would be useful.

[hans]

lib/Frontend/TextDiagnostic.cpp

770–777 ↗

(On Diff #69304)

If I understand correctly, getCanonicalName only works with a DirectoryEntry, which is supposed to be a directory, not a full path to a file. So I didn't see any other way than getting the canonical name for the dir, and then joining it with the filename.

cfe/trunk/include/clang/Basic/DiagnosticOptions.def

 #endif
 
 SEMANTIC_DIAGOPT(IgnoreWarnings, 1, 0)   /// -w
 DIAGOPT(NoRewriteMacros, 1, 0)  /// -Wno-rewrite-macros
 DIAGOPT(Pedantic, 1, 0)         /// -pedantic
 DIAGOPT(PedanticErrors, 1, 0)   /// -pedantic-errors
 DIAGOPT(ShowColumn, 1, 1)       /// Show column number on diagnostics.
 DIAGOPT(ShowLocation, 1, 1)     /// Show source location information.
+DIAGOPT(AbsolutePath, 1, 0)     /// Use absolute paths.
 DIAGOPT(ShowCarets, 1, 1)       /// Show carets in diagnostics.
 DIAGOPT(ShowFixits, 1, 1)       /// Show fixit information.
 DIAGOPT(ShowSourceRanges, 1, 0) /// Show source ranges in numeric form.
 DIAGOPT(ShowParseableFixits, 1, 0) /// Show machine parseable fix-its.
 DIAGOPT(ShowPresumedLoc, 1, 0)  /// Show presumed location for diagnostics.
 DIAGOPT(ShowOptionNames, 1, 0)  /// Show the option name for mappable
                                 /// diagnostics.
 DIAGOPT(ShowNoteIncludeStack, 1, 0) /// Show include stacks for notes.

cfe/trunk/include/clang/Driver/CLCompatOptions.td

 // Ignored:
 
 def _SLASH_analyze_ : CLIgnoredFlag<"analyze-">;
 def _SLASH_bigobj : CLIgnoredFlag<"bigobj">;
 def _SLASH_cgthreads : CLIgnoredJoined<"cgthreads">;
 def _SLASH_d2FastFail : CLIgnoredFlag<"d2FastFail">;
 def _SLASH_d2Zi_PLUS : CLIgnoredFlag<"d2Zi+">;
 def _SLASH_errorReport : CLIgnoredJoined<"errorReport">;
-def _SLASH_Fd : CLIgnoredJoined<"Fd">;
 def _SLASH_FC : CLIgnoredFlag<"FC">;
+def _SLASH_Fd : CLIgnoredJoined<"Fd">;
 def _SLASH_FS : CLIgnoredFlag<"FS">, HelpText<"Force synchronous PDB writes">;
 def _SLASH_GF : CLIgnoredFlag<"GF">;
 def _SLASH_kernel_ : CLIgnoredFlag<"kernel-">;
 def _SLASH_nologo : CLIgnoredFlag<"nologo">;
 def _SLASH_Og : CLIgnoredFlag<"Og">;
 def _SLASH_openmp_ : CLIgnoredFlag<"openmp-">;
 def _SLASH_RTC : CLIgnoredJoined<"RTC">;
 def _SLASH_sdl : CLIgnoredFlag<"sdl">;

cfe/trunk/include/clang/Driver/Options.td

 def fno_pascal_strings : Flag<["-"], "fno-pascal-strings">, Group<f_Group>;
 def fno_rtti : Flag<["-"], "fno-rtti">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Disable generation of rtti information">;
 def fno_short_enums : Flag<["-"], "fno-short-enums">, Group<f_Group>;
 def fno_show_column : Flag<["-"], "fno-show-column">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Do not include column number on diagnostics">;
 def fno_show_source_location : Flag<["-"], "fno-show-source-location">, Group<f_Group>,
   Flags<[CC1Option]>, HelpText<"Do not include source location information with diagnostics">;
+def fdiagnostics_absolute_paths : Flag<["-"], "fdiagnostics-absolute-paths">, Group<f_Group>,
+  Flags<[CC1Option, CoreOption]>, HelpText<"Print absolute paths in diagnostics">;
 def fno_spell_checking : Flag<["-"], "fno-spell-checking">, Group<f_Group>,
   Flags<[CC1Option]>, HelpText<"Disable spell-checking">;
 def fno_stack_protector : Flag<["-"], "fno-stack-protector">, Group<f_Group>,
   HelpText<"Disable the use of stack protectors">;
 def fno_strict_aliasing : Flag<["-"], "fno-strict-aliasing">, Group<f_Group>,
   Flags<[DriverOption, CoreOption]>;
 def fstruct_path_tbaa : Flag<["-"], "fstruct-path-tbaa">, Group<f_Group>;
 def fno_struct_path_tbaa : Flag<["-"], "fno-struct-path-tbaa">, Group<f_Group>;

cfe/trunk/include/clang/Frontend/TextDiagnostic.h

void emitImportLocation(SourceLocation Loc, PresumedLoc PLoc,
                           StringRef ModuleName,
                           const SourceManager &SM) override;
 
   void emitBuildingModuleLocation(SourceLocation Loc, PresumedLoc PLoc,
                                   StringRef ModuleName,
                                   const SourceManager &SM) override;
 
 private:
+  void emitFilename(StringRef Filename, const SourceManager &SM);
+
   void emitSnippetAndCaret(SourceLocation Loc, DiagnosticsEngine::Level Level,
                            SmallVectorImpl<CharSourceRange>& Ranges,
                            ArrayRef<FixItHint> Hints,
                            const SourceManager &SM);
 
   void emitSnippet(StringRef SourceLine);
 
   void emitParseableFixits(ArrayRef<FixItHint> Hints, const SourceManager &SM);
 };
 
 } // end namespace clang
 
 #endif

cfe/trunk/lib/Driver/Tools.cpp

 
   if (Args.hasArg(options::OPT_fansi_escape_codes))
     CmdArgs.push_back("-fansi-escape-codes");
 
   if (!Args.hasFlag(options::OPT_fshow_source_location,
                     options::OPT_fno_show_source_location))
     CmdArgs.push_back("-fno-show-source-location");
 
+  if (Args.hasArg(options::OPT_fdiagnostics_absolute_paths))
+    CmdArgs.push_back("-fdiagnostics-absolute-paths");
+
   if (!Args.hasFlag(options::OPT_fshow_column, options::OPT_fno_show_column,
                     true))
     CmdArgs.push_back("-fno-show-column");
 
   if (!Args.hasFlag(options::OPT_fspell_checking,
                     options::OPT_fno_spell_checking))
     CmdArgs.push_back("-fno-spell-checking");
 

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
   Opts.PedanticErrors = Args.hasArg(OPT_pedantic_errors);
   Opts.ShowCarets = !Args.hasArg(OPT_fno_caret_diagnostics);
   Opts.ShowColors = parseShowColorsArgs(Args, DefaultDiagColor);
   Opts.ShowColumn = Args.hasFlag(OPT_fshow_column,
                                  OPT_fno_show_column,
                                  /*Default=*/true);
   Opts.ShowFixits = !Args.hasArg(OPT_fno_diagnostics_fixit_info);
   Opts.ShowLocation = !Args.hasArg(OPT_fno_show_source_location);
+  Opts.AbsolutePath = Args.hasArg(OPT_fdiagnostics_absolute_paths);
   Opts.ShowOptionNames = Args.hasArg(OPT_fdiagnostics_show_option);
 
   llvm::sys::Process::UseANSIEscapeCodes(Args.hasArg(OPT_fansi_escape_codes));
 
   // Default behavior is to not to show note include stacks.
   Opts.ShowNoteIncludeStack = false;
   if (Arg *A = Args.getLastArg(OPT_fdiagnostics_show_note_include_stack,
                                OPT_fno_diagnostics_show_note_include_stack))

cfe/trunk/lib/Frontend/TextDiagnostic.cpp

 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Locale.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 
 using namespace clang;
 
 static const enum raw_ostream::Colors noteColor =
   raw_ostream::BLACK;
 static const enum raw_ostream::Colors remarkColor =
else {
     assert(Normal && "Formatting should have returned to normal");
   }
 
   if (ShowColors)
     OS.resetColor();
   OS << '\n';
 }
 
+void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
+  SmallVector<char, 128> AbsoluteFilename;
+  if (DiagOpts->AbsolutePath) {
+    const DirectoryEntry *Dir = SM.getFileManager().getDirectory(
+        llvm::sys::path::parent_path(Filename));
+    if (Dir) {
+      StringRef DirName = SM.getFileManager().getCanonicalName(Dir);
+      llvm::sys::path::append(AbsoluteFilename, DirName,
+                              llvm::sys::path::filename(Filename));
+      Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
+    }
+  }
+
+  OS << Filename;
+}
+
 /// \brief Print out the file/line/column information and include trace.
 ///
 /// This method handlen the emission of the diagnostic location information.
 /// This includes extracting as much location information as is present for
 /// the diagnostic and printing it, as well as any include stack or source
 /// ranges necessary.
 void TextDiagnostic::emitDiagnosticLoc(SourceLocation Loc, PresumedLoc PLoc,
                                        DiagnosticsEngine::Level Level,
                                        ArrayRef<CharSourceRange> Ranges,
                                        const SourceManager &SM) {
   if (PLoc.isInvalid()) {
     // At least print the file name if available:
     FileID FID = SM.getFileID(Loc);
     if (FID.isValid()) {
       const FileEntry* FE = SM.getFileEntryForID(FID);
       if (FE && FE->isValid()) {
-        OS << FE->getName();
+        emitFilename(FE->getName(), SM);
         if (FE->isInPCH())
           OS << " (in PCH)";
         OS << ": ";
       }
     }
     return;
   }
   unsigned LineNo = PLoc.getLine();
 
   if (!DiagOpts->ShowLocation)
     return;
 
   if (DiagOpts->ShowColors)
     OS.changeColor(savedColor, true);
 
-  OS << PLoc.getFilename();
+  emitFilename(PLoc.getFilename(), SM);
   switch (DiagOpts->getFormat()) {
   case DiagnosticOptions::Clang: OS << ':'  << LineNo; break;
   case DiagnosticOptions::MSVC:  OS << '('  << LineNo; break;
   case DiagnosticOptions::Vi:    OS << " +" << LineNo; break;
   }
 
   if (DiagOpts->ShowColumn)
     // Compute the column number.

cfe/trunk/test/Driver/cl-options.c

 // RUN:     /EHsc \
 // RUN:     /F \
 // RUN:     /FA \
 // RUN:     /FAc \
 // RUN:     /Fafilename \
 // RUN:     /FAs \
 // RUN:     /FAu \
 // RUN:     /favor:blend \
-// RUN:     /FC \
 // RUN:     /Fifoo \
 // RUN:     /Fmfoo \
 // RUN:     /FpDebug\main.pch \
 // RUN:     /Frfoo \
 // RUN:     /FRfoo \
 // RUN:     /FU foo \
 // RUN:     /Fx \
 // RUN:     /G1 \

 // (/Zs is for syntax-only, -Werror makes it fail hard on unknown options)
 // RUN: %clang_cl \
 // RUN:     --driver-mode=cl \
 // RUN:     -fcolor-diagnostics \
 // RUN:     -fno-color-diagnostics \
 // RUN:     -fdiagnostics-color \
 // RUN:     -fno-diagnostics-color \
 // RUN:     -fdiagnostics-parseable-fixits \
+// RUN:     -fdiagnostics-absolute-paths \
 // RUN:     -ferror-limit=10 \
 // RUN:     -fmsc-version=1800 \
 // RUN:     -fno-strict-aliasing \
 // RUN:     -fstrict-aliasing \
 // RUN:     -fsyntax-only \
 // RUN:     -fms-compatibility \
 // RUN:     -fno-ms-compatibility \
 // RUN:     -fms-extensions \

cfe/trunk/test/Frontend/Inputs/absolute-paths.h

+int f() {
+  // Oops, no return.
+}

cfe/trunk/test/Frontend/absolute-paths.c

+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. %s 2>&1 | FileCheck -check-prefix=NORMAL -check-prefix=CHECK %s
+// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SystemHeaderPrefix/.. -fdiagnostics-absolute-paths %s 2>&1 | FileCheck -check-prefix=ABSOLUTE -check-prefix=CHECK %s
+
+#include "absolute-paths.h"
+
+// Check whether the diagnostic from the header above includes the dummy
+// directory in the path.
+// NORMAL: SystemHeaderPrefix
+// ABSOLUTE-NOT: SystemHeaderPrefix
+// CHECK: warning: control reaches end of non-void function
+
+
+// For files which don't exist, just print the filename.
+#line 123 "non-existant.c"
+int g() {}
+// NORMAL: non-existant.c:123:10: warning: control reaches end of non-void function
+// ABSOLUTE: non-existant.c:123:10: warning: control reaches end of non-void function