[clang] f9fbda7 - Revert "[clang] Canonicalize system headers in dependency file when -canonical-prefixes"

Arthur Eubanks via cfe-commits cfe-commits at lists.llvm.org
Mon May 1 10:56:03 PDT 2023


Author: Arthur Eubanks
Date: 2023-05-01T10:55:53-07:00
New Revision: f9fbda7102fb6da79dedf9b15694d7a61e8c49b7

URL: https://github.com/llvm/llvm-project/commit/f9fbda7102fb6da79dedf9b15694d7a61e8c49b7
DIFF: https://github.com/llvm/llvm-project/commit/f9fbda7102fb6da79dedf9b15694d7a61e8c49b7.diff

LOG: Revert "[clang] Canonicalize system headers in dependency file when -canonical-prefixes"

This reverts commit 8f0dd4ef3ed2137d1e2554204127434fc46ee190.

Causes bot failures: https://lab.llvm.org/buildbot/#builders/139/builds/39964

Added: 
    

Modified: 
    clang/include/clang/Driver/Options.td
    clang/include/clang/Frontend/DependencyOutputOptions.h
    clang/include/clang/Frontend/Utils.h
    clang/lib/Driver/ToolChains/Clang.cpp
    clang/lib/Frontend/DependencyFile.cpp

Removed: 
    clang/test/Driver/canonical-system-headers.c
    clang/test/Preprocessor/Inputs/canonical-system-headers/a.h
    clang/test/Preprocessor/canonical-system-headers.c


