From 7ef500107665d340f1f138d626ac2d9d01bd2614 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Wed, 4 May 2022 16:48:48 +0200 Subject: [PATCH] ClangCodeModel: Speed up compilation db generation We needlessly re-evaluated the same compiler options again and again for all files in a project part. Now we only do the actual file-related work per file. Along the way, we dissolved some unneeded classes and made CompilerOptionsBuilder non- polymorphic. Change-Id: I9710d641a57032936cc0812515974dbc91676c8c Reviewed-by: David Schulz --- .../clangcodemodel/clangcodemodelplugin.cpp | 6 +- src/plugins/clangcodemodel/clangdclient.cpp | 23 +- .../clangmodelmanagersupport.cpp | 4 +- src/plugins/clangcodemodel/clangutils.cpp | 228 +++++------------- src/plugins/clangcodemodel/clangutils.h | 11 +- .../cppeditor/clangdiagnosticconfig.cpp | 2 +- src/plugins/cppeditor/clangdiagnosticconfig.h | 2 +- .../cppeditor/compileroptionsbuilder.cpp | 10 +- .../cppeditor/compileroptionsbuilder.h | 9 +- 9 files changed, 104 insertions(+), 191 deletions(-) diff --git a/src/plugins/clangcodemodel/clangcodemodelplugin.cpp b/src/plugins/clangcodemodel/clangcodemodelplugin.cpp index 46761fc2353..3c28e8400bb 100644 --- a/src/plugins/clangcodemodel/clangcodemodelplugin.cpp +++ b/src/plugins/clangcodemodel/clangcodemodelplugin.cpp @@ -80,11 +80,13 @@ void ClangCodeModelPlugin::generateCompilationDB() if (!projectInfo) return; + const CppEditor::ClangDiagnosticConfig warningsConfig + = warningsConfigForProject(target->project()); QFuture task = QtConcurrent::run(&Internal::generateCompilationDB, projectInfo, projectInfo->buildRoot(), CompilationDbPurpose::Project, - warningsConfigForProject(target->project()), - optionsForProject(target->project())); + warningsConfig, + optionsForProject(target->project(), warningsConfig)); Core::ProgressManager::addTask(task, tr("Generating Compilation DB"), "generate compilation db"); m_generatorWatcher.setFuture(task); } diff --git a/src/plugins/clangcodemodel/clangdclient.cpp b/src/plugins/clangcodemodel/clangdclient.cpp index 9379335089f..7482b1fa07f 100644 --- a/src/plugins/clangcodemodel/clangdclient.cpp +++ b/src/plugins/clangcodemodel/clangdclient.cpp @@ -1453,15 +1453,12 @@ private: }; static void addToCompilationDb(QJsonObject &cdb, - const CppEditor::ClangDiagnosticConfig &projectWarnings, + const CppEditor::CompilerOptionsBuilder &optionsBuilder, const QStringList &projectOptions, - const CppEditor::ProjectPart::ConstPtr &projectPart, const Utils::FilePath &workingDir, const Utils::FilePath &sourceFile) { - // TODO: Do we really need to re-calculate the project part options per source file? - QStringList args = createClangOptions(*projectPart, sourceFile.toString(), - projectWarnings, projectOptions); + QStringList args = clangOptionsForFile(optionsBuilder, sourceFile.toString(), projectOptions); // TODO: clangd seems to apply some heuristics depending on what we put here. // Should we make use of them or keep using our own? @@ -1493,9 +1490,12 @@ ClangdClient::ClangdClient(Project *project, const Utils::FilePath &jsonDbDir) setQuickFixAssistProvider(new ClangdQuickFixProvider(this)); if (!project) { QJsonObject initOptions; - const QStringList clangOptions = createClangOptions( - *CppEditor::CppModelManager::instance()->fallbackProjectPart(), {}, - warningsConfigForProject(nullptr), optionsForProject(nullptr)); + const CppEditor::ClangDiagnosticConfig warningsConfig = warningsConfigForProject(nullptr); + CppEditor::CompilerOptionsBuilder optionsBuilder = clangOptionsBuilder( + *CppEditor::CppModelManager::instance()->fallbackProjectPart(), + warningsConfig); + const QStringList clangOptions = clangOptionsForFile( + optionsBuilder, {}, optionsForProject(nullptr, warningsConfig)); initOptions.insert("fallbackFlags", QJsonArray::fromStringList(clangOptions)); setInitializationOptions(initOptions); } @@ -1913,8 +1913,11 @@ void ClangdClient::updateParserConfig(const Utils::FilePath &filePath, if (!projectPart) return; QJsonObject cdbChanges; - addToCompilationDb(cdbChanges, warningsConfigForProject(project()), - optionsForProject(project()), projectPart, filePath.parentDir(), filePath); + const CppEditor::ClangDiagnosticConfig warningsConfig = warningsConfigForProject(project()); + CppEditor::CompilerOptionsBuilder optionsBuilder = clangOptionsBuilder(*projectPart, + warningsConfig); + addToCompilationDb(cdbChanges, optionsBuilder, optionsForProject(project(), warningsConfig), + filePath.parentDir(), filePath); QJsonObject settings; addCompilationDb(settings, cdbChanges); DidChangeConfigurationParams configChangeParams; diff --git a/src/plugins/clangcodemodel/clangmodelmanagersupport.cpp b/src/plugins/clangcodemodel/clangmodelmanagersupport.cpp index 55687d4bedd..5c3d7eff8cd 100644 --- a/src/plugins/clangcodemodel/clangmodelmanagersupport.cpp +++ b/src/plugins/clangcodemodel/clangmodelmanagersupport.cpp @@ -419,10 +419,10 @@ void ClangModelManagerSupport::updateLanguageClient( }); }); + const ClangDiagnosticConfig warningsConfig = warningsConfigForProject(project); auto future = Utils::runAsync(&Internal::generateCompilationDB, projectInfo, jsonDbDir, CompilationDbPurpose::CodeModel, - warningsConfigForProject(project), - optionsForProject(project)); + warningsConfig, optionsForProject(project, warningsConfig)); generatorWatcher->setFuture(future); m_generatorSynchronizer.addFuture(future); } diff --git a/src/plugins/clangcodemodel/clangutils.cpp b/src/plugins/clangcodemodel/clangutils.cpp index 9254e204408..05664bbc21b 100644 --- a/src/plugins/clangcodemodel/clangutils.cpp +++ b/src/plugins/clangcodemodel/clangutils.cpp @@ -65,48 +65,6 @@ using namespace Utils; namespace ClangCodeModel { namespace Internal { -class LibClangOptionsBuilder final : public CompilerOptionsBuilder -{ -public: - LibClangOptionsBuilder(const ProjectPart &projectPart, - UseBuildSystemWarnings useBuildSystemWarnings) - : CompilerOptionsBuilder(projectPart, - UseSystemHeader::No, - UseTweakedHeaderPaths::Yes, - UseLanguageDefines::No, - useBuildSystemWarnings, - QString(CLANG_VERSION), - FilePath(CLANG_INCLUDE_DIR)) - { - } - - void addProjectMacros() final - { - addMacros({ProjectExplorer::Macro("Q_CREATOR_RUN", "1")}); - CompilerOptionsBuilder::addProjectMacros(); - } - - void addExtraOptions() final - { - addDummyUiHeaderOnDiskIncludePath(); - add("-fmessage-length=0", /*gccOnlyOption=*/true); - add("-fdiagnostics-show-note-include-stack", /*gccOnlyOption=*/true); - add("-fretain-comments-from-system-headers", /*gccOnlyOption=*/true); - add("-fmacro-backtrace-limit=0"); - add("-ferror-limit=1000"); - } - -private: - void addDummyUiHeaderOnDiskIncludePath() - { - const QString path = ClangModelManagerSupport::instance()->dummyUiHeaderOnDiskDirPath(); - if (!path.isEmpty()) { - prepend(QDir::toNativeSeparators(path)); - prepend("-I"); - } - } -}; - ProjectPart::ConstPtr projectPartForFile(const QString &filePath) { if (const auto parser = CppEditor::BaseEditorDocumentParser::get(filePath)) @@ -225,10 +183,10 @@ static QStringList projectPartArguments(const ProjectPart &projectPart) static QJsonObject createFileObject(const FilePath &buildDir, const QStringList &arguments, + const CompilerOptionsBuilder &optionsBuilder, const ProjectPart &projectPart, const ProjectFile &projFile, CompilationDbPurpose purpose, - const ClangDiagnosticConfig &warningsConfig, const QStringList &projectOptions) { QJsonObject fileObject; @@ -254,9 +212,8 @@ static QJsonObject createFileObject(const FilePath &buildDir, args.append(langOptionPart); } } else { - // TODO: Do we really need to re-calculate the project part options per source file? - args = QJsonArray::fromStringList(createClangOptions(projectPart, projFile.path, - warningsConfig, projectOptions)); + args = QJsonArray::fromStringList(clangOptionsForFile(optionsBuilder, projFile.path, + projectOptions)); args.prepend("clang"); // TODO: clang-cl for MSVC targets? Does it matter at all what we put here? } @@ -288,11 +245,13 @@ GenerateCompilationDbResult generateCompilationDB(const CppEditor::ProjectInfo:: for (ProjectPart::ConstPtr projectPart : projectInfo->projectParts()) { QStringList args; + const CompilerOptionsBuilder optionsBuilder = clangOptionsBuilder(*projectPart, + warningsConfig); if (purpose == CompilationDbPurpose::Project) args = projectPartArguments(*projectPart); for (const ProjectFile &projFile : projectPart->files) { - const QJsonObject json = createFileObject(baseDir, args, *projectPart, projFile, - purpose, warningsConfig, projectOptions); + const QJsonObject json = createFileObject(baseDir, args, optionsBuilder, *projectPart, + projFile, purpose, projectOptions); if (compileCommandsFile.size() > 1) compileCommandsFile.write(","); compileCommandsFile.write('\n' + QJsonDocument(json).toJson().trimmed()); @@ -372,121 +331,25 @@ static ClangProjectSettings &getProjectSettings(ProjectExplorer::Project *projec return ClangModelManagerSupport::instance()->projectSettings(project); } -// TODO: Can we marry this with CompilerOptionsBuilder? -class FileOptionsBuilder -{ -public: - FileOptionsBuilder(const QString &filePath, const CppEditor::ProjectPart &projectPart, - const ClangDiagnosticConfig &warningsConfig, - const QStringList &projectOptions) - : m_filePath(filePath) - , m_projectPart(projectPart) - , m_warningsConfig(warningsConfig) - , m_builder(projectPart) - { - // Determine the driver mode from toolchain and flags. - m_builder.evaluateCompilerFlags(); - m_isClMode = m_builder.isClStyle(); - - addLanguageOptions(); - addGlobalDiagnosticOptions(); // Before addDiagnosticOptions() so users still can overwrite. - addDiagnosticOptions(); - m_options.append(projectOptions); - addPrecompiledHeaderOptions(); - } - - const QStringList &options() const { return m_options; } - CppEditor::UseBuildSystemWarnings useBuildSystemWarnings() const - { - return m_useBuildSystemWarnings; - } - -private: - void addLanguageOptions() - { - // Determine file kind with respect to ambiguous headers. - CppEditor::ProjectFile::Kind fileKind = CppEditor::ProjectFile::Unclassified; - if (!m_filePath.isEmpty()) - fileKind = CppEditor::ProjectFile::classify(m_filePath); - if (fileKind == CppEditor::ProjectFile::AmbiguousHeader) { - fileKind = m_projectPart.languageVersion <= ::Utils::LanguageVersion::LatestC - ? CppEditor::ProjectFile::CHeader - : CppEditor::ProjectFile::CXXHeader; - } - - m_builder.reset(); - m_builder.updateFileLanguage(fileKind); - - m_options.append(m_builder.options()); - } - - void addDiagnosticOptions() - { - addDiagnosticOptionsForConfig(m_warningsConfig); - } - - void addDiagnosticOptionsForConfig(const CppEditor::ClangDiagnosticConfig &diagnosticConfig) - { - m_useBuildSystemWarnings = diagnosticConfig.useBuildSystemWarnings() - ? CppEditor::UseBuildSystemWarnings::Yes - : CppEditor::UseBuildSystemWarnings::No; - - const QStringList options = m_isClMode - ? CppEditor::clangArgsForCl(diagnosticConfig.clangOptions()) - : diagnosticConfig.clangOptions(); - m_options.append(options); - } - - void addGlobalDiagnosticOptions() - { - m_options += CppEditor::ClangDiagnosticConfigsModel::globalDiagnosticOptions(); - } - - void addPrecompiledHeaderOptions() - { - using namespace CppEditor; - - if (getPchUsage() == UsePrecompiledHeaders::No) - return; - - if (m_projectPart.precompiledHeaders.contains(m_filePath)) - return; - - m_builder.reset(); - m_builder.addPrecompiledHeaderOptions(UsePrecompiledHeaders::Yes); - - m_options.append(m_builder.options()); - } - -private: - const QString &m_filePath; - const CppEditor::ProjectPart &m_projectPart; - const ClangDiagnosticConfig &m_warningsConfig; - - CppEditor::UseBuildSystemWarnings m_useBuildSystemWarnings = CppEditor::UseBuildSystemWarnings::No; - CppEditor::CompilerOptionsBuilder m_builder; - bool m_isClMode = false; - QStringList m_options; -}; } // namespace -QStringList createClangOptions(const ProjectPart &projectPart, const QString &filePath, - const ClangDiagnosticConfig &warningsConfig, - const QStringList &projectOptions) +QStringList clangOptionsForFile(CompilerOptionsBuilder optionsBuilder, + const QString &filePath, const QStringList &projectOptions) { - const FileOptionsBuilder fileOptions(filePath, projectPart, warningsConfig, projectOptions); - LibClangOptionsBuilder optionsBuilder(projectPart, fileOptions.useBuildSystemWarnings()); - const QStringList projectPartOptions = optionsBuilder.build(CppEditor::ProjectFile::Unsupported, - UsePrecompiledHeaders::No); - - // FIXME: Sanitize FileOptionsBuilder instead. - QStringList fileArgs = fileOptions.options(); - if (projectPartOptions.contains(QLatin1String("-TP"))) - fileArgs.removeAll(QLatin1String("/TP")); - if (projectPartOptions.contains(QLatin1String("-TC"))) - fileArgs.removeAll(QLatin1String("/TC")); - - return projectPartOptions + fileArgs; + ProjectFile::Kind fileKind = filePath.isEmpty() + ? ProjectFile::Unclassified : ProjectFile::classify(filePath); + if (fileKind == ProjectFile::AmbiguousHeader) { + fileKind = optionsBuilder.projectPart().languageVersion <= LanguageVersion::LatestC + ? ProjectFile::CHeader : ProjectFile::CXXHeader; + } + auto pchUsage = getPchUsage(); + if (pchUsage == UsePrecompiledHeaders::Yes + && optionsBuilder.projectPart().precompiledHeaders.contains(filePath)) { + pchUsage = UsePrecompiledHeaders::No; + } + optionsBuilder.updateFileLanguage(fileKind); + optionsBuilder.addPrecompiledHeaderOptions(pchUsage); + return projectOptions + optionsBuilder.options(); } ClangDiagnosticConfig warningsConfigForProject(Project *project) @@ -505,11 +368,16 @@ ClangDiagnosticConfig warningsConfigForProject(Project *project) return CppEditor::codeModelSettings()->clangDiagnosticConfig(); } -const QStringList optionsForProject(ProjectExplorer::Project *project) +const QStringList optionsForProject(ProjectExplorer::Project *project, + const ClangDiagnosticConfig &warningsConfig) { - if (project) - return getProjectSettings(project).commandLineOptions(); - return ClangProjectSettings::globalCommandLineOptions(); + QStringList options = ClangDiagnosticConfigsModel::globalDiagnosticOptions(); + if (!warningsConfig.useBuildSystemWarnings()) { + options += project + ? getProjectSettings(project).commandLineOptions() + : ClangProjectSettings::globalCommandLineOptions(); + } + return options; } // 7.3.3: using typename(opt) nested-name-specifier unqualified-id ; @@ -548,5 +416,37 @@ QString textUntilPreviousStatement(TextEditor::TextDocumentManipulatorInterface return manipulator.textAt(endPosition, startPosition - endPosition); } +CompilerOptionsBuilder clangOptionsBuilder(const ProjectPart &projectPart, + const ClangDiagnosticConfig &warningsConfig) +{ + const auto useBuildSystemWarnings = warningsConfig.useBuildSystemWarnings() + ? UseBuildSystemWarnings::Yes + : UseBuildSystemWarnings::No; + CompilerOptionsBuilder optionsBuilder(projectPart, UseSystemHeader::No, + UseTweakedHeaderPaths::Yes, UseLanguageDefines::No, + useBuildSystemWarnings, QString(CLANG_VERSION), + FilePath(CLANG_INCLUDE_DIR)); + optionsBuilder.provideAdditionalMacros({ProjectExplorer::Macro("Q_CREATOR_RUN", "1")}); + optionsBuilder.build(ProjectFile::Unclassified, UsePrecompiledHeaders::No); + const QString uiIncludePath + = ClangModelManagerSupport::instance()->dummyUiHeaderOnDiskDirPath(); + if (!uiIncludePath.isEmpty()) { + optionsBuilder.prepend(QDir::toNativeSeparators(uiIncludePath)); + optionsBuilder.prepend("-I"); + } + optionsBuilder.add("-fmessage-length=0", /*gccOnlyOption=*/true); + optionsBuilder.add("-fdiagnostics-show-note-include-stack", /*gccOnlyOption=*/true); + optionsBuilder.add("-fretain-comments-from-system-headers", /*gccOnlyOption=*/true); + optionsBuilder.add("-fmacro-backtrace-limit=0"); + optionsBuilder.add("-ferror-limit=1000"); + + if (useBuildSystemWarnings == UseBuildSystemWarnings::No) { + for (const QString &opt : warningsConfig.clangOptions()) + optionsBuilder.add(opt, true); + } + + return optionsBuilder; +} + } // namespace Internal } // namespace Clang diff --git a/src/plugins/clangcodemodel/clangutils.h b/src/plugins/clangcodemodel/clangutils.h index 761c318037a..3122813456a 100644 --- a/src/plugins/clangcodemodel/clangutils.h +++ b/src/plugins/clangcodemodel/clangutils.h @@ -57,11 +57,14 @@ CppEditor::CppEditorDocumentHandle *cppDocument(const QString &filePath); void setLastSentDocumentRevision(const QString &filePath, uint revision); CppEditor::ClangDiagnosticConfig warningsConfigForProject(ProjectExplorer::Project *project); -const QStringList optionsForProject(ProjectExplorer::Project *project); +const QStringList optionsForProject(ProjectExplorer::Project *project, + const CppEditor::ClangDiagnosticConfig &warningsConfig); -QStringList createClangOptions(const CppEditor::ProjectPart &projectPart, const QString &filePath, - const CppEditor::ClangDiagnosticConfig &warningsConfig, - const QStringList &projectOptions); +CppEditor::CompilerOptionsBuilder clangOptionsBuilder( + const CppEditor::ProjectPart &projectPart, + const CppEditor::ClangDiagnosticConfig &warningsConfig); +QStringList clangOptionsForFile(CppEditor::CompilerOptionsBuilder optionsBuilder, + const QString &filePath, const QStringList &projectOptions); CppEditor::ProjectPart::ConstPtr projectPartForFile(const QString &filePath); CppEditor::ProjectPart::ConstPtr projectPartForFileBasedOnProcessor(const QString &filePath); diff --git a/src/plugins/cppeditor/clangdiagnosticconfig.cpp b/src/plugins/cppeditor/clangdiagnosticconfig.cpp index a1aed99a5ce..0dcbbe24f80 100644 --- a/src/plugins/cppeditor/clangdiagnosticconfig.cpp +++ b/src/plugins/cppeditor/clangdiagnosticconfig.cpp @@ -52,7 +52,7 @@ void ClangDiagnosticConfig::setDisplayName(const QString &displayName) m_displayName = displayName; } -QStringList ClangDiagnosticConfig::clangOptions() const +const QStringList ClangDiagnosticConfig::clangOptions() const { return m_clangOptions; } diff --git a/src/plugins/cppeditor/clangdiagnosticconfig.h b/src/plugins/cppeditor/clangdiagnosticconfig.h index 6536f0dd902..ced20ca8a35 100644 --- a/src/plugins/cppeditor/clangdiagnosticconfig.h +++ b/src/plugins/cppeditor/clangdiagnosticconfig.h @@ -53,7 +53,7 @@ public: bool isReadOnly() const; void setIsReadOnly(bool isReadOnly); - QStringList clangOptions() const; + const QStringList clangOptions() const; void setClangOptions(const QStringList &options); bool useBuildSystemWarnings() const; diff --git a/src/plugins/cppeditor/compileroptionsbuilder.cpp b/src/plugins/cppeditor/compileroptionsbuilder.cpp index f92975c8dd7..e3b5ad82b0f 100644 --- a/src/plugins/cppeditor/compileroptionsbuilder.cpp +++ b/src/plugins/cppeditor/compileroptionsbuilder.cpp @@ -120,8 +120,6 @@ CompilerOptionsBuilder::CompilerOptionsBuilder(const ProjectPart &projectPart, { } -CompilerOptionsBuilder::~CompilerOptionsBuilder() = default; - QStringList CompilerOptionsBuilder::build(ProjectFile::Kind fileKind, UsePrecompiledHeaders usePrecompiledHeaders) { @@ -158,14 +156,17 @@ QStringList CompilerOptionsBuilder::build(ProjectFile::Kind fileKind, addHeaderPathOptions(); - addExtraOptions(); - insertWrappedQtHeaders(); insertWrappedMingwHeaders(); return options(); } +void CompilerOptionsBuilder::provideAdditionalMacros(const ProjectExplorer::Macros ¯os) +{ + m_additionalMacros = macros; +} + void CompilerOptionsBuilder::add(const QString &arg, bool gccOnlyOption) { add(QStringList{arg}, gccOnlyOption); @@ -413,6 +414,7 @@ void CompilerOptionsBuilder::addProjectMacros() } addMacros(m_projectPart.projectMacros); + addMacros(m_additionalMacros); } void CompilerOptionsBuilder::addMacros(const Macros ¯os) diff --git a/src/plugins/cppeditor/compileroptionsbuilder.h b/src/plugins/cppeditor/compileroptionsbuilder.h index 6ae97323354..3e8c540c740 100644 --- a/src/plugins/cppeditor/compileroptionsbuilder.h +++ b/src/plugins/cppeditor/compileroptionsbuilder.h @@ -52,13 +52,13 @@ public: UseBuildSystemWarnings useBuildSystemWarnings = UseBuildSystemWarnings::No, const QString &clangVersion = {}, const Utils::FilePath &clangIncludeDirectory = {}); - virtual ~CompilerOptionsBuilder(); QStringList build(ProjectFile::Kind fileKind, UsePrecompiledHeaders usePrecompiledHeaders); QStringList options() const { return m_options; } // Add options based on project part - virtual void addProjectMacros(); + void provideAdditionalMacros(const ProjectExplorer::Macros ¯os); + void addProjectMacros(); void addSyntaxOnly(); void addWordWidth(); void addHeaderPathOptions(); @@ -90,7 +90,6 @@ public: void add(const QString &arg, bool gccOnlyOption = false); void prepend(const QString &arg); void add(const QStringList &args, bool gccOnlyOptions = false); - virtual void addExtraOptions() {} static UseToolchainMacros useToolChainMacros(); void reset(); @@ -98,6 +97,8 @@ public: void evaluateCompilerFlags(); bool isClStyle() const; + const ProjectPart &projectPart() const { return m_projectPart; } + private: void addIncludeDirOptionForPath(const ProjectExplorer::HeaderPath &path); bool excludeDefineDirective(const ProjectExplorer::Macro ¯o) const; @@ -118,6 +119,8 @@ private: const QString m_clangVersion; const Utils::FilePath m_clangIncludeDirectory; + ProjectExplorer::Macros m_additionalMacros; + struct { QStringList flags; bool isLanguageVersionSpecified = false;