Skip to content

Conversation

@r-barnes
Copy link

Allows the code to compile in clang 19 with -Wdeprecated-this-capture enabled.

I suggest enabling this for the entire repo to prevent incompatibilities as folks upgrade to newer compilers and language standards.

Allows the code to compile in clang 19 with `-Wdeprecated-this-capture` enabled.

I suggest enabling this for the entire repo to prevent incompatibilities as folks upgrade to newer compilers and language standards.
@r-barnes
Copy link
Author

I see some

  /tmp/pip-req-build-lwhqn334/src/treelearner/cuda/../feature_histogram.hpp(286): error: explicit capture matches default

in test outputs. These tests look as though they're using clang-10, which was released in 2020. It might be a good idea to move to a more modern compiler since clang-19 is moving in the direction of making the opposite behaviour an error.

@jameslamb jameslamb changed the title Capture 'this' in lambda for threshold function [c++] Capture 'this' in lambda for threshold function Oct 11, 2025
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your interest in LightGBM, but I think it might be too early for LightGBM to accept a change like this.

Look at this error from compiling the library (as part of the R package) with clang-20:

treelearner/feature_histogram.hpp:360:46: warning: explicit capture of 'this' with a capture default of '=' is a C++20 extension [-Wc++20-extensions]
360 | int_find_best_threshold_fun_ = =, this {

(build link)

That matches what I see at https://en.cppreference.com/w/cpp/20.html, which says that "Allow Lambda capture [=, this]" is a C++20 feature.

This project supports C++17 as its minimum version (#7016) and isn't yet ready to update to a newer minimum version. I'm especially not willing to reduce LightGBM's portability just for a feature that appears to only be about code style and not performance, safety, etc.


I see some

/tmp/pip-req-build-lwhqn334/src/treelearner/cuda/../feature_histogram.hpp(286): error: explicit capture matches default

in test outputs. These tests look as though they're using clang-10, which was released in 2020. It might be a good idea to move to a more modern compiler since clang-19 is moving in the direction of making the opposite behaviour an error.

In future conversations here, please provide links for things like this. That helps reviewers to directly find the logs you're talking about (I had to search a bit).

Looks like in this case that's the cuda 18.0 pip (Ubuntu 20.04) job: https://github.com/microsoft/LightGBM/actions/runs/18420932442/job/52494625518?pr=7053#step:5:7815

Note that those CUDA jobs are explicitly testing the ability to build lightgbm (the Python package and lib_lightgbm.so) from source, not being used to precompile artifacts, so "just use a newer compiler" isn't something we should do lightly. In those jobs we're getting clang-10 because that's the stable version on Ubuntu 20.04, which has not yet reached end-of-life (https://ubuntu.com/about/release-cycle) and which therefore some LightGBM users may be expected to be using.

Here that CI job is giving us useful feedback... it's saying that if this PR were merged as-is, someone installing clang with apt-get install clang on Ubuntu 20.04 might find themselves unable to build the CUDA variant of the project.


NOTE: I'm not very familiar with C++, so please do let me know if I've misunderstood or mischaracterized anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants