From: NAKAMURA Takumi Date: Tue, 29 Apr 2014 06:58:59 +0000 (+0000) Subject: Revert r207477 (and r207479), "Check -Werror options during module validation" X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=69c9612d1ae89052fc8e379aeb060c102d842181;p=clang Revert r207477 (and r207479), "Check -Werror options during module validation" It tried to introduce cyclic dependencies. Serialization shouldn't depend on Frontend, since Frontend depends on Serialization. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@207497 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h index 76b2eac9b5..f7f84440a5 100644 --- a/include/clang/Basic/Diagnostic.h +++ b/include/clang/Basic/Diagnostic.h @@ -21,7 +21,6 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" -#include "llvm/ADT/iterator_range.h" #include #include @@ -365,14 +364,6 @@ public: /// \brief Retrieve the diagnostic options. DiagnosticOptions &getDiagnosticOptions() const { return *DiagOpts; } - typedef llvm::iterator_range diag_mapping_range; - - /// \brief Get the current set of diagnostic mappings. - diag_mapping_range getDiagnosticMappings() const { - const DiagState &DS = *GetCurDiagState(); - return diag_mapping_range(DS.begin(), DS.end()); - } - DiagnosticConsumer *getClient() { return Client; } const DiagnosticConsumer *getClient() const { return Client; } diff --git a/include/clang/Basic/DiagnosticSerializationKinds.td b/include/clang/Basic/DiagnosticSerializationKinds.td index be9d2bdbd2..db1ca43327 100644 --- a/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/include/clang/Basic/DiagnosticSerializationKinds.td @@ -39,8 +39,6 @@ def err_pch_langopt_mismatch : Error<"%0 was %select{disabled|enabled}1 in " "PCH file but is currently %select{disabled|enabled}2">; def err_pch_langopt_value_mismatch : Error< "%0 differs in PCH file vs. current file">; -def err_pch_diagopt_mismatch : Error<"%0 is currently enabled, but was not in " - "the PCH file">; def err_pch_version_too_old : Error< "PCH file uses an older PCH format that is no longer supported">; diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index 32bec0b2d7..0d2bfc6797 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/include/clang/Serialization/ASTReader.h @@ -133,9 +133,8 @@ public: /// /// \returns true to indicate the diagnostic options are invalid, or false /// otherwise. - virtual bool - ReadDiagnosticOptions(IntrusiveRefCntPtr DiagOpts, - bool Complain) { + virtual bool ReadDiagnosticOptions(const DiagnosticOptions &DiagOpts, + bool Complain) { return false; } @@ -212,7 +211,7 @@ public: bool ReadLanguageOptions(const LangOptions &LangOpts, bool Complain) override; bool ReadTargetOptions(const TargetOptions &TargetOpts, bool Complain) override; - bool ReadDiagnosticOptions(IntrusiveRefCntPtr DiagOpts, + bool ReadDiagnosticOptions(const DiagnosticOptions &DiagOpts, bool Complain) override; bool ReadFileSystemOptions(const FileSystemOptions &FSOpts, bool Complain) override; @@ -245,8 +244,6 @@ public: bool Complain) override; bool ReadTargetOptions(const TargetOptions &TargetOpts, bool Complain) override; - bool ReadDiagnosticOptions(IntrusiveRefCntPtr DiagOpts, - bool Complain) override; bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, bool Complain, std::string &SuggestedPredefines) override; void ReadCounter(const serialization::ModuleFile &M, unsigned Value) override; diff --git a/lib/Frontend/FrontendActions.cpp b/lib/Frontend/FrontendActions.cpp index a8df7fd1cb..3d65ae32c0 100644 --- a/lib/Frontend/FrontendActions.cpp +++ b/lib/Frontend/FrontendActions.cpp @@ -459,25 +459,6 @@ namespace { return false; } - virtual bool - ReadDiagnosticOptions(IntrusiveRefCntPtr DiagOpts, - bool Complain) override { - Out.indent(2) << "Diagnostic options:\n"; -#define DIAGOPT(Name, Bits, Default) DUMP_BOOLEAN(DiagOpts->Name, #Name); -#define ENUM_DIAGOPT(Name, Type, Bits, Default) \ - Out.indent(4) << #Name << ": " << DiagOpts->get##Name() << "\n"; -#define VALUE_DIAGOPT(Name, Bits, Default) \ - Out.indent(4) << #Name << ": " << DiagOpts->Name << "\n"; -#include "clang/Basic/DiagnosticOptions.def" - - Out.indent(4) << "Warning options:\n"; - for (const std::string &Warning : DiagOpts->Warnings) { - Out.indent(6) << "-W" << Warning << "\n"; - } - - return false; - } - bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, bool Complain) override { Out.indent(2) << "Header search options:\n"; diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index c6aec4592b..834917de1d 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -29,7 +29,6 @@ #include "clang/Basic/TargetOptions.h" #include "clang/Basic/Version.h" #include "clang/Basic/VersionTuple.h" -#include "clang/Frontend/Utils.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/HeaderSearchOptions.h" #include "clang/Lex/MacroInfo.h" @@ -91,7 +90,7 @@ ChainedASTReaderListener::ReadTargetOptions(const TargetOptions &TargetOpts, Second->ReadTargetOptions(TargetOpts, Complain); } bool ChainedASTReaderListener::ReadDiagnosticOptions( - IntrusiveRefCntPtr DiagOpts, bool Complain) { + const DiagnosticOptions &DiagOpts, bool Complain) { return First->ReadDiagnosticOptions(DiagOpts, Complain) || Second->ReadDiagnosticOptions(DiagOpts, Complain); } @@ -292,120 +291,6 @@ namespace { DeclsMap; } -static bool checkDiagnosticGroupMappings(DiagnosticsEngine &StoredDiags, - DiagnosticsEngine &Diags, - bool Complain) { - typedef DiagnosticsEngine::Level Level; - - // Check current mappings for new -Werror mappings, and the stored mappings - // for cases that were explicitly mapped to *not* be errors that are now - // errors because of options like -Werror. - DiagnosticsEngine *MappingSources[] = { &Diags, &StoredDiags }; - - for (DiagnosticsEngine *MappingSource : MappingSources) { - for (auto DiagIDMappingPair : MappingSource->getDiagnosticMappings()) { - diag::kind DiagID = DiagIDMappingPair.first; - Level CurLevel = Diags.getDiagnosticLevel(DiagID, SourceLocation()); - if (CurLevel < DiagnosticsEngine::Error) - continue; // not significant - Level StoredLevel = - StoredDiags.getDiagnosticLevel(DiagID, SourceLocation()); - if (StoredLevel < DiagnosticsEngine::Error) { - if (Complain) - Diags.Report(diag::err_pch_diagopt_mismatch) << "-Werror=" + - Diags.getDiagnosticIDs()->getWarningOptionForDiag(DiagID).str(); - return true; - } - } - } - - return false; -} - -static DiagnosticsEngine::ExtensionHandling -isExtHandlingFromDiagsError(DiagnosticsEngine &Diags) { - DiagnosticsEngine::ExtensionHandling Ext = - Diags.getExtensionHandlingBehavior(); - if (Ext == DiagnosticsEngine::Ext_Warn && Diags.getWarningsAsErrors()) - Ext = DiagnosticsEngine::Ext_Error; - return Ext; -} - -static bool checkDiagnosticMappings(DiagnosticsEngine &StoredDiags, - DiagnosticsEngine &Diags, - bool IsSystem, bool Complain) { - // Top-level options - if (IsSystem) { - if (Diags.getSuppressSystemWarnings()) - return false; - // If -Wsystem-headers was not enabled before, be conservative - if (StoredDiags.getSuppressSystemWarnings()) { - if (Complain) - Diags.Report(diag::err_pch_diagopt_mismatch) << "-Wsystem-headers"; - return true; - } - } - - if (Diags.getWarningsAsErrors() && !StoredDiags.getWarningsAsErrors()) { - if (Complain) - Diags.Report(diag::err_pch_diagopt_mismatch) << "-Werror"; - return true; - } - - if (Diags.getWarningsAsErrors() && Diags.getEnableAllWarnings() && - !StoredDiags.getEnableAllWarnings()) { - if (Complain) - Diags.Report(diag::err_pch_diagopt_mismatch) << "-Weverything -Werror"; - return true; - } - - if (isExtHandlingFromDiagsError(Diags) && - !isExtHandlingFromDiagsError(StoredDiags)) { - if (Complain) - Diags.Report(diag::err_pch_diagopt_mismatch) << "-pedantic-errors"; - return true; - } - - return checkDiagnosticGroupMappings(StoredDiags, Diags, Complain); -} - -bool PCHValidator::ReadDiagnosticOptions( - IntrusiveRefCntPtr DiagOpts, bool Complain) { - DiagnosticsEngine &ExistingDiags = PP.getDiagnostics(); - IntrusiveRefCntPtr DiagIDs(ExistingDiags.getDiagnosticIDs()); - IntrusiveRefCntPtr Diags( - new DiagnosticsEngine(DiagIDs, DiagOpts.getPtr())); - // This should never fail, because we would have processed these options - // before writing them to an ASTFile. - ProcessWarningOptions(*Diags, *DiagOpts, /*Report*/false); - - ModuleManager &ModuleMgr = Reader.getModuleManager(); - assert(ModuleMgr.size() >= 1 && "what ASTFile is this then"); - - // If the original import came from a file explicitly generated by the user, - // don't check the diagnostic mappings. - // FIXME: currently this is approximated by checking whether this is not a - // module import. - // Note: ModuleMgr.rbegin() may not be the current module, but it must be in - // the transitive closure of its imports, since unrelated modules cannot be - // imported until after this module finishes validation. - ModuleFile *TopImport = *ModuleMgr.rbegin(); - while (!TopImport->ImportedBy.empty()) - TopImport = TopImport->ImportedBy[0]; - if (TopImport->Kind != MK_Module) - return false; - - StringRef ModuleName = TopImport->ModuleName; - assert(!ModuleName.empty() && "diagnostic options read before module name"); - - Module *M = PP.getHeaderSearchInfo().lookupModule(ModuleName); - assert(M && "missing module"); - - // FIXME: if the diagnostics are incompatible, save a DiagnosticOptions that - // contains the union of their flags. - return checkDiagnosticMappings(*Diags, ExistingDiags, M->IsSystem, Complain); -} - /// \brief Collect the macro definitions provided by the given preprocessor /// options. static void collectMacroDefinitions(const PreprocessorOptions &PPOpts, @@ -2383,11 +2268,11 @@ ASTReader::ReadControlBlock(ModuleFile &F, } case DIAGNOSTIC_OPTIONS: { - bool Complain = (ClientLoadCapabilities & ARR_OutOfDate)==0; + bool Complain = (ClientLoadCapabilities & ARR_ConfigurationMismatch)==0; if (Listener && &F == *ModuleMgr.begin() && ParseDiagnosticOptions(Record, Complain, *Listener) && - !DisableValidation) - return OutOfDate; + !DisableValidation && !AllowConfigurationMismatch) + return ConfigurationMismatch; break; } @@ -4596,15 +4481,15 @@ bool ASTReader::ParseTargetOptions(const RecordData &Record, bool ASTReader::ParseDiagnosticOptions(const RecordData &Record, bool Complain, ASTReaderListener &Listener) { - IntrusiveRefCntPtr DiagOpts(new DiagnosticOptions); + DiagnosticOptions DiagOpts; unsigned Idx = 0; -#define DIAGOPT(Name, Bits, Default) DiagOpts->Name = Record[Idx++]; +#define DIAGOPT(Name, Bits, Default) DiagOpts.Name = Record[Idx++]; #define ENUM_DIAGOPT(Name, Type, Bits, Default) \ - DiagOpts->set##Name(static_cast(Record[Idx++])); + DiagOpts.set##Name(static_cast(Record[Idx++])); #include "clang/Basic/DiagnosticOptions.def" for (unsigned N = Record[Idx++]; N; --N) { - DiagOpts->Warnings.push_back(ReadString(Record, Idx)); + DiagOpts.Warnings.push_back(ReadString(Record, Idx)); } return Listener.ReadDiagnosticOptions(DiagOpts, Complain); diff --git a/test/Modules/Werror-Wsystem-headers.m b/test/Modules/Werror-Wsystem-headers.m deleted file mode 100644 index c4cd1a6378..0000000000 --- a/test/Modules/Werror-Wsystem-headers.m +++ /dev/null @@ -1,23 +0,0 @@ -// REQUIRES: shell -// RUN: rm -rf %t -// RUN: rm -rf %t-saved -// RUN: mkdir %t-saved - -// Initial module build -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -isysroot %S/Inputs/System -triple x86_64-apple-darwin10 -fsyntax-only %s -verify -// RUN: cp %t/cstd.pcm %t-saved/cstd.pcm - -// Even with -Werror don't rebuild a system module -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -isysroot %S/Inputs/System -triple x86_64-apple-darwin10 -fsyntax-only %s -verify -Werror -// RUN: diff %t/cstd.pcm %t-saved/cstd.pcm - -// Unless -Wsystem-headers is on -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -isysroot %S/Inputs/System -triple x86_64-apple-darwin10 -fsyntax-only %s -verify \ -// RUN: -Werror=unused -Wsystem-headers -// RUN: not diff %t/cstd.pcm %t-saved/cstd.pcm - -// expected-no-diagnostics -@import cstd; diff --git a/test/Modules/Werror.m b/test/Modules/Werror.m deleted file mode 100644 index 94a98a5a19..0000000000 --- a/test/Modules/Werror.m +++ /dev/null @@ -1,75 +0,0 @@ -// REQUIRES: shell -// RUN: rm -rf %t -// RUN: rm -rf %t-saved -// RUN: mkdir -p %t-saved - -// Initial module build (-Werror=header-guard) -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -Werror=header-guard -// RUN: cp %t/Module.pcm %t-saved/Module.pcm - -// Building with looser -Werror options does not rebuild -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella -// RUN: diff %t/Module.pcm %t-saved/Module.pcm - -// Make the build more restricted (-Werror) -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -Werror -Wno-incomplete-umbrella -// RUN: not diff %t/Module.pcm %t-saved/Module.pcm -// RUN: cp %t/Module.pcm %t-saved/Module.pcm - -// Ensure -Werror=header-guard is less strict than -Werror -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -Werror=header-guard -Wno-incomplete-umbrella -// RUN: diff %t/Module.pcm %t-saved/Module.pcm - -// But -Werror=unused is not, because some of its diags are DefaultIgnore -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -Werror=unused -// RUN: not diff %t/Module.pcm %t-saved/Module.pcm -// RUN: cp %t/Module.pcm %t-saved/Module.pcm - -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -Werror -Wno-incomplete-umbrella - -// FIXME: when rebuilding the module, take the union of the diagnostic options -// so that we don't need to rebuild here -// RUN-DISABLED: diff %t/Module.pcm %t-saved/Module.pcm - -// -Wno-everything, -Werror -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -Wno-everything -Wall -Werror -// RUN: cp %t/Module.pcm %t-saved/Module.pcm -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -Wall -Werror -// RUN: not diff %t/Module.pcm %t-saved/Module.pcm - -// -pedantic, -Werror is not compatible with -Wall -Werror -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -Werror -pedantic -// RUN: not diff %t/Module.pcm %t-saved/Module.pcm -// RUN: cp %t/Module.pcm %t-saved/Module.pcm - -// -pedantic-errors is less strict that -pedantic, -Werror -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -pedantic-errors -// RUN: diff %t/Module.pcm %t-saved/Module.pcm - -// -Wsystem-headers does not affect non-system modules -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fdisable-module-hash \ -// RUN: -F %S/Inputs -fsyntax-only %s -verify -Wno-incomplete-umbrella \ -// RUN: -pedantic-errors -Wsystem-headers -// RUN: diff %t/Module.pcm %t-saved/Module.pcm - -// expected-no-diagnostics -@import Module; diff --git a/test/Modules/module_file_info.m b/test/Modules/module_file_info.m index 13e086ee48..9f1ff2221b 100644 --- a/test/Modules/module_file_info.m +++ b/test/Modules/module_file_info.m @@ -2,7 +2,7 @@ @import DependsOnModule; // RUN: rm -rf %t -// RUN: %clang_cc1 -w -Wunused -fmodules -fdisable-module-hash -fmodules-cache-path=%t -F %S/Inputs -DBLARG -DWIBBLE=WOBBLE %s +// RUN: %clang_cc1 -w -fmodules -fdisable-module-hash -fmodules-cache-path=%t -F %S/Inputs -DBLARG -DWIBBLE=WOBBLE %s // RUN: %clang_cc1 -module-file-info %t/DependsOnModule.pcm | FileCheck %s // CHECK: Generated by this Clang: @@ -21,11 +21,6 @@ // CHECK: ABI: // CHECK: Linker version: -// CHECK: Diagnostic options: -// CHECK: IgnoreWarnings: Yes -// CHECK: Warning options: -// CHECK: -Wunused - // CHECK: Header search options: // CHECK: System root [-isysroot=]: '/' // CHECK: Use builtin include directories [-nobuiltininc]: Yes diff --git a/test/Modules/resolution-change.m b/test/Modules/resolution-change.m index 011782eec2..a69014c6ae 100644 --- a/test/Modules/resolution-change.m +++ b/test/Modules/resolution-change.m @@ -6,9 +6,6 @@ // Use the PCH with the same header search options; should be fine // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs/modules-with-same-name/DependsOnA -I %S/Inputs/modules-with-same-name/path1/A -include-pch %t-A.pch %s -fsyntax-only -Werror -// Different -W options are ok -// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -I %S/Inputs/modules-with-same-name/DependsOnA -I %S/Inputs/modules-with-same-name/path1/A -include-pch %t-A.pch %s -fsyntax-only -Werror -Wauto-import - // Use the PCH with no way to resolve DependsOnA // RUN: not %clang_cc1 -fmodules -fmodules-cache-path=%t -include-pch %t-A.pch %s -fsyntax-only 2>&1 | FileCheck -check-prefix=CHECK-NODOA %s // CHECK-NODOA: module 'DependsOnA' imported by AST file '{{.*A.pch}}' not found