[ML] Fix compiler warnings across the codebase by edsavage · Pull Request #2985 · elastic/ml-cpp

edsavage · 2026-03-12T21:11:14Z

Summary

Reduces Clang warnings from ~2,500 to 86 (all remaining are -Wunsafe-buffer-usage, which require a std::span migration and are out of scope here).
Adds three warning suppressions to clang.cmake:
- -Wno-switch-default (174 warnings) — conflicts with the more useful -Wswitch-enum; adding default: to exhaustive enum switches would suppress missing-case detection when new enum values are added.
- -Wno-nrvo (19 warnings) — purely informational C++23 diagnostic about NRVO eligibility.
- -Wno-missing-noreturn (5 warnings) — all remaining cases are lambdas where [[noreturn]] cannot be applied pre-C++23.
Fixes 80 code warnings across 59 files:
- Removes 37 unused const variables (dead code from state serialisation refactors).
- Removes 2 unused functions (calibrationExperiment, dataGenerator in tests) and 2 unused-but-set variables.
- Fixes 9 shadow warnings by renaming inner variables.
- Fixes 8 implicit int-to-float conversions with explicit static_cast<double>().
- Fixes 2 tautological-compare logic bugs where !(p >= 0.0 || p <= 1.0) was always false — corrected to !(p >= 0.0 && p <= 1.0).
- Removes 2 redundant default: cases in exhaustive enum switches.
- Fixes 1 pessimizing-move, 1 range-loop-bind-reference, 1 sign-compare, 1 shorten-64-to-32, 1 CTAD issue.
- Removes unnecessary virtual from method in final class.
- Adds [[noreturn]] to named function throws().

Test plan

CI passes on all platforms (Linux x86_64, Linux aarch64, macOS aarch64, Windows x86_64)
clang-format check passes
Verify no behavioural changes other than the tautological-compare bug fixes

Made with Cursor

Reduce Clang warnings from ~2500 to 86 (all remaining are -Wunsafe-buffer-usage which require a std::span migration). Compiler flag suppressions (clang.cmake): - -Wno-switch-default: conflicts with the more useful -Wswitch-enum - -Wno-nrvo: purely informational C++23 diagnostic - -Wno-missing-noreturn: remaining cases are lambdas where [[noreturn]] cannot be applied pre-C++23 Code fixes across 59 files: - Remove 37 unused const variables (dead code from state serialisation refactors) - Remove 2 unused functions and 2 unused-but-set variables - Fix 9 shadow warnings by renaming inner variables - Fix 8 implicit int-to-float conversions with static_cast - Fix 2 tautological-compare logic bugs where the condition !(p >= 0.0 || p <= 1.0) was always false - Remove 2 redundant default cases in exhaustive enum switches - Fix 1 pessimizing-move, 1 range-loop-bind-reference, 1 sign-compare, 1 shorten-64-to-32, 1 CTAD issue - Remove unnecessary virtual from method in final class - Add [[noreturn]] to named function throws() - Add missing newline at EOF Made-with: Cursor

prodsecmachine · 2026-03-12T22:28:58Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Made-with: Cursor

Made-with: Cursor # Conflicts: # bin/pytorch_inference/Main.cc

Change [=] to [this, f] in CConcurrentWrapper to silence -Wdeprecated on GCC 13 and Clang. Made-with: Cursor

Copilot

Pull request overview

This PR reduces Clang warning volume across the ML C++ codebase and associated tests, including a couple of small correctness fixes, by removing unused declarations, tightening types/casts, and adjusting warning configuration.

Changes:

Add Clang warning suppressions in CMake and remove/adjust code patterns that trigger noisy diagnostics.
Remove unused constants/functions/variables and resolve shadowing / conversion warnings in production and test code.
Fix probability-range validation logic in CCategoricalTools (tautological compare) and other small correctness/typing issues.

Reviewed changes

