[clangd] Allow to build Clangd without decision forest

[ilya-biryukov]

Summary

Make it possible to disable building the decision forest ranking
model for clangd. To unbreak build of Clangd on PPC32 in gentoo, see
https://bugs.gentoo.org/829602

[ilya-biryukov]

This seems to build in both modes, but I still need to figure out what to do with tests with decision forest off.
Also, maybe there is a way to minimize the use of preprocessor even more.

clang-tools-extra/clangd/decision-forest/DecisionForest.cpp

15 ↗

(On Diff #479270)

Note to self: split into two namespace declarations for consistency with the rest of clangd

[mgorny]

Thanks a lot for doing this!

[sammccall]

clang-tools-extra/clangd/CMakeLists.txt

54

this muddies the waters a bit: the code in DecisionForest.cpp is *not* the generated completion model.
as mentioned below, I think it's a little clearer to have this be unconditionally a source file, and #ifdef the contents.

clang-tools-extra/clangd/Features.inc.in

7

we could include this in the feature-string too (Feature.cpp) but I think it's probably more noise than signal.

clang-tools-extra/clangd/decision-forest/DecisionForest_disabled.cpp

19 ↗

(On Diff #479270)

As much as I dislike the preprocessor, I think conditionally including source files is undiscoverable enough (if you're reading the source code) that I'd have a preference for this being an #else branch in DecisionForest.cpp.

(It's also one mechanism vs two, given that we *do* still need the preprocessor elsewhere)

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

215

hmm, while reordering these: hueristics -> heuristics? :-)

216

CodeCompleteOptions().RankingModel is also the one used in unit tests.
I think the default value in the struct should be ifdef'd, or if we really want to avoid pp-conditionals in header files, we could have an Auto value which acts sa the platform-default.

[ilya-biryukov]

• Merge DecisionForest.cpp and *_disabled.cpp, change #ifndef to #if
• Add decision_forest to the feature string
• Set Heuristics as the default code completion model in tests
• Do not compile decision forest tests when it's disabled
• Remove the use of preprocessor for defining the default value of -ranking-model flag to clangd

clang-tools-extra/clangd/Features.inc.in

7

Maybe include only when it's disabled? I've added this to the commit, PTAL.

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

215

I'm not sure how the reordering happened in the first place, TBH 🤔
I kept it and fixed the typo, but also let me know if I should keep the old order instead.

216

I have moved it to the .cpp file. I think that avoiding conditionals in the preprocessor is a good thing definitely.

I also considered putting the constant into DecisionForest.cpp to reduce the number of files that use #if CLANGD_DECISION_FOREST.
But the downside is that definition of the constant gets harder to find, so I opted for keeping it in this file.

[sammccall]

Looks great, thanks!

clang-tools-extra/clangd/CMakeLists.txt

21

May want to add a motivating comment like "this involves generating and compiling large source files, which can run into toolchain limitations"?

(This does violate the convention that build configurations should be as mysterious as possible!)

clang-tools-extra/clangd/CodeComplete.cpp

26

Include feature.h instead of relying on it transitively?

(Important here as losing the include changes the behavior)

clang-tools-extra/clangd/CodeComplete.h

134

Nice, somehow i forgot this was an option

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

14

Feature.h here too

[ilya-biryukov]

• Add a comment to CMakelists.txt explaining motivation for the option
• Add #include "Feature.h"
• Revert changes to the order of compiler flags to keep this change more focused
• clang-format
• replace #ifndef with #if ! in ClangdMain.cpp

[ilya-biryukov]

I have sent a follow-up NFC change to fix a typo in the documentation of --ranking-model=heuristics. Note that I decided to avoid changing the order of options for --ranking-model in the documentation, this didn't seem important.

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
   CLANGD_MALLOC_TRIM
   CLANGD_TIDY_CHECKS
   LLVM_ENABLE_ZLIB
+  CLANGD_DECISION_FOREST
 )
 
 configure_file(
   ${CMAKE_CURRENT_SOURCE_DIR}/Features.inc.in
   ${CMAKE_CURRENT_BINARY_DIR}/Features.inc
 )
 
 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)
+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
   AST.cpp
   ASTSignals.cpp
   ClangdLSPServer.cpp
   ClangdServer.cpp
   CodeComplete.cpp
   CodeCompletionStrings.cpp
   CollectMacros.cpp
   CompileCommands.cpp
   Compiler.cpp
   Config.cpp
   ConfigCompile.cpp
   ConfigProvider.cpp
   ConfigYAML.cpp
+  DecisionForest.cpp
   Diagnostics.cpp
   DraftStore.cpp
   DumpAST.cpp
   ExpectedTypes.cpp
   FeatureModule.cpp
   Feature.cpp
   FindSymbols.cpp
   FindTarget.cpp
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 {
   std::function<void(const CodeCompletion &, const SymbolQualitySignals &,
                      const SymbolRelevanceSignals &, float Score)>
       RecordCCResult;
 
   /// Model to use for ranking code completion candidates.
   enum CodeCompletionRankingModel {
     Heuristics,
     DecisionForest,
-  } RankingModel = DecisionForest;
+  };
+  static const CodeCompletionRankingModel DefaultRankingModel;
+  CodeCompletionRankingModel RankingModel = DefaultRankingModel;
 
   /// 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;

clang-tools-extra/clangd/CodeComplete.cpp

 //===----------------------------------------------------------------------===//
 
 #include "CodeComplete.h"
 #include "AST.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
 #include "ExpectedTypes.h"
 #include "FileDistance.h"
 #include "FuzzyMatch.h"
 #include "Headers.h"
 #include "Hover.h"
 #include "Preamble.h"
 #include "Protocol.h"
 #include "Quality.h"
 #include "SourceCode.h"
 #include "URI.h"
 #include "index/Index.h"

 #include <limits>
 #include <utility>
 
 // We log detailed candidate here if you run with -debug-only=codecomplete.
 #define DEBUG_TYPE "CodeComplete"
 
 namespace clang {
 namespace clangd {
+
+#if CLANGD_DECISION_FOREST
+const CodeCompleteOptions::CodeCompletionRankingModel
+    CodeCompleteOptions::DefaultRankingModel =
+        CodeCompleteOptions::DecisionForest;
+#else
+const CodeCompleteOptions::CodeCompletionRankingModel
+    CodeCompleteOptions::DefaultRankingModel = CodeCompleteOptions::Heuristics;
+#endif
+
 namespace {
 
 CompletionItemKind toCompletionItemKind(index::SymbolKind Kind) {
   using SK = index::SymbolKind;
   switch (Kind) {
   case SK::Unknown:
     return CompletionItemKind::Missing;
   case SK::Module:

clang-tools-extra/clangd/DecisionForest.cpp

+//===--- DecisionForest.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 "Features.inc"
+
+#if !CLANGD_DECISION_FOREST
+#include "Quality.h"
+#include <cstdlib>
+
+namespace clang {
+namespace clangd {
+DecisionForestScores
+evaluateDecisionForest(const SymbolQualitySignals &Quality,
+                       const SymbolRelevanceSignals &Relevance, float Base) {
+  llvm::errs() << "Clangd was compiled without decision forest support.\n";
+  std::abort();
+}
+
+} // namespace clangd
+} // namespace clang
+
+#else // !CLANGD_DECISION_FOREST
+
+#include "CompletionModel.h"
+#include "Quality.h"
+#include <cmath>
+
+namespace clang {
+namespace clangd {
+
+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);
+  E.setSymbolCategory(Quality.Category);
+
+  SymbolRelevanceSignals::DerivedSignals Derived =
+      Relevance.calculateDerivedSignals();
+  int NumMatch = 0;
+  if (Relevance.ContextWords) {
+    for (const auto &Word : Relevance.ContextWords->keys()) {
+      if (Relevance.Name.contains_insensitive(Word)) {
+        ++NumMatch;
+      }
+    }
+  }
+  E.setIsNameInContext(NumMatch > 0);
+  E.setNumNameInContext(NumMatch);
+  E.setFractionNameInContext(
+      Relevance.ContextWords && !Relevance.ContextWords->empty()
+          ? NumMatch * 1.0 / Relevance.ContextWords->size()
+          : 0);
+  E.setIsInBaseClass(Relevance.InBaseClass);
+  E.setFileProximityDistanceCost(Derived.FileProximityDistance);
+  E.setSemaFileProximityScore(Relevance.SemaFileProximityScore);
+  E.setSymbolScopeDistanceCost(Derived.ScopeProximityDistance);
+  E.setSemaSaysInScope(Relevance.SemaSaysInScope);
+  E.setScope(Relevance.Scope);
+  E.setContextKind(Relevance.Context);
+  E.setIsInstanceMember(Relevance.IsInstanceMember);
+  E.setHadContextType(Relevance.HadContextType);
+  E.setHadSymbolType(Relevance.HadSymbolType);
+  E.setTypeMatchesPreferred(Relevance.TypeMatchesPreferred);
+
+  DecisionForestScores Scores;
+  // Exponentiating DecisionForest prediction makes the score of each tree a
+  // multiplciative boost (like NameMatch). This allows us to weigh the
+  // prediction score and NameMatch appropriately.
+  Scores.ExcludingName = pow(Base, Evaluate(E));
+  // Following cases are not part of the generated training dataset:
+  //  - Symbols with `NeedsFixIts`.
+  //  - Forbidden symbols.
+  //  - Keywords: Dataset contains only macros and decls.
+  if (Relevance.NeedsFixIts)
+    Scores.ExcludingName *= 0.5;
+  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;
+}
+
+} // namespace clangd
+} // namespace clang
+
+ #endif // !CLANGD_DECISION_FOREST
 No newline at end of file

clang-tools-extra/clangd/Feature.cpp

 #endif
 #if CLANGD_BUILD_XPC
       "+xpc"
 #endif
 
 #if !CLANGD_TIDY_CHECKS
       "-tidy"
 #endif
+
+#if !CLANGD_DECISION_FOREST
+      "-decision_forest"
+#endif
       ;
 }
 
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/Features.inc.in

 // IWYU pragma: private, include "Features.h"
 #define CLANGD_BUILD_XPC @CLANGD_BUILD_XPC@
 #define CLANGD_ENABLE_REMOTE @CLANGD_ENABLE_REMOTE@
 #define ENABLE_GRPC_REFLECTION @ENABLE_GRPC_REFLECTION@
 #define CLANGD_MALLOC_TRIM @CLANGD_MALLOC_TRIM@
 #define CLANGD_TIDY_CHECKS @CLANGD_TIDY_CHECKS@
+#define CLANGD_DECISION_FOREST @CLANGD_DECISION_FOREST@

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"
-#include "CompletionModel.h"
 #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;
 }
 
-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);
-  E.setSymbolCategory(Quality.Category);
-
-  SymbolRelevanceSignals::DerivedSignals Derived =
-      Relevance.calculateDerivedSignals();
-  int NumMatch = 0;
-  if (Relevance.ContextWords) {
-    for (const auto &Word : Relevance.ContextWords->keys()) {
-      if (Relevance.Name.contains_insensitive(Word)) {
-        ++NumMatch;
-      }
-    }
-  }
-  E.setIsNameInContext(NumMatch > 0);
-  E.setNumNameInContext(NumMatch);
-  E.setFractionNameInContext(
-      Relevance.ContextWords && !Relevance.ContextWords->empty()
-          ? NumMatch * 1.0 / Relevance.ContextWords->size()
-          : 0);
-  E.setIsInBaseClass(Relevance.InBaseClass);
-  E.setFileProximityDistanceCost(Derived.FileProximityDistance);
-  E.setSemaFileProximityScore(Relevance.SemaFileProximityScore);
-  E.setSymbolScopeDistanceCost(Derived.ScopeProximityDistance);
-  E.setSemaSaysInScope(Relevance.SemaSaysInScope);
-  E.setScope(Relevance.Scope);
-  E.setContextKind(Relevance.Context);
-  E.setIsInstanceMember(Relevance.IsInstanceMember);
-  E.setHadContextType(Relevance.HadContextType);
-  E.setHadSymbolType(Relevance.HadSymbolType);
-  E.setTypeMatchesPreferred(Relevance.TypeMatchesPreferred);
-
-  DecisionForestScores Scores;
-  // Exponentiating DecisionForest prediction makes the score of each tree a
-  // multiplciative boost (like NameMatch). This allows us to weigh the
-  // prediction score and NameMatch appropriately.
-  Scores.ExcludingName = pow(Base, Evaluate(E));
-  // Following cases are not part of the generated training dataset:
-  //  - Symbols with `NeedsFixIts`.
-  //  - Forbidden symbols.
-  //  - Keywords: Dataset contains only macros and decls.
-  if (Relevance.NeedsFixIts)
-    Scores.ExcludingName *= 0.5;
-  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;
-}
-
 // 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.
   uint32_t U = llvm::FloatToBits(F);

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"),
-           clEnumValN(CodeCompleteOptions::DecisionForest, "decision_forest",
-                      "Use Decision Forest model to rank completion items")),
+    values(clEnumValN(CodeCompleteOptions::DecisionForest, "decision_forest",
+                      "Use Decision Forest model to rank completion items"),
+           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",
     cat(Features),
if (EC) {
       InputMirrorStream.reset();
       llvm::errs() << "Error while opening an input mirror file: "
                    << EC.message();
     } else {
       InputMirrorStream->SetUnbuffered();
     }
   }
 
+#ifndef CLANGD_DECISION_FOREST
+  if (RankingModel == clangd::CodeCompleteOptions::DecisionForest) {
+    llvm::errs() << "Clangd was compiled without decision forest support.\n";
+    return 1;
+  }
+#endif
+
   // Setup tracing facilities if CLANGD_TRACE is set. In practice enabling a
   // trace flag in your editor's config is annoying, launching with
   // `CLANGD_TRACE=trace.json vim` is easier.
   llvm::Optional<llvm::raw_fd_ostream> TracerStream;
   std::unique_ptr<trace::EventTracer> Tracer;
   const char *JSONTraceFile = getenv("CLANGD_TRACE");
   const char *MetricsCSVFile = getenv("CLANGD_METRICS");
   const char *TracerFile = JSONTraceFile ? JSONTraceFile : MetricsCSVFile;

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

 //===-- CodeCompleteTests.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 "ASTSignals.h"
 #include "Annotations.h"
 #include "ClangdServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
 #include "Matchers.h"
 #include "Protocol.h"
 #include "Quality.h"
 #include "SourceCode.h"
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/Index.h"
return codeComplete(FilePath, Test.point(), /*Preamble=*/nullptr, ParseInput,
                       Opts);
 }
 
 Symbol withReferences(int N, Symbol S) {
   S.References = N;
   return S;
 }
 
+#if CLANGD_DECISION_FOREST
 TEST(DecisionForestRankingModel, NameMatchSanityTest) {
   clangd::CodeCompleteOptions Opts;
   Opts.RankingModel = CodeCompleteOptions::DecisionForest;
   auto Results = completions(
       R"cpp(
 struct MemberAccess {
   int ABG();
   int AlphaBetaGamma();
EXPECT_THAT(
       ElementsAre(named("clangD"), named("clangA")));
   EXPECT_THAT(
       completions("int main() { clang^ }",
                   {withReferences(NumReferences, ns("clangA")), func("clangD")},
                   Opts)
           .Completions,
       ElementsAre(named("clangA"), named("clangD")));
 }
+#endif // CLANGD_DECISION_FOREST
 
 TEST(DecisionForestRankingModel, DecisionForestScorerCallbackTest) {
   clangd::CodeCompleteOptions Opts;
   constexpr float MagicNumber = 1234.5678f;
   Opts.RankingModel = CodeCompleteOptions::DecisionForest;
   Opts.DecisionForestScorer = [&](const SymbolQualitySignals &,
                                   const SymbolRelevanceSignals &, float Base) {
     DecisionForestScores Scores;