[clang] Use resolved path for headers in decluse diagnostics

[kadircet]

Summary

A header spelling in a source file could potentially mean multiple
sources, especially in projects with multiple include search paths. We
try to eliminate this ambiguity by using the resolved path in the
diagnostics. This should improve the life of both the developers and the
tooling.

[kadircet]

• handle windows paths in tests

[sammccall]

This seems sensible to me but hard to know if it's going to be an improvement in practice. Can you paste a realistic example?

@rsmith any opinion about this diag?

clang/test/Modules/declare-use1.cpp

6

I'd be tempted to leave the slashes in the globs for readability

[kadircet]

It is the absolute path of the header. e.g:

module TestTU does not depend on a module exporting '/work/llvm/clang-tools-extra/clangd/AST.h'

rather than

module TestTU does not depend on a module exporting 'AST.h'

[sammccall]

We discussed this a bit offline, and IIUC the plan is not to push this through at least for now. Notes:

• path to a file is more accurate/informative than spelling, since it really is the file rather than the name that is restricted
• however in practice the spelling probably isn't ambiguous and is more readable, so this is probably a regression overall for display
• our motivation here is to be able to unambiguously extract the path programmatically.
  • Adding a struct { string; FileEntry*; } as a diagnostic argument kind is another (somewhat intrusive) way out.
  • The (private) codebase we're experimenting on expresses most #includes relative to a single root, so we can probably just live without this at least for a while
• If we do this getName() is probably indeed better than tryGetRealPathName, resolving symlinks may have some bad effects

clang/lib/Lex/ModuleMap.cpp

