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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
