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 <david.schulz@qt.io>

src/plugins/clangcodemodel/clangcodemodelplugin.cpp

@@ -80,11 +80,13 @@ void ClangCodeModelPlugin::generateCompilationDB()
     if (!projectInfo)
         return;
 
+    const CppEditor::ClangDiagnosticConfig warningsConfig
+            = warningsConfigForProject(target->project());
     QFuture<GenerateCompilationDbResult> task
             = QtConcurrent::run(&Internal::generateCompilationDB, projectInfo,
                                 projectInfo->buildRoot(), CompilationDbPurpose::Project,
-                                warningsConfigForProject(target->project()),
+                                warningsConfig,
-                                optionsForProject(target->project()));
+                                optionsForProject(target->project(), warningsConfig));
     Core::ProgressManager::addTask(task, tr("Generating Compilation DB"), "generate compilation db");
     m_generatorWatcher.setFuture(task);
 }

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 = clangOptionsForFile(optionsBuilder, sourceFile.toString(), projectOptions);
-    QStringList args = createClangOptions(*projectPart, sourceFile.toString(),
-                                          projectWarnings, 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(
+        const CppEditor::ClangDiagnosticConfig warningsConfig = warningsConfigForProject(nullptr);
-                    *CppEditor::CppModelManager::instance()->fallbackProjectPart(), {},
+        CppEditor::CompilerOptionsBuilder optionsBuilder = clangOptionsBuilder(
-                    warningsConfigForProject(nullptr), optionsForProject(nullptr));
+                    *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()),
+    const CppEditor::ClangDiagnosticConfig warningsConfig = warningsConfigForProject(project());
-                       optionsForProject(project()), projectPart, filePath.parentDir(), filePath);
+    CppEditor::CompilerOptionsBuilder optionsBuilder = clangOptionsBuilder(*projectPart,
+                                                                           warningsConfig);
+    addToCompilationDb(cdbChanges, optionsBuilder, optionsForProject(project(), warningsConfig),
+                       filePath.parentDir(), filePath);
     QJsonObject settings;
     addCompilationDb(settings, cdbChanges);
     DidChangeConfigurationParams configChangeParams;

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),
+                                  warningsConfig, optionsForProject(project, warningsConfig));
-                                  optionsForProject(project));
     generatorWatcher->setFuture(future);
     m_generatorSynchronizer.addFuture(future);
 }

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(clangOptionsForFile(optionsBuilder, projFile.path,
-        args = QJsonArray::fromStringList(createClangOptions(projectPart, projFile.path,
+                                                              projectOptions));
-                                                             warningsConfig, 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,
+            const QJsonObject json = createFileObject(baseDir, args, optionsBuilder, *projectPart,
-                                                      purpose, warningsConfig, projectOptions);
+                                                      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,
+QStringList clangOptionsForFile(CompilerOptionsBuilder optionsBuilder,
-                               const ClangDiagnosticConfig &warningsConfig,
+                                const QString &filePath, const QStringList &projectOptions)
-                               const QStringList &projectOptions)
 {
-    const FileOptionsBuilder fileOptions(filePath, projectPart, warningsConfig, projectOptions);
+    ProjectFile::Kind fileKind = filePath.isEmpty()
-    LibClangOptionsBuilder optionsBuilder(projectPart, fileOptions.useBuildSystemWarnings());
+            ? ProjectFile::Unclassified : ProjectFile::classify(filePath);
-    const QStringList projectPartOptions = optionsBuilder.build(CppEditor::ProjectFile::Unsupported,
+    if (fileKind == ProjectFile::AmbiguousHeader) {
-                                                                UsePrecompiledHeaders::No);
+        fileKind = optionsBuilder.projectPart().languageVersion <= LanguageVersion::LatestC
-
+                ? ProjectFile::CHeader : ProjectFile::CXXHeader;
-    // FIXME: Sanitize FileOptionsBuilder instead.
+    }
-    QStringList fileArgs = fileOptions.options();
+    auto pchUsage = getPchUsage();
-    if (projectPartOptions.contains(QLatin1String("-TP")))
+    if (pchUsage == UsePrecompiledHeaders::Yes
-        fileArgs.removeAll(QLatin1String("/TP"));
+            && optionsBuilder.projectPart().precompiledHeaders.contains(filePath)) {
-    if (projectPartOptions.contains(QLatin1String("-TC")))
+        pchUsage = UsePrecompiledHeaders::No;
-        fileArgs.removeAll(QLatin1String("/TC"));
+    }
-
+    optionsBuilder.updateFileLanguage(fileKind);
-    return projectPartOptions + fileArgs;
+    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)
+    QStringList options = ClangDiagnosticConfigsModel::globalDiagnosticOptions();
-        return getProjectSettings(project).commandLineOptions();
+    if (!warningsConfig.useBuildSystemWarnings()) {
-    return ClangProjectSettings::globalCommandLineOptions();
+        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

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,
+CppEditor::CompilerOptionsBuilder clangOptionsBuilder(
-                               const CppEditor::ClangDiagnosticConfig &warningsConfig,
+        const CppEditor::ProjectPart &projectPart,
-                               const QStringList &projectOptions);
+        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);

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;
 }

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;

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 &macros)
+{
+    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 &macros)

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 &macros);
+    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 &macro) 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;