conrade-ctc wrote: Oh, wow, @AaronBallman, in adding a barrage of tests, I found some pre-existing I found 3 existing bugs in `APFloat`
> > Generally LGTM though I did not check the implementation of the algorithm > > very carefully. The results look correct to me in terms of the testing > > changes, but I think there are some tests we should add: > > ``` > > double bar = 0x10.1p0; > > ``` > > > > > > > > > > > > > > > > > > > > > > > > do we correctly round-trip that or do we still convert that to a decimal > > value instead of the hex literal? > > Another question would be: do we have sufficient test coverage for some of > > the odder floating-point formats like double double or all the tiny > > floating point types for AI? > > Good point. I'll add a barrage of corner case tests, compiling a list now, > and I'll ping when they are pushed to the PR. @AaronBallman, radix is not preserved in round-trip... that requires a bit more surgery (need radix field in AST nodes, etc). Okay with you to post an issue for that one, and follow-up with a separate PR, or do you think it's worth adding to this PR? Also, glad you pressed on wider test coverage, other that the existing bug @efriedma-quic stressed in the comment above, I stumbled on another existing bug in `toString`... I'll add another issue for that one, and follow up with a separate PR to address, unless you think it makes sense to pack it all into this PR. What do you think: add the issues and follow up with PRs, or get them into this one? https://github.com/llvm/llvm-project/pull/218471 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