################################################################################
diff  --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 182f0290736d8..8c80c7f5ff56a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5835,9 +5835,6 @@ let Flags = [CC1Option, NoDriverOption] in {
 def sys_header_deps : Flag<["-"], "sys-header-deps">,
   HelpText<"Include system headers in dependency output">,
   MarshallingInfoFlag<DependencyOutputOpts<"IncludeSystemHeaders">>;
-def canonical_system_headers : Flag<["-"], "canonical-system-headers">,
-  HelpText<"Canonicalize system headers in dependency output">,
-  MarshallingInfoFlag<DependencyOutputOpts<"CanonicalSystemHeaders">>;
 def module_file_deps : Flag<["-"], "module-file-deps">,
   HelpText<"Include module files in dependency output">,
   MarshallingInfoFlag<DependencyOutputOpts<"IncludeModuleFiles">>;

diff  --git a/clang/include/clang/Frontend/DependencyOutputOptions.h b/clang/include/clang/Frontend/DependencyOutputOptions.h
index e140ff9baa117..e0f445bb5970c 100644
--- a/clang/include/clang/Frontend/DependencyOutputOptions.h
+++ b/clang/include/clang/Frontend/DependencyOutputOptions.h
@@ -34,8 +34,6 @@ enum ExtraDepKind {
 class DependencyOutputOptions {
 public:
   unsigned IncludeSystemHeaders : 1; ///< Include system header dependencies.
-  unsigned
-      CanonicalSystemHeaders : 1; ///< canonicalize system header dependencies.
   unsigned ShowHeaderIncludes : 1;   ///< Show header inclusions (-H).
   unsigned UsePhonyTargets : 1;      ///< Include phony targets for each
                                      /// dependency, which can avoid some 'make'
@@ -87,11 +85,10 @@ class DependencyOutputOptions {
 
 public:
   DependencyOutputOptions()
-      : IncludeSystemHeaders(0), CanonicalSystemHeaders(0),
-        ShowHeaderIncludes(0), UsePhonyTargets(0), AddMissingHeaderDeps(0),
-        IncludeModuleFiles(0), ShowSkippedHeaderIncludes(0),
-        HeaderIncludeFormat(HIFMT_Textual), HeaderIncludeFiltering(HIFIL_None) {
-  }
+      : IncludeSystemHeaders(0), ShowHeaderIncludes(0), UsePhonyTargets(0),
+        AddMissingHeaderDeps(0), IncludeModuleFiles(0),
+        ShowSkippedHeaderIncludes(0), HeaderIncludeFormat(HIFMT_Textual),
+        HeaderIncludeFiltering(HIFIL_None) {}
 };
 
 }  // end namespace clang

diff  --git a/clang/include/clang/Frontend/Utils.h b/clang/include/clang/Frontend/Utils.h
index 8300e45d15fe5..143cf4359f00b 100644
--- a/clang/include/clang/Frontend/Utils.h
+++ b/clang/include/clang/Frontend/Utils.h
@@ -41,7 +41,6 @@ class ExternalSemaSource;
 class FrontendOptions;
 class PCHContainerReader;
 class Preprocessor;
-class FileManager;
 class PreprocessorOptions;
 class PreprocessorOutputOptions;
 
@@ -80,14 +79,11 @@ class DependencyCollector {
   /// Return true if system files should be passed to sawDependency().
   virtual bool needSystemDependencies() { return false; }
 
-  /// Return true if system files should be canonicalized.
-  virtual bool shouldCanonicalizeSystemDependencies() { return false; }
-
   /// Add a dependency \p Filename if it has not been seen before and
   /// sawDependency() returns true.
   virtual void maybeAddDependency(StringRef Filename, bool FromModule,
                                   bool IsSystem, bool IsModuleFile,
-                                  FileManager *FileMgr, bool IsMissing);
+                                  bool IsMissing);
 
 protected:
   /// Return true if the filename was added to the list of dependencies, false
@@ -116,10 +112,6 @@ class DependencyFileGenerator : public DependencyCollector {
   bool sawDependency(StringRef Filename, bool FromModule, bool IsSystem,
                      bool IsModuleFile, bool IsMissing) final;
 
-  bool shouldCanonicalizeSystemDependencies() override {
-    return CanonicalSystemHeaders;
-  }
-
 protected:
   void outputDependencyFile(llvm::raw_ostream &OS);
 
@@ -129,7 +121,6 @@ class DependencyFileGenerator : public DependencyCollector {
   std::string OutputFile;
   std::vector<std::string> Targets;
   bool IncludeSystemHeaders;
-  bool CanonicalSystemHeaders;
   bool PhonyTarget;
   bool AddMissingHeaderDeps;
   bool SeenMissingHeader;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 5b93b84dfcf15..33ad50ad13483 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1139,9 +1139,6 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
     if (ArgM->getOption().matches(options::OPT_M) ||
         ArgM->getOption().matches(options::OPT_MD))
       CmdArgs.push_back("-sys-header-deps");
-    if (Args.hasFlag(options::OPT_canonical_prefixes,
-                     options::OPT_no_canonical_prefixes, true))
-      CmdArgs.push_back("-canonical-system-headers");
     if ((isa<PrecompileJobAction>(JA) &&
          !Args.hasArg(options::OPT_fno_module_file_deps)) ||
         Args.hasArg(options::OPT_fmodule_file_deps))

diff  --git a/clang/lib/Frontend/DependencyFile.cpp b/clang/lib/Frontend/DependencyFile.cpp
index 44268e71dc241..fe4218b6e672d 100644
--- a/clang/lib/Frontend/DependencyFile.cpp
+++ b/clang/lib/Frontend/DependencyFile.cpp
@@ -49,7 +49,6 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
       DepCollector.maybeAddDependency(
           llvm::sys::path::remove_leading_dotslash(*Filename),
           /*FromModule*/ false, isSystem(FileType), /*IsModuleFile*/ false,
-          &PP.getFileManager(),
           /*IsMissing*/ false);
   }
 
@@ -57,11 +56,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
                    SrcMgr::CharacteristicKind FileType) override {
     StringRef Filename =
         llvm::sys::path::remove_leading_dotslash(SkippedFile.getName());
-    DepCollector.maybeAddDependency(Filename,
-                                    /*FromModule=*/false,
+    DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
                                     /*IsSystem=*/isSystem(FileType),
                                     /*IsModuleFile=*/false,
-                                    &PP.getFileManager(),
                                     /*IsMissing=*/false);
   }
 
@@ -72,12 +69,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
                           StringRef RelativePath, const Module *Imported,
                           SrcMgr::CharacteristicKind FileType) override {
     if (!File)
-      DepCollector.maybeAddDependency(FileName,
-                                      /*FromModule*/ false,
-                                      /*IsSystem*/ false,
-                                      /*IsModuleFile*/ false,
-                                      &PP.getFileManager(),
-                                      /*IsMissing*/ true);
+      DepCollector.maybeAddDependency(FileName, /*FromModule*/false,
+                                     /*IsSystem*/false, /*IsModuleFile*/false,
+                                     /*IsMissing*/true);
     // Files that actually exist are handled by FileChanged.
   }
 
@@ -88,11 +82,9 @@ struct DepCollectorPPCallbacks : public PPCallbacks {
       return;
     StringRef Filename =
         llvm::sys::path::remove_leading_dotslash(File->getName());
-    DepCollector.maybeAddDependency(Filename,
-                                    /*FromModule=*/false,
+    DepCollector.maybeAddDependency(Filename, /*FromModule=*/false,
                                     /*IsSystem=*/isSystem(FileType),
                                     /*IsModuleFile=*/false,
-                                    &PP.getFileManager(),
                                     /*IsMissing=*/false);
   }
 
@@ -108,12 +100,10 @@ struct DepCollectorMMCallbacks : public ModuleMapCallbacks {
   void moduleMapFileRead(SourceLocation Loc, const FileEntry &Entry,
                          bool IsSystem) override {
     StringRef Filename = Entry.getName();
-    DepCollector.maybeAddDependency(Filename,
-                                    /*FromModule*/ false,
-                                    /*IsSystem*/ IsSystem,
-                                    /*IsModuleFile*/ false,
-                                    /*FileMgr*/ nullptr,
-                                    /*IsMissing*/ false);
+    DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
+                                    /*IsSystem*/IsSystem,
+                                    /*IsModuleFile*/false,
+                                    /*IsMissing*/false);
   }
 };
 
@@ -128,11 +118,9 @@ struct DepCollectorASTListener : public ASTReaderListener {
   }
   void visitModuleFile(StringRef Filename,
                        serialization::ModuleKind Kind) override {
-    DepCollector.maybeAddDependency(Filename,
-                                    /*FromModule*/ true,
-                                    /*IsSystem*/ false, /*IsModuleFile*/ true,
-                                    /*FileMgr*/ nullptr,
-                                    /*IsMissing*/ false);
+    DepCollector.maybeAddDependency(Filename, /*FromModule*/true,
+                                   /*IsSystem*/false, /*IsModuleFile*/true,
+                                   /*IsMissing*/false);
   }
   bool visitInputFile(StringRef Filename, bool IsSystem,
                       bool IsOverridden, bool IsExplicitModule) override {
@@ -144,9 +132,8 @@ struct DepCollectorASTListener : public ASTReaderListener {
     if (auto FE = FileMgr.getOptionalFileRef(Filename))
       Filename = FE->getName();
 
-    DepCollector.maybeAddDependency(Filename, /*FromModule*/ true, IsSystem,
-                                    /*IsModuleFile*/ false, /*FileMgr*/ nullptr,
-                                    /*IsMissing*/ false);
+    DepCollector.maybeAddDependency(Filename, /*FromModule*/true, IsSystem,
+                                   /*IsModuleFile*/false, /*IsMissing*/false);
     return true;
   }
 };
@@ -155,15 +142,9 @@ struct DepCollectorASTListener : public ASTReaderListener {
 void DependencyCollector::maybeAddDependency(StringRef Filename,
                                              bool FromModule, bool IsSystem,
                                              bool IsModuleFile,
-                                             FileManager *FileMgr,
                                              bool IsMissing) {
-  if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing)) {
-    if (IsSystem && FileMgr && shouldCanonicalizeSystemDependencies()) {
-      if (auto F = FileMgr->getFile(Filename))
-        Filename = FileMgr->getCanonicalName(*F);
-    }
+  if (sawDependency(Filename, FromModule, IsSystem, IsModuleFile, IsMissing))
     addDependency(Filename);
-  }
 }
 
 bool DependencyCollector::addDependency(StringRef Filename) {
@@ -211,7 +192,6 @@ DependencyFileGenerator::DependencyFileGenerator(
     const DependencyOutputOptions &Opts)
     : OutputFile(Opts.OutputFile), Targets(Opts.Targets),
       IncludeSystemHeaders(Opts.IncludeSystemHeaders),
-      CanonicalSystemHeaders(Opts.CanonicalSystemHeaders),
       PhonyTarget(Opts.UsePhonyTargets),
       AddMissingHeaderDeps(Opts.AddMissingHeaderDeps), SeenMissingHeader(false),
       IncludeModuleFiles(Opts.IncludeModuleFiles),

diff  --git a/clang/test/Driver/canonical-system-headers.c b/clang/test/Driver/canonical-system-headers.c
deleted file mode 100644
index a7ab57521fc22..0000000000000
--- a/clang/test/Driver/canonical-system-headers.c
+++ /dev/null
@@ -1,6 +0,0 @@
-// RUN: %clang -MD -no-canonical-prefixes -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-NO
-// RUN: %clang -MD -canonical-prefixes -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-YES
-// RUN: %clang -MD -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-YES
-
-// CHECK-YES: "-canonical-system-headers"
-// CHECK-NO-NOT: "-canonical-system-headers"

diff  --git a/clang/test/Preprocessor/Inputs/canonical-system-headers/a.h b/clang/test/Preprocessor/Inputs/canonical-system-headers/a.h
deleted file mode 100644
index e69de29bb2d1d..0000000000000

diff  --git a/clang/test/Preprocessor/canonical-system-headers.c b/clang/test/Preprocessor/canonical-system-headers.c
deleted file mode 100644
index ec443d11838d5..0000000000000
--- a/clang/test/Preprocessor/canonical-system-headers.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// don't create symlinks on windows
-// UNSUPPORTED: system-windows
-// REQUIRES: shell
-
-// RUN: rm -rf %t
-// RUN: mkdir -p %t/foo/
-// RUN: ln -f -s %S/Inputs/canonical-system-headers %t/foo/include
-// RUN: %clang_cc1 -isystem %T/foo/include -sys-header-deps -MT foo.o -dependency-file %t2 %s -fsyntax-only
-// RUN: FileCheck %s --check-prefix=NOCANON --implicit-check-not=a.h < %t2
-// RUN: %clang_cc1 -isystem %T/foo/include -sys-header-deps -MT foo.o -dependency-file %t2 %s -fsyntax-only -canonical-system-headers
-// RUN: FileCheck %s --check-prefix=CANON --implicit-check-not=a.h < %t2
-
-// NOCANON: foo/include/a.h
-// CANON: Inputs/canonical-system-headers/a.h
-
-#include <a.h>


        


More information about the cfe-commits mailing list