[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

[Florin Iucha via Phabricator via cfe-commits]

0x8000-0000 marked 2 inline comments as done.
0x8000-0000 added inline comments.


================
Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:76-86
+  IgnoredFloatingPointValues.reserve(IgnoredFloatingPointValuesInput.size());
+  IgnoredDoublePointValues.reserve(IgnoredFloatingPointValuesInput.size());
+  for (const auto &InputValue : IgnoredFloatingPointValuesInput) {
+    llvm::APFloat FloatValue(llvm::APFloat::IEEEsingle());
+    FloatValue.convertFromString(InputValue, DefaultRoundingMode);
+    IgnoredFloatingPointValues.push_back(FloatValue.convertToFloat());
+
----------------
aaron.ballman wrote:
> 0x8000-0000 wrote:
> > aaron.ballman wrote:
> > > This is where I would construct an `APFloat` object from the string given. As for the semantics to be used, I would recommend getting it from `TargetInfo::getDoubleFormat()` on the belief that we aren't going to care about precision (explained in the documentation).
> > Here is the problem I tried to explain last night but perhaps I wasn't clear enough.
> > 
> > When we parse the input list from strings, we have to commit to one floating point value "semantic" - in our case single or double precision.
> > 
> > When we encounter the value in the source code and it is captured by a matcher, it comes as either one of those values.
> > 
> > Floats with different semantics can't be directly compared - so we have to maintain two distinct arrays.
> > 
> > If we do that, rather than store APFloats and sort/compare them with awkward lambdas, we might as well just use the native float/double and be done with it more cleanly.
> >When we encounter the value in the source code and it is captured by a matcher, it comes as either one of those values.
> 
> It may also come in as long double or __float128, for instance, because there are type suffixes for that.
> 
> > Floats with different semantics can't be directly compared - so we have to maintain two distinct arrays.
> 
> Yes, floats with different semantics cannot be directly compared. That's why I said below that we should coerce the literal values.
> 
> > If we do that, rather than store APFloats and sort/compare them with awkward lambdas, we might as well just use the native float/double and be done with it more cleanly.
> 
> There are too many different floating-point semantics for this to be viable, hence why coercion is a reasonable behavior.
Let me see if I understood it - your proposal is: store only doubles, and when a floating-point literal is encountered in code, do not use the FloatingLiteral instance, but parse it again into a double and compare exactly. If the comparison matches - ignore it.

In that case what is the value of storing APFloats with double semantics in the IgnoredValues array, instead of doubles?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49114