[PATCH] D65912: [clang-tidy] Add new check for math constants

Fri Aug 9 05:14:46 PDT 2019

aaron.ballman added a comment.

In D65912#1621898 <https://reviews.llvm.org/D65912#1621898>, @ZaMaZaN4iK wrote:

> The main reason why I've created this differential - asking to you about usefulness of this check for clang-tidy. I understand that there are a some TODO and formatting issues - it's ok for now.
>
> As far I understand your remarks - you are not against this check so I can continue my work on this path. Great.

I think the idea is a reasonable one, yes. One thing I would be interested in knowing is how often the check behaves when run over some large, real-world code bases. Does it catch any true positives? Does it have false positives?

> Let's discuss about some open questions:
> 
> 1. Should we support C language and <math.h> header file? I think - yes, it's quite easy to implement.

I think so, yes.

> 1. Should we support converting to Boost.Constants? I don't think so.

Perhaps as a follow-up patch, if someone was interested in doing the work.

> 1. In header file instead of defining math constants directly should we use them from <cmath> as much as possible?

Yes, but you should probably ignore system header files so that you don't flag implementations providing their own sets of constants.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65912/new/

https://reviews.llvm.org/D65912