[clangd] Make decision forest model optional

[mgorny]

Summary

Make it possible to disable building the decision forest ranking
model for clangd. This was suggested on D136283 as an alternative
to splitting the huge generated file in order to fix building on PPC32.

Bug: https://bugs.gentoo.org/829602

[mgorny]

@thesamesam, could you test it on PPC32, please?

[thesamesam]

Works on PPC.

[Arfrever]

In clang-tools-extra/clangd/unittests/CMakeLists.txt, usage of CompletionModel.cmake probably also should be dependent on CLANGD_DECISION_FOREST.

In clang-tools-extra/clangd/CMakeLists.txt and clang-tools-extra/clangd/unittests/CMakeLists.txt, calls to target_include_directories() for directories with generated Completion Model headers can be also made dependent on CLANGD_DECISION_FOREST.

--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -146,10 +146,12 @@
   omp_gen
   )
 
-# Include generated CompletionModel headers.
-target_include_directories(clangDaemon PUBLIC
-  $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
-)
+if(CLANGD_DECISION_FOREST)
+  # Include generated Completion Model header.
+  target_include_directories(clangDaemon PUBLIC
+    $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
+  )
+endif()
 
 clang_target_link_libraries(clangDaemon
   PRIVATE
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -19,8 +19,12 @@
   set(REMOTE_TEST_SOURCES remote/MarshallingTests.cpp)
 endif()
 
-include(${CMAKE_CURRENT_SOURCE_DIR}/../quality/CompletionModel.cmake)
-gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/decision_forest_model DecisionForestRuntimeTest ::ns1::ns2::test::Example)
+set(DECISIONFORESTRUNTIMETEST_SOURCES)
+if(CLANGD_DECISION_FOREST)
+  include(${CMAKE_CURRENT_SOURCE_DIR}/../quality/CompletionModel.cmake)
+  gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/decision_forest_model DecisionForestRuntimeTest ::ns1::ns2::test::Example)
+  list(APPEND DECISIONFORESTRUNTIMETEST_SOURCES ${CMAKE_CURRENT_BINARY_DIR}/DecisionForestRuntimeTest.cpp)
+endif()
 
 add_custom_target(ClangdUnitTests)
 add_unittest(ClangdUnitTests ClangdTests
@@ -95,7 +99,7 @@
   TypeHierarchyTests.cpp
   URITests.cpp
   XRefsTests.cpp
-  ${CMAKE_CURRENT_BINARY_DIR}/DecisionForestRuntimeTest.cpp
+  ${DECISIONFORESTRUNTIMETEST_SOURCES}
 
   support/CancellationTests.cpp
   support/ContextTests.cpp
@@ -134,10 +138,12 @@
   $<TARGET_OBJECTS:obj.clangDaemonTweaks>
   )
 
-# Include generated ComletionModel headers.
-target_include_directories(ClangdTests PUBLIC
-  $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
-)
+if(CLANGD_DECISION_FOREST)
+  # Include generated Completion Model header.
+  target_include_directories(ClangdTests PUBLIC
+    $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
+  )
+endif()
 
 clang_target_link_libraries(ClangdTests
   PRIVATE

(Above patch also fixes typo in Comletion and fixes grammatical number of headers to singular header, since there is only 1 generated Completion Model header in each of those directories.)

[sammccall]

Thanks for doing this!

clang-tools-extra/clangd/CMakeLists.txt

52

Rather than add_definitions, could you instead add a line to clangd/Features.inc.in, and #include Features.h where needed?

There are probably tradeoffs between these approaches, but that's the one we're using for other optional features.

We also use #if rather than #if defined() which ensures we're not forgetting to include a definition and getting some default behavior.

clang-tools-extra/clangd/CodeComplete.h

129

this approach feels a bit heavy on the preprocessor, especially for a header.
It makes the decision-forest-less version seem "complete" but at the cost of having to understand two variants of the code.

what about:

• keeping all the existing APIs
• using #if to change the default value of RankingModel, but otherwise leaving this struct unchanged
• providing an alternate definition of evaluateDecisionForest that just calls std::abort()
• having ClangdMain exit with an error if DecisionForest is selected and not enabled (or simply ignoring the flag)

[mgorny]

clang-tools-extra/clangd/CodeComplete.h

129

This is what I originally wanted to do. However, it doesn't seem that this code path is covered by the test suite and I have no clue how to use clangd (or at least the test suite didn't see it crash when I added fatal asserts to the decision forest paths), so I've decided that ensure compile-time failures is the safer approach.

