Skip to content

fix: replace unused-result of set::extract with erase#14911

Merged
xokdvium merged 1 commit intoNixOS:masterfrom
tomfitzhenry:set-extract-unused-result
Jan 5, 2026
Merged

fix: replace unused-result of set::extract with erase#14911
xokdvium merged 1 commit intoNixOS:masterfrom
tomfitzhenry:set-extract-unused-result

Conversation

@tomfitzhenry
Copy link
Contributor

@tomfitzhenry tomfitzhenry commented Jan 2, 2026

As of llvm/llvm-project#169982 this occurence is caught by LLVM, and it's the only such example.

I don't think believe this changes behaviour.

For context, #5106 introduced the extract call.

Motivation

Nix builds with Werror=unused-result and as of llvm/llvm-project#169982 that this will catch more cases.


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

As of llvm/llvm-project#169982 this will be
caught by LLVM, and it's the only such example.
@tomfitzhenry tomfitzhenry marked this pull request as ready for review January 2, 2026 02:10
@tomfitzhenry tomfitzhenry changed the title fix: replace unused set::extract with erase fix: replace unused-result of set::extract with erase Jan 2, 2026
Copy link
Contributor

@xokdvium xokdvium left a comment

Choose a reason for hiding this comment

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

Hm, not sure it's a correctness issue. node_handle does free the element in its destructor, but erase is more concise. libc++ might be too aggressive with [[nodiscard]] here.

@xokdvium xokdvium added this pull request to the merge queue Jan 5, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 5, 2026
@xokdvium xokdvium added this pull request to the merge queue Jan 5, 2026
Merged via the queue into NixOS:master with commit 7610d07 Jan 5, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants