Issue 9357 - Better diagnostic on float literals #1520
Conversation
ghost
mentioned this pull request
| @@ -2475,6 +2475,9 @@ char *RealExp::toChars() | |||
| if (type->isimaginary()) | |||
| strcat(buffer, "i"); | |||
|
|
|||
| if (((real_t)(dinteger_t)value) == value) | |||
There was a problem hiding this comment.
Unfortunately this doesn't actually work, big values are not printed as integers. Maybe use strchr instead and search for a . or e?
There was a problem hiding this comment.
Can you show a test-case where it fails?
There was a problem hiding this comment.
Well, int x = 1048576.0L;. Large powers of two are representable in floating point types but are not printed as integers.
ghost
closed this
| @@ -2465,7 +2465,7 @@ void ErrorExp::toCBuffer(OutBuffer *buf, HdrGenState *hgs) | |||
|
|
|||
| char *RealExp::toChars() | |||
| { | |||
| char buffer[sizeof(value) * 3 + 8 + 1 + 1]; | |||
| char buffer[256]; | |||
There was a problem hiding this comment.
Why introduce a magic number? Instead it needs a comment. sizeof(value)*3 is because each byte of mantissa is max of 256 (3 characters). The string will be "-M.MMMMe-4932". (ie, 8 chars more than mantissa). Plus one for trailing \0. Plus one for rounding.
There was a problem hiding this comment.
That's what I asked about in the last pull, but I didn't get any proper answers. Should have cc'd you.
There was a problem hiding this comment.
@donc This formula just gives you a rough upper bound, managing to introduce four magic numbers instead of one. Why bother with this nonsense when you can just allocate a huge chunk of stack space? Are we planning to compile dmd for an 8-bit micro with 256 bytes of stack?
There was a problem hiding this comment.
None of those numbers are arbitrary. That's the point. Note that real doesn't have fixed length, it's just bigger than a double. It might be a 128-bit quadruple. Is 256 bytes enough?
Probably, but you have to do a calculation like this one to work it out. I'd rather not do that calculation everytime I read the code.
ghost
reopened this
|
This as far as I could get it done while trying to decypher the specs on this page. Format specifiers are unbelievably convoluted. Edit: Actually the only thing that's necessary is to make at least one trailing zero present if the floating-point is equal to an integral. I'm not sure what the format spec for that would be. |
ghost
closed this
ghost
reopened this
ghost
closed this
http://d.puremagic.com/issues/show_bug.cgi?id=9357
Opened as a more-general solution to #1514.