[sammccall]

clang-tools-extra/clangd/CodeComplete.h

129

When I add std::abort() to evaluateDecisionForest then I see lots of failed tests:

********************
Failed Tests (69):
  Clangd :: completion-auto-trigger.test
  Clangd :: completion-snippets.test
  Clangd :: completion.test
  Clangd :: protocol.test
  Clangd Unit Tests :: ./ClangdTests/0/73
...
  Clangd Unit Tests :: ./ClangdTests/9/73


Testing Time: 4.22s
  Unsupported:   7
  Passed     : 193
  Failed     :  69

The compile-time failures may be safer in some ways, but I think this use of preprocessor is more intrusive than we'd like to to maintain to work around toolchain limitations on a platform where (AFAIK) we don't have any actual users.
(i.e. even if the runtime behavior were untested, that is probably still preferable)

[mgorny]

Closing in favor of D139107.

clang-tools-extra/clangd/CMakeLists.txt

if (NOT DEFINED CLANGD_BUILD_XPC)
   endif ()
 
   llvm_canonicalize_cmake_booleans(CLANGD_BUILD_XPC_DEFAULT)
 
   set(CLANGD_BUILD_XPC ${CLANGD_BUILD_XPC_DEFAULT} CACHE BOOL "Build XPC Support For Clangd." FORCE)
   unset(CLANGD_BUILD_XPC_DEFAULT)
 endif ()
 
+option(CLANGD_DECISION_FOREST "Enable decision forest model for ranking code completion items" ON)
 option(CLANGD_MALLOC_TRIM "Call malloc_trim(3) periodically in Clangd. (only takes effect when using glibc)" ON)
 # -DCLANG_TIDY_CHECKS=Off avoids a dependency on clang-tidy, reducing rebuilds.
 option(CLANGD_TIDY_CHECKS "Link all clang-tidy checks into clangd" ON)
 
 llvm_canonicalize_cmake_booleans(
   CLANGD_BUILD_XPC
   CLANGD_ENABLE_REMOTE
   ENABLE_GRPC_REFLECTION

 
 set(LLVM_LINK_COMPONENTS
   Support
   AllTargetsInfos
   FrontendOpenMP
   Option
   )
 
+set(COMPLETIONMODEL_SOURCES)
+if(CLANGD_DECISION_FOREST)
   include(${CMAKE_CURRENT_SOURCE_DIR}/quality/CompletionModel.cmake)
   gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/quality/model CompletionModel clang::clangd::Example)
