[clang-tools-extra] [clang-tidy] add modernize-use-std-numbers (PR #66583)

Julian Schmidt via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 3 13:01:08 PST 2023


5chmidti wrote:

It looks like GitHub decided to not list all new commits in the conversation view... there are a few more in the commits tab.
The newly pushed changes start at commit [refactor from using transformer to a normal check](https://github.com/llvm/llvm-project/pull/66583/commits/b1adc10d5a0b1456725979a1f480a517b37b6b2f).

The reason I found the issue I have created is what I had to change/remove in this commit:
[fix literal in template problem in exchange for not resolving implicit casts](https://github.com/llvm/llvm-project/pull/66583/commits/01d6b2e4e01e06e894710756ee83c668995bafbb)
I wanted the replacements to resolve implicit casts, like they do for explicit casts currently. The test shows the (IMO) degraded quality of the replacements.
After my findings/comment in that issue, I decided to push my changes this way to maybe move forward with this check and leave the implicit cast matching for another pr in the future. What do you think?

> Looks like having this check implemented as an multiple matchers isn't a good idea, simply because we pickup first one that match instead a nearest one. This leads to bugs when dealing with proper values.

There is a single matcher that matches literals and formulas. Formulas are applied as soon as a matched pattern is found. All matched literals are collected and sorted, if no formula was matched and the literal that differs the least from its constant is selected. 

> In ideal conditions something like x* 3.14 should be even detected as PI.

and

> also this precision 0.001 should be put into configuration option.

This is now configurable with the config value `DiffThreshold`. The default value is `0.001`, like it was before. Should this default be lower to detect your `3.14` example?

> Also warning message should already say what from std::numbers should be used and how far are current and proposed values from them self.

The messages now follow the following format:

- `prefer 'std::numbers::pi' to this literal, differs by '2.34e-2'` (using scientific notation to be more compact than printing `0.00000000123456789`)
- `prefer 'std::numbers::pi_v<float>' to this formula` (the message uses the replacement code from the fix-it hint)
- `prefer 'std::numbers::pi' to this macro, differs by '2.34e-2'` (if we are replacing a macro that is just a literal)
- `prefer 'std::numbers::inv_sqrt3' to this macro` (for things like `#define INV_SQRT3 1 / bar::sqrt(3)`)

So the messages include what kind of symbol/code is being replaced, the inserted code, and the difference to the inserted value if the replaced code is a literal or a macro that is a literal. The difference that is being printed is the absolute difference. Does the sign matter?

https://github.com/llvm/llvm-project/pull/66583


More information about the cfe-commits mailing list