mgorny added inline comments.
================ Comment at: clang-tools-extra/clangd/CodeComplete.h:129 +#if defined(CLANGD_DECISION_FOREST) +# define DEFAULT_RANKING_MODEL DecisionForest ---------------- sammccall wrote: > 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) 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138520/new/ https://reviews.llvm.org/D138520 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
