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