Page MenuHomePhabricator

[APFloat] Prevent construction of APFloat with Semantics and FP value
ClosedPublic

Authored by ekatz on Nov 19 2019, 2:47 AM.

Details

Summary

Constructor invocations such as APFloat(APFloat::IEEEdouble(), 0.0) may seem like they accept a FP (floating point) value, but the overload they reach is actually the integerPart one, not a float or double overload (which only exists when fltSemantics isn't passed).

This may lead to possible loss of data, by the conversion from float or double to 'integerPart'.

To prevent future mistakes, a new constructor overload, which accepts any FP value and marked with delete, to prevent its usage.

Fix PR34095.

Diff Detail

Event Timeline

ekatz created this revision.Nov 19 2019, 2:47 AM

Perhaps there should be an "= delete" constructor to make this a compile error?

Perhaps there should be an "= delete" constructor to make this a compile error?

I agree. This seems, as evidenced by our current code, confusing. Make we should have a private constructor for this and clients need to use some APFloat::createFromInt or similar to get this behavior?

ekatz updated this revision to Diff 230099.Nov 19 2019, 10:25 AM
ekatz retitled this revision from [APFloat] Fix construction of APFloat with FP value instead of integer to [APFloat] Prevent construction of APFloat with Semantics and FP value.
ekatz edited the summary of this revision. (Show Details)

Added a new constructor overload, which accepts any FP value and marked with delete, to prevent its usage.

This must be done with SFINAE, otherwise (a simple overload for double/float) we get "ambiguous call to overloaded function", unless we overload each specific type we allow (not only integerPart).

efriedma accepted this revision.Dec 3 2019, 5:05 PM

LGTM

This revision is now accepted and ready to land.Dec 3 2019, 5:05 PM
This revision was automatically updated to reflect the committed changes.