+  list(APPEND COMPLETIONMODEL_SOURCES ${CMAKE_CURRENT_BINARY_DIR}/CompletionModel.cpp)
+  add_definitions(-DCLANGD_DECISION_FOREST=1)
+endif()
 
 if(MSVC AND NOT CLANG_CL)
  set_source_files_properties(CompileCommands.cpp PROPERTIES COMPILE_FLAGS -wd4130) # disables C4130: logical operation on address of string constant
 endif()
 
 include_directories(BEFORE "${CMAKE_CURRENT_BINARY_DIR}/../clang-tidy")
 
 add_clang_library(clangDaemon
add_clang_library(clangDaemon
   SemanticHighlighting.cpp
   SemanticSelection.cpp
   SourceCode.cpp
   SystemIncludeExtractor.cpp
   TidyProvider.cpp
   TUScheduler.cpp
   URI.cpp
   XRefs.cpp
-  ${CMAKE_CURRENT_BINARY_DIR}/CompletionModel.cpp
+  ${COMPLETIONMODEL_SOURCES}
 
   index/Background.cpp
   index/BackgroundIndexLoader.cpp
   index/BackgroundIndexStorage.cpp
   index/BackgroundQueue.cpp
   index/BackgroundRebuild.cpp
   index/CanonicalIncludes.cpp
   index/FileIndex.cpp

clang-tools-extra/clangd/CodeComplete.h

struct CodeCompleteOptions {
   /// arbitrary order.
   ///
   /// This callbacks allows capturing various internal structures used by clangd
   /// during code completion. Eg: Symbol quality and relevance signals.
   std::function<void(const CodeCompletion &, const SymbolQualitySignals &,
                      const SymbolRelevanceSignals &, float Score)>
       RecordCCResult;
 
+#if defined(CLANGD_DECISION_FOREST)
+#  define DEFAULT_RANKING_MODEL DecisionForest
+#else
+#  define DEFAULT_RANKING_MODEL Heuristics
+#endif
+
   /// Model to use for ranking code completion candidates.
   enum CodeCompletionRankingModel {
     Heuristics,
+#if defined(CLANGD_DECISION_FOREST)
     DecisionForest,
-  } RankingModel = DecisionForest;
+#endif
+  } RankingModel = DEFAULT_RANKING_MODEL;
 
+#if defined(CLANGD_DECISION_FOREST)
   /// Callback used to score a CompletionCandidate if DecisionForest ranking
   /// model is enabled.
   /// This allows us to inject experimental models and compare them with
   /// baseline model using A/B testing.
   std::function<DecisionForestScores(
       const SymbolQualitySignals &, const SymbolRelevanceSignals &, float Base)>
       DecisionForestScorer = &evaluateDecisionForest;
   /// Weight for combining NameMatch and Prediction of DecisionForest.
   /// CompletionScore is NameMatch * pow(Base, Prediction).
   /// The optimal value of Base largely depends on the semantics of the model
   /// and prediction score (e.g. algorithm used during training, number of
   /// trees, etc.). Usually if the range of Prediction is [-20, 20] then a Base
   /// in [1.2, 1.7] works fine.
   /// Semantics: E.g. For Base = 1.3, if the Prediction score reduces by 2.6
   /// points then completion score reduces by 50% or 1.3^(-2.6).
   float DecisionForestBase = 1.3f;
+#endif
 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ API.
 // We don't use the LSP structures here (unlike most features) as we want
 // to expose more data to allow for more precise testing and evaluation.
 struct CodeCompletion {
   // The unqualified name of the symbol or other completion item.
   std::string Name;

clang-tools-extra/clangd/CodeComplete.cpp

case RM::Heuristics:
       // NameMatch is in fact a multiplier on total score, so rescoring is
       // sound.
       Scores.ExcludingName =
           Relevance.NameMatch > std::numeric_limits<float>::epsilon()
               ? Scores.Total / Relevance.NameMatch
               : Scores.Quality;
       return Scores;
 
+#if defined(CLANGD_DECISION_FOREST)
     case RM::DecisionForest:
       DecisionForestScores DFScores = Opts.DecisionForestScorer(
           Quality, Relevance, Opts.DecisionForestBase);
       Scores.ExcludingName = DFScores.ExcludingName;
       Scores.Total = DFScores.Total;
       return Scores;
+#endif
     }
     llvm_unreachable("Unhandled CodeCompletion ranking model.");
   }
 
   // Scores a candidate and adds it to the TopN structure.
   void addCandidate(TopN<ScoredBundle, ScoredBundleGreater> &Candidates,
                     CompletionCandidate::Bundle Bundle) {
     SymbolQualitySignals Quality;

clang-tools-extra/clangd/Quality.h

struct SymbolRelevanceSignals {
   float evaluateHeuristics() const;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &,
                               const SymbolRelevanceSignals &);
 
 /// Combine symbol quality and relevance into a single score.
 float evaluateSymbolAndRelevance(float SymbolQuality, float SymbolRelevance);
 
+#if defined(CLANGD_DECISION_FOREST)
 /// Same semantics as CodeComplete::Score. Quality score and Relevance score
 /// have been removed since DecisionForest cannot assign individual scores to
 /// Quality and Relevance signals.
 struct DecisionForestScores {
   float Total = 0.f;
   float ExcludingName = 0.f;
 };
 
 DecisionForestScores
 evaluateDecisionForest(const SymbolQualitySignals &Quality,
                        const SymbolRelevanceSignals &Relevance, float Base);
+#endif
 
 /// TopN<T> is a lossy container that preserves only the "best" N elements.
 template <typename T, typename Compare = std::greater<T>> class TopN {
 public:
   using value_type = T;
   TopN(size_t N, Compare Greater = Compare())
       : N(N), Greater(std::move(Greater)) {}
 

clang-tools-extra/clangd/Quality.cpp

 //===--- Quality.cpp ---------------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Quality.h"
 #include "AST.h"
 #include "ASTSignals.h"
+#if defined(CLANGD_DECISION_FOREST)
 #include "CompletionModel.h"
+#endif
 #include "FileDistance.h"
 #include "SourceCode.h"
 #include "index/Symbol.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclVisitor.h"
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
 
   return OS;
 }
 
 float evaluateSymbolAndRelevance(float SymbolQuality, float SymbolRelevance) {
   return SymbolQuality * SymbolRelevance;
 }
 
+#if defined(CLANGD_DECISION_FOREST)
 DecisionForestScores
 evaluateDecisionForest(const SymbolQualitySignals &Quality,
                        const SymbolRelevanceSignals &Relevance, float Base) {
   Example E;
   E.setIsDeprecated(Quality.Deprecated);
   E.setIsReservedName(Quality.ReservedName);
   E.setIsImplementationDetail(Quality.ImplementationDetail);
   E.setNumReferences(Quality.References);
if (Relevance.Forbidden)
     Scores.ExcludingName *= 0;
   if (Quality.Category == SymbolQualitySignals::Keyword)
     Scores.ExcludingName *= 4;
 
   // NameMatch should be a multiplier on total score to support rescoring.
   Scores.Total = Relevance.NameMatch * Scores.ExcludingName;
   return Scores;
 }
+#endif
 
 // Produces an integer that sorts in the same order as F.
 // That is: a < b <==> encodeFloat(a) < encodeFloat(b).
 static uint32_t encodeFloat(float F) {
   static_assert(std::numeric_limits<float>::is_iec559);
   constexpr uint32_t TopBit = ~(~uint32_t{0} >> 1);
 
   // Get the bits of the float. Endianness is the same as for integers.

clang-tools-extra/clangd/benchmarks/CMakeLists.txt

+if(CLANGD_DECISION_FOREST)
   add_subdirectory(CompletionModel)
+endif()
 
 add_benchmark(IndexBenchmark IndexBenchmark.cpp)
 
 target_link_libraries(IndexBenchmark
   PRIVATE
   clangDaemon
   LLVMSupport
   )

clang-tools-extra/clangd/tool/ClangdMain.cpp

opt<CodeCompleteOptions::CodeCompletionParse> CodeCompletionParse{
     init(CodeCompleteOptions().RunParser),
     Hidden,
 };
 
 opt<CodeCompleteOptions::CodeCompletionRankingModel> RankingModel{
     "ranking-model",
     cat(Features),
     desc("Model to use to rank code-completion items"),
-    values(clEnumValN(CodeCompleteOptions::Heuristics, "heuristics",
-                      "Use hueristics to rank code completion items"),
+    values(
+#if defined(CLANGD_DECISION_FOREST)
            clEnumValN(CodeCompleteOptions::DecisionForest, "decision_forest",
-                      "Use Decision Forest model to rank completion items")),
+                      "Use Decision Forest model to rank completion items"),
+#endif
+           clEnumValN(CodeCompleteOptions::Heuristics, "heuristics",
+                      "Use hueristics to rank code completion items")),
     init(CodeCompleteOptions().RankingModel),
     Hidden,
 };
 
 // FIXME: also support "plain" style where signatures are always omitted.
 enum CompletionStyleFlag { Detailed, Bundled };
 opt<CompletionStyleFlag> CompletionStyle{
     "completion-style",

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

return codeComplete(FilePath, Test.point(), /*Preamble=*/nullptr, ParseInput,
                       Opts);
 }
 
 Symbol withReferences(int N, Symbol S) {
   S.References = N;
   return S;
 }
 
+#if defined(CLANGD_DECISION_FOREST)
 TEST(DecisionForestRankingModel, NameMatchSanityTest) {
   clangd::CodeCompleteOptions Opts;
   Opts.RankingModel = CodeCompleteOptions::DecisionForest;
   auto Results = completions(
       R"cpp(
 struct MemberAccess {
   int ABG();
   int AlphaBetaGamma();
TEST(DecisionForestRankingModel, DecisionForestScorerCallbackTest) {
   // Do not use DecisionForestScorer for heuristics model.
   Opts.RankingModel = CodeCompleteOptions::Heuristics;
   Results = completions(Code,
                         /*IndexSymbols=*/{}, Opts);
   ASSERT_EQ(Results.Completions.size(), 1u);
   EXPECT_NE(Results.Completions[0].Score.Total, MagicNumber);
   EXPECT_NE(Results.Completions[0].Score.ExcludingName, MagicNumber);
 }
+#endif
 
 TEST(CompletionTest, Limit) {
   clangd::CodeCompleteOptions Opts;
   Opts.Limit = 2;
   auto Results = completions(R"cpp(
 struct ClassWithMembers {
   int AAA();
   int BBB();