if (Known != Headers.end()) {
     }
 
     Excluded = true;
   }
 
   // We have found a header, but it is private.
   if (Private) {
     Diags.Report(FilenameLoc, diag::warn_use_of_private_header_outside_module)
-        << Filename;
+        << File->getName();
     return;
   }
 
   // We have found a module, but we don't use it.
   if (NotUsed) {
     Diags.Report(FilenameLoc, diag::err_undeclared_use_of_module)
-        << RequestingModule->getTopLevelModule()->Name << Filename;
+        << RequestingModule->getTopLevelModule()->Name << File->getName();
     return;
   }
 
   if (Excluded || isHeaderInUmbrellaDirs(File))
     return;
 
   // At this point, only non-modular includes remain.
 
   if (RequestingModule && LangOpts.ModulesStrictDeclUse) {
     Diags.Report(FilenameLoc, diag::err_undeclared_use_of_module)
-        << RequestingModule->getTopLevelModule()->Name << Filename;
+        << RequestingModule->getTopLevelModule()->Name << File->getName();
   } else if (RequestingModule && RequestingModuleIsModuleInterface &&
              LangOpts.isCompilingModule()) {
     // Do not diagnose when we are not compiling a module.
     diag::kind DiagID = RequestingModule->getTopLevelModule()->IsFramework ?
         diag::warn_non_modular_include_in_framework_module :
         diag::warn_non_modular_include_in_module;
     Diags.Report(FilenameLoc, DiagID) << RequestingModule->getFullModuleName()
         << File->getName();

clang/test/Modules/declare-use1.cpp

 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s -verify
 
 #include "g.h"
 #include "e.h"
-#include "f.h" // expected-error {{module XG does not depend on a module exporting 'f.h'}}
+#include "f.h" // expected-error-re {{module XG does not depend on a module exporting '{{.*}}{{/|\\}}f.h'}}
 #include "i.h"
 #include "sub.h"
 const int g2 = g1 + e + f + aux_i + sub;

clang/test/Modules/declare-use2.cpp

 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-decluse -fmodule-name=XH -I %S/Inputs/declare-use %s -verify
 
 #include "h.h"
 #include "e.h"
-#include "f.h" // expected-error {{module XH does not depend on a module exporting 'f.h'}}
+#include "f.h" // expected-error-re {{module XH does not depend on a module exporting '{{.*}}{{/|\\}}f.h'}}
 const int h2 = h1+e+f;

clang/test/Modules/declare-use3.cpp

 // RUN: rm -rf %t
 // RUN: %clang_cc1 -include "g.h" -include "e.h" -include "f.h" -include "i.h" -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s -verify
-// expected-error {{module XG does not depend on a module exporting 'f.h'}}
+// expected-error-re {{module XG does not depend on a module exporting '{{.*}}{{/|\\}}f.h'}}
 const int g2 = g1 + e + f + aux_i;

clang/test/Modules/header-attribs.cpp

 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fmodules -I%S/Inputs/header-attribs -fmodule-map-file=%S/Inputs/header-attribs/textual.modulemap -fmodules-cache-path=%t -verify %s -fmodule-name=A -fmodules-strict-decluse
 // RUN: not %clang_cc1 -fmodules -I%S/Inputs/header-attribs -emit-module -x c++-module-map %S/Inputs/header-attribs/modular.modulemap -fmodules-cache-path=%t -fmodule-name=A 2>&1 | FileCheck %s --check-prefix BUILD-MODULAR
 
 #include "foo.h" // ok, stats match
-#include "bar.h" // expected-error {{does not depend on a module exporting 'bar.h'}}
-#include "baz.h" // expected-error {{does not depend on a module exporting 'baz.h'}}
+#include "bar.h" // expected-error-re {{does not depend on a module exporting '{{.*}}{{/|\\}}bar.h'}}
+#include "baz.h" // expected-error-re {{does not depend on a module exporting '{{.*}}{{/|\\}}baz.h'}}
 
 // FIXME: Explain why the 'bar.h' found on disk doesn't match the module map.
 // BUILD-MODULAR: error: header 'bar.h' not found

clang/test/Modules/strict-decluse-headers.cpp

 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: touch %t/foo.h
 // RUN: echo '#include "foo.h"' > %t/bar.h
 // RUN: touch %t/baz.h
 // RUN: echo 'module X { header "bar.h" header "baz.h" }' > %t/map
 //
 // RUN: not %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%t/map -I%t -fmodules-strict-decluse -fmodule-name=X -x c++ %t/bar.h %t/baz.h 2>&1 | FileCheck %s
 // RUN: not %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%t/map -I%t -fmodules-strict-decluse -fmodule-name=X -x c++ %t/baz.h %t/bar.h 2>&1 | FileCheck %s
 //
 // Don't crash on this: (FIXME: we should produce an error that the specified module name is not known)
 // RUN: %clang_cc1 -fsyntax-only -fmodules -I%t -fmodules-strict-decluse -fmodule-name=X -x c++ %t/baz.h %t/bar.h
 //
 // Don't crash on this: (FIXME: we should produce an error that the specified file is not part of the specified module)
 // RUN: %clang_cc1 -fsyntax-only -fmodules -fmodule-map-file=%t/map -I%t -fmodules-strict-decluse -fmodule-name=X -x c++ %t/foo.h
 //
-// CHECK: module X does not depend on a module exporting 'foo.h'
+// CHECK: module X does not depend on a module exporting '{{.*}}{{/|\\}}foo.h'

clang/test/Modules/strict-decluse.cpp

 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-strict-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s -verify
 
 #include "g.h"
 #include "e.h"
-#include "f.h" // expected-error {{module XG does not depend on a module exporting 'f.h'}}
-#include "i.h" // expected-error {{module XG does not depend on a module exporting 'i.h'}}
+#include "f.h" // expected-error-re {{module XG does not depend on a module exporting '{{.*}}{{/|\\}}f.h'}}
+#include "i.h" // expected-error-re {{module XG does not depend on a module exporting '{{.*}}{{/|\\}}i.h'}}
 
 const int g2 = g1 + e + f + aux_i;

clang/test/Modules/textual-headers.cpp

 // RUN: rm -rf %t
 // RUN: %clang_cc1 -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-strict-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s -verify
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-strict-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s -verify -fno-modules-error-recovery
 
 #define GIMME_A_K
 #include "k.h"
 
 #define GIMME_AN_L
-#include "l.h" // expected-error {{module XG does not depend on a module exporting 'l.h'}}
+#include "l.h" // expected-error-re {{module XG does not depend on a module exporting '{{.*}}{{/|\\}}l.h'}}
 
-#include "m2.h" // expected-error {{module XG does not depend on a module exporting 'm2.h'}}
+#include "m2.h" // expected-error-re {{module XG does not depend on a module exporting '{{.*}}{{/|\\}}m2.h'}}
 const int use_m = m; // expected-error {{undeclared identifier}}
 
 #define GIMME_AN_M
-#include "m.h" // expected-error {{use of private header from outside its module: 'm.h'}}
+#include "m.h" // expected-error-re {{use of private header from outside its module: '{{.*}}{{/|\\}}m.h'}}
 const int use_m_2 = m;
 
 const int g = k + l;