Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to avoid a compile error due to float to int truncation with GCCGO #9233

Merged
merged 3 commits into from
Dec 19, 2014

Conversation

inatatsu
Copy link
Contributor

The unit test pkg/units/size_test.go fails with GCCGO due to a compilation error:

inagaki@black3:~/upstream/src/github.com/docker/docker/pkg/units$ go version
go version gccgo (GCC) 5.0.0 20141029 (experimental) linux/amd64
inagaki@black3:~/upstream/src/github.com/docker/docker/pkg/units$ go test
# _/home/inagaki/upstream/src/github.com/docker/docker/pkg/units
./size_test.go:28:43: error: floating point constant truncated to integer
  assertEquals(t, "2.22 PB", HumanSize(2.22*PB))
                                           ^
FAIL    _/home/inagaki/upstream/src/github.com/docker/docker/pkg/units [build failed]
inagaki@black3:~/upstream/src/github.com/docker/docker/pkg/units$

According to the discussion on GCC Bugzilla, this is due to the difference between GCCGO and GC in the implementations of floating point constant expression. The language specifies that a compiler raises an error when a floating point constant expression is truncated to integer, a precision of the internal representation is implementation-dependent. While my workaround is modifying size_test.go as:

assertEquals(t, "2.22 PB", HumanSize(int64(float64(2.22*PB))))

, is it better to modify the parameter of HumanSize() from int64 to float64 like BytesSize()?
Signed-off-by: Tatsushi Inagaki e29253@jp.ibm.com

Signed-off-by: Tatsushi Inagaki <e29253@jp.ibm.com>
@crosbymichael
Copy link
Contributor

I would probably change HumanSize to take a float64

…ith GCCGO"

This reverts commit 967a42f.

Signed-off-by: Tatsushi Inagaki <e29253@jp.ibm.com>

Roll back the change to fix the parameter of HumanSize from int64 to float64
…oat to int truncation

Signed-off-by: Tatsushi Inagaki <e29253@jp.ibm.com>
@inatatsu
Copy link
Contributor Author

Modified HumanSize() to take a float64 parameter and added casts to float64 around the arguments for HumanSize() .

@inatatsu
Copy link
Contributor Author

inatatsu commented Dec 1, 2014

@vieux @jfrazelle Do we need additional changes to merge this PR? This problem can occur even with GC depending on the value of floating point constant.

@vbatts
Copy link
Contributor

vbatts commented Dec 11, 2014

LGTM

@inatatsu
Copy link
Contributor Author

@crosbymichael ping?

@crosbymichael
Copy link
Contributor

LGTM

crosbymichael added a commit that referenced this pull request Dec 19, 2014
Fix to avoid a compile error due to float to int truncation with GCCGO
@crosbymichael crosbymichael merged commit 2acb856 into moby:master Dec 19, 2014
@tiborvass
Copy link
Contributor

@crosbymichael Unit tests are failing, why did we merge this?

@unclejack
Copy link
Contributor

This wasn't needed any more. A much smaller PR was #9715 and it fixed the same problem. This PR introduced a conflict with master once that was merged.

I'm going to send a revert PR for this one.

@tiborvass
Copy link
Contributor

FYI, instead of reverting this PR, we fixed the compile errors in the unit tests.

@inatatsu inatatsu deleted the fix-pkg-units-size-for-gccgo branch February 29, 2016 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants