From f54146c1789544b7348823a3cc6f488009de630e Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 23 Mar 2016 18:46:57 +0000 Subject: [PATCH] clang-cl: Fix remaining bugs in interaction of /Yc and /FI /showIncludes. Instead of putting the /Yc header into ExtraDeps, give DependencyOutputOptions a dedicated field for /Yc mode, and let HeaderIncludesCallback hang on to the full DependencyOutputOptions object, not just ExtraDeps. Reverts parts of r263352 that are now no longer needed. llvm-svn: 264182 --- .../clang/Frontend/DependencyOutputOptions.h | 3 ++ clang/include/clang/Frontend/Utils.h | 10 ++-- clang/lib/Frontend/CompilerInstance.cpp | 8 ++-- clang/lib/Frontend/HeaderIncludeGen.cpp | 53 +++++++++++----------- clang/test/Driver/cl-pch-showincludes.cpp | 9 ++-- 5 files changed, 41 insertions(+), 42 deletions(-) diff --git a/clang/include/clang/Frontend/DependencyOutputOptions.h b/clang/include/clang/Frontend/DependencyOutputOptions.h index 129b534..0be36cd 100644 --- a/clang/include/clang/Frontend/DependencyOutputOptions.h +++ b/clang/include/clang/Frontend/DependencyOutputOptions.h @@ -50,6 +50,9 @@ public: /// A list of filenames to be used as extra dependencies for every target. std::vector ExtraDeps; + /// In /showIncludes mode, pretend the main TU is a header with this name. + std::string ShowIncludesPretendHeader; + /// \brief The file to write GraphViz-formatted header dependencies to. std::string DOTOutputFile; diff --git a/clang/include/clang/Frontend/Utils.h b/clang/include/clang/Frontend/Utils.h index a5f667e..ebd2df8 100644 --- a/clang/include/clang/Frontend/Utils.h +++ b/clang/include/clang/Frontend/Utils.h @@ -142,15 +142,13 @@ public: /// AttachDependencyGraphGen - Create a dependency graph generator, and attach /// it to the given preprocessor. - void AttachDependencyGraphGen(Preprocessor &PP, StringRef OutputFile, - StringRef SysRoot); +void AttachDependencyGraphGen(Preprocessor &PP, StringRef OutputFile, + StringRef SysRoot); /// AttachHeaderIncludeGen - Create a header include list generator, and attach /// it to the given preprocessor. /// -/// \param ExtraHeaders - If not empty, will write the header filenames, just -/// like they were included during a regular preprocessing. Useful for -/// implicit include dependencies, like sanitizer blacklists. +/// \param DepOpts - Options controlling the output. /// \param ShowAllHeaders - If true, show all header information instead of just /// headers following the predefines buffer. This is useful for making sure /// includes mentioned on the command line are also reported, but differs from @@ -160,7 +158,7 @@ public: /// \param ShowDepth - Whether to indent to show the nesting of the includes. /// \param MSStyle - Whether to print in cl.exe /showIncludes style. void AttachHeaderIncludeGen(Preprocessor &PP, - const std::vector &ExtraHeaders, + const DependencyOutputOptions &DepOpts, bool ShowAllHeaders = false, StringRef OutputPath = "", bool ShowDepth = true, bool MSStyle = false); diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 453e6dc..a57023a 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -360,18 +360,18 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { // Handle generating header include information, if requested. if (DepOpts.ShowHeaderIncludes) - AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps); + AttachHeaderIncludeGen(*PP, DepOpts); if (!DepOpts.HeaderIncludeOutputFile.empty()) { StringRef OutputPath = DepOpts.HeaderIncludeOutputFile; if (OutputPath == "-") OutputPath = ""; - AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps, + AttachHeaderIncludeGen(*PP, DepOpts, /*ShowAllHeaders=*/true, OutputPath, /*ShowDepth=*/false); } if (DepOpts.PrintShowIncludes) { - AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps, + AttachHeaderIncludeGen(*PP, DepOpts, /*ShowAllHeaders=*/true, /*OutputPath=*/"", /*ShowDepth=*/true, /*MSStyle=*/true); } @@ -765,7 +765,7 @@ bool CompilerInstance::InitializeSourceManager( /*SuggestedModule=*/nullptr, /*SkipCache=*/true); // Also add the header to /showIncludes output. if (File) - DepOpts.ExtraDeps.push_back(File->getName()); + DepOpts.ShowIncludesPretendHeader = File->getName(); } if (!File) { Diags.Report(diag::err_fe_error_reading) << InputFile; diff --git a/clang/lib/Frontend/HeaderIncludeGen.cpp b/clang/lib/Frontend/HeaderIncludeGen.cpp index 25b0c4e..818365c 100644 --- a/clang/lib/Frontend/HeaderIncludeGen.cpp +++ b/clang/lib/Frontend/HeaderIncludeGen.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "clang/Frontend/DependencyOutputOptions.h" #include "clang/Frontend/Utils.h" #include "clang/Basic/SourceManager.h" #include "clang/Frontend/FrontendDiagnostic.h" @@ -19,25 +20,23 @@ namespace { class HeaderIncludesCallback : public PPCallbacks { SourceManager &SM; raw_ostream *OutputFile; - const std::vector &ExtraHeaders; + const DependencyOutputOptions &DepOpts; unsigned CurrentIncludeDepth; bool HasProcessedPredefines; bool OwnsOutputFile; bool ShowAllHeaders; bool ShowDepth; bool MSStyle; - bool PrintedExtraHeaders; public: HeaderIncludesCallback(const Preprocessor *PP, bool ShowAllHeaders_, raw_ostream *OutputFile_, - const std::vector &ExtraHeaders, + const DependencyOutputOptions &DepOpts, bool OwnsOutputFile_, bool ShowDepth_, bool MSStyle_) - : SM(PP->getSourceManager()), OutputFile(OutputFile_), - ExtraHeaders(ExtraHeaders), CurrentIncludeDepth(0), - HasProcessedPredefines(false), OwnsOutputFile(OwnsOutputFile_), - ShowAllHeaders(ShowAllHeaders_), ShowDepth(ShowDepth_), - MSStyle(MSStyle_), PrintedExtraHeaders(false) {} + : SM(PP->getSourceManager()), OutputFile(OutputFile_), DepOpts(DepOpts), + CurrentIncludeDepth(0), HasProcessedPredefines(false), + OwnsOutputFile(OwnsOutputFile_), ShowAllHeaders(ShowAllHeaders_), + ShowDepth(ShowDepth_), MSStyle(MSStyle_) {} ~HeaderIncludesCallback() override { if (OwnsOutputFile) @@ -78,10 +77,9 @@ static void PrintHeaderInfo(raw_ostream *OutputFile, const char* Filename, } void clang::AttachHeaderIncludeGen(Preprocessor &PP, - const std::vector &ExtraHeaders, - bool ShowAllHeaders, - StringRef OutputPath, bool ShowDepth, - bool MSStyle) { + const DependencyOutputOptions &DepOpts, + bool ShowAllHeaders, StringRef OutputPath, + bool ShowDepth, bool MSStyle) { raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs(); bool OwnsOutputFile = false; @@ -101,8 +99,15 @@ void clang::AttachHeaderIncludeGen(Preprocessor &PP, } } + // Print header info for extra headers, pretending they were discovered by + // the regular preprocessor. The primary use case is to support proper + // generation of Make / Ninja file dependencies for implicit includes, such + // as sanitizer blacklists. It's only important for cl.exe compatibility, + // the GNU way to generate rules is -M / -MM / -MD / -MMD. + for (auto Header : DepOpts.ExtraDeps) + PrintHeaderInfo(OutputFile, Header.c_str(), ShowDepth, 2, MSStyle); PP.addPPCallbacks(llvm::make_unique( - &PP, ShowAllHeaders, OutputFile, ExtraHeaders, OwnsOutputFile, ShowDepth, + &PP, ShowAllHeaders, OutputFile, DepOpts, OwnsOutputFile, ShowDepth, MSStyle)); } @@ -110,17 +115,6 @@ void HeaderIncludesCallback::FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind NewFileType, FileID PrevFID) { - if (!PrintedExtraHeaders) { - // Print header info for extra headers, pretending they were discovered by - // the regular preprocessor. The primary use case is to support proper - // generation of Make / Ninja file dependencies for implicit includes, such - // as sanitizer blacklists. It's only important for cl.exe compatibility, - // the GNU way to generate rules is -M / -MM / -MD / -MMD. - for (auto Header : ExtraHeaders) - PrintHeaderInfo(OutputFile, Header.c_str(), ShowDepth, 2, MSStyle); - PrintedExtraHeaders = true; - } - // Unless we are exiting a #include, make sure to skip ahead to the line the // #include directive was at. PresumedLoc UserLoc = SM.getPresumedLoc(Loc); @@ -136,8 +130,13 @@ void HeaderIncludesCallback::FileChanged(SourceLocation Loc, // We track when we are done with the predefines by watching for the first // place where we drop back to a nesting depth of 1. - if (CurrentIncludeDepth == 1 && !HasProcessedPredefines) + if (CurrentIncludeDepth == 1 && !HasProcessedPredefines) { + if (!DepOpts.ShowIncludesPretendHeader.empty()) { + PrintHeaderInfo(OutputFile, DepOpts.ShowIncludesPretendHeader.c_str(), + ShowDepth, 2, MSStyle); + } HasProcessedPredefines = true; + } return; } else @@ -150,7 +149,9 @@ void HeaderIncludesCallback::FileChanged(SourceLocation Loc, (ShowAllHeaders && CurrentIncludeDepth > 2)); unsigned IncludeDepth = CurrentIncludeDepth; if (!HasProcessedPredefines) - --IncludeDepth; // Ignore indent from . + --IncludeDepth; // Ignore indent from . + else if (!DepOpts.ShowIncludesPretendHeader.empty()) + ++IncludeDepth; // Pretend inclusion by ShowIncludesPretendHeader. // Dump the header include information we are past the predefines buffer or // are showing all headers and this isn't the magic implicit diff --git a/clang/test/Driver/cl-pch-showincludes.cpp b/clang/test/Driver/cl-pch-showincludes.cpp index 76b7cd1..8115e6b5 100644 --- a/clang/test/Driver/cl-pch-showincludes.cpp +++ b/clang/test/Driver/cl-pch-showincludes.cpp @@ -11,8 +11,7 @@ // RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader2.h /Fp%t.pch /c /Fo%t -- %s \ // RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YC %s // CHECK-YC: Note: including file: {{[^ ]*header2.h}} -// FIXME: header1.h should be indented one more: -// CHECK-YC: Note: including file: {{[^ ]*header1.h}} +// CHECK-YC: Note: including file: {{[^ ]*header1.h}} // CHECK-YC: Note: including file: {{[^ ]*header3.h}} // When using the pch, only the direct include is printed. @@ -35,11 +34,9 @@ // /FI flags before the /Yc arg should be printed, /FI flags after it shouldn't. // RUN: %clang_cl -Werror /showIncludes /I%S/Inputs /Ycheader2.h /FIheader0.h /FIheader2.h /FIheader4.h /Fp%t.pch /c /Fo%t -- %s \ // RUN: | FileCheck --strict-whitespace -check-prefix=CHECK-YCFI %s -// FIXME: The order of the first two lines here must be reversed: -// CHECK-YCFI: Note: including file: {{[^ ]*header2.h}} // CHECK-YCFI: Note: including file: {{[^ ]*header0.h}} -// FIXME: header1.h should be indented one more: -// CHECK-YCFI: Note: including file: {{[^ ]*header1.h}} +// CHECK-YCFI: Note: including file: {{[^ ]*header2.h}} +// CHECK-YCFI: Note: including file: {{[^ ]*header1.h}} // CHECK-YCFI: Note: including file: {{[^ ]*header4.h}} // CHECK-YCFI: Note: including file: {{[^ ]*header3.h}} -- 2.7.4