Copilot reviewed 58 out of 60 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
lib/model/unittest/ModelTestHelpers.h	Marks header-local helper functions as `[[maybe_unused]]` to silence unused-function warnings.
lib/model/unittest/CTokenListDataCategorizerTest.cc	Renames lambda parameter to avoid shadowing/warnings.
lib/model/unittest/CModelMemoryTest.cc	Removes unused string constant from test TU.
lib/model/FunctionTypes.cc	Removes unused `EMPTY_FUNCTIONS` declaration.
lib/model/CSearchKey.cc	Removes unused tag constant(s).
lib/model/CResourceMonitor.cc	Removes redundant `default:` in an exhaustive enum switch.
lib/model/CMetricBucketGatherer.cc	Removes unused `EMPTY_DOUBLE_VEC` constant.
lib/model/CEventRateModel.cc	Adds explicit cast for category id passed to `concentration(...)`.
lib/model/CDetectorEqualizer.cc	Renames lambda parameters to avoid shadowing / improve clarity.
lib/model/CDataGatherer.cc	Removes unused `EMPTY_STRING` constant.
lib/model/CAnnotatedProbability.cc	Removes unused persistence tag constant.
lib/maths/time_series/unittest/CCalendarCyclicTestTest.cc	Fixes sign-compare warning by casting loop index in comparison.
lib/maths/time_series/CTimeSeriesDecompositionStateSerialiser.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/time_series/CTimeSeriesDecompositionDetail.cc	Adds explicit cast to match expected floating-point parameter types.
lib/maths/time_series/CTimeSeriesDecomposition.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/time_series/CSeasonalComponentAdaptiveBucketing.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/time_series/CSeasonalComponent.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/time_series/CDecompositionComponent.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/time_series/CCalendarCyclicTest.cc	Makes integer→double conversions explicit to silence warnings.
lib/maths/time_series/CCalendarComponentAdaptiveBucketing.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/time_series/CCalendarComponent.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/time_series/CAdaptiveBucketing.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/unittest/CToolsTest.cc	Makes mixed-type loop bound comparison explicit.
lib/maths/common/unittest/CMultivariateNormalConjugateTest.cc	Removes unused helper functions in tests.
lib/maths/common/CXMeansOnline1d.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CStatisticalTests.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CPriorStateSerialiser.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CPoissonMeanConjugate.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/COneOfNPrior.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CNormalMeanPrecConjugate.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CNaturalBreaksClassifier.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CMultivariateConstantPrior.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CMultinomialConjugate.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CMultimodalPrior.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CModel.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CLogNormalMeanPrecConjugate.cc	Removes unused `EMPTY_STRING` constant.
lib/maths/common/CGammaRateConjugate.cc	Removes unused `UNKNOWN_VALUE_STRING` constant.
lib/maths/common/CCategoricalTools.cc	Fixes tautological probability-range checks (`
lib/maths/analytics/unittest/CDataFrameUtilsTest.cc	Adds explicit cast for map key conversion in test assertions.
lib/maths/analytics/CBoostedTreeLoss.cc	Removes unused tag constant.
lib/maths/analytics/CBoostedTreeFactory.cc	Removes unused tag constant.
lib/core/unittest/CConcurrencyTest.cc	Marks a test helper as `[[noreturn]]` to address missing-noreturn warnings.
lib/core/CStateMachine.cc	Removes unused tag constants.
lib/core/CJsonStateRestoreTraverser.cc	Removes redundant `default:` in switch used for logging token types.
lib/api/unittest/CTestAnomalyJob.cc	Formatting-only newline/line-ending adjustment.
lib/api/unittest/CInferenceModelMetadataTest.cc	Makes mixed integer/floating arithmetic explicit.
lib/api/unittest/CDataFrameAnalyzerTrainingTest.cc	Removes pessimizing move into `stringstream`.
lib/api/unittest/CAnomalyJobTest.cc	Renames test constants and makes template argument explicit for `uniform_int_distribution`.
lib/api/CSingleFieldDataCategorizer.cc	Renames lambda captures to avoid shadowing warnings.
lib/api/CModelSizeStatsJsonWriter.cc	Removes unused JSON field name constant.
lib/api/CForecastRunner.cc	Makes integer→double conversion explicit before applying fractional limit.
lib/api/CFieldDataCategorizer.cc	Renames lambda captures to avoid shadowing warnings in background persistence.
lib/api/CDetectionRulesJsonParser.cc	Removes unused `CONDITION` constant (keeps `CONDITIONS`).
lib/api/CDataFrameAnalysisInstrumentation.cc	Removes unused JSON tag constants.
include/model/CMetricModelFactory.h	Removes redundant `virtual` on method in a `final` class.
include/maths/common/CBootstrapClusterer.h	Removes unused locals and updates trace logging to derive partition sizes from parity vector.
include/maths/common/CBasicStatistics.h	Makes coordinate conversion explicit in custom central-moments add.
include/core/CConcurrentWrapper.h	Makes lambda capture explicit to avoid deprecated implicit `this` capture warnings.
cmake/compiler/clang.cmake	Adds warning suppressions for `missing-noreturn`, `nrvo`, and `switch-default`.
bin/pytorch_inference/Main.cc	Adds explicit cast for Torch thread count API.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-22T21:38:02Z

lib/core/CJsonStateRestoreTraverser.cc

        case SBoostJsonHandler::E_TokenStringPart:
            tokenTypeName = "string_part";
            break;
-        default:
-            tokenTypeName = "unknown";
-            break;
        }

        LOG_ERROR(<< "JSON state must be object at root. Found token type: " << tokenTypeName


With the default: removed, tokenTypeName will remain the empty string if m_Handler.s_Type ever has an unexpected value (e.g. new enum value added later), which makes the log line less informative. Consider initializing tokenTypeName to something like "unknown" before the switch (so you can keep the switch exhaustive without reintroducing a default:).

Address Copilot review: if a new enum value is added to SBoostJsonHandler, the switch won't cover it and the log message would contain an empty string. Initialising to "unknown" keeps the error log informative. Made-with: Cursor

edsavage added :ml >refactoring labels Mar 12, 2026

edsavage added 4 commits March 13, 2026 14:04

[ML] Run clang-format on compiler warning fixes

1edba0a

Made-with: Cursor

Merge remote-tracking branch 'upstream/main' into fix/compiler-warnings

0f382ef

Made-with: Cursor # Conflicts: # bin/pytorch_inference/Main.cc

[ML] Fix deprecated implicit this capture via [=] in C++20

27e3df7

Change [=] to [this, f] in CConcurrentWrapper to silence -Wdeprecated on GCC 13 and Clang. Made-with: Cursor

Formatting

5e44996

edsavage requested a review from Copilot March 22, 2026 21:32

Copilot started reviewing on behalf of edsavage March 22, 2026 21:33 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Fix compiler warnings across the codebase#2985

[ML] Fix compiler warnings across the codebase#2985
edsavage wants to merge 6 commits intoelastic:mainfrom
edsavage:fix/compiler-warnings

edsavage commented Mar 12, 2026

Uh oh!

prodsecmachine commented Mar 12, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

edsavage commented Mar 12, 2026

Summary

Test plan

Uh oh!

prodsecmachine commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

prodsecmachine commented Mar 12, 2026 •

edited

Loading