Skip to content

Conversation

jmcabo
Copy link
Contributor

@jmcabo jmcabo commented Nov 25, 2013

Fixed -O -inline and -release incompatibilty with Orange. (inlining was omitting the 'else' block of the static ifs in toData and fromData).
I really needed to make it work with -O -inline, and I'm happy that I've found the problems. If there is a better way, please use that instead.

Also added an automatic "unittest.bat" runner for Windows ("ported" unittest.sh, gathering *.d in the same way in a batch file).

The "Primitive" test now passes in Windows, though the FP string zero representation should actually be portable? (Maybe I have a locale difference? "0x0.p+0" vs "0x0p+0").

--Juan Manuel Cabo

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this issue reported?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but tests failed when -O -inline was added to unittest.sh (and there was an obscure comment in the Makefile's). The output was the following:

    orange.serialization.SerializationException.SerializationException@orange\serial
    ization\SerializationException.d(25): Could not find an element "array" with the
     attribute "key" with the value "arr".
    ----------------
    0x004273EA in D6orange13serialization10Serializer10Serializer6__ctorMFC6orange13
    serialization8archives7Archive7ArchiveZC6orange13serialization10Serializer10Seri
    alizer9__lambda2MFNaNfC6orange13serialization22SerializationException22Serializa
    tionExceptionZv
    ----------------

when compiling with -g -O -inline -release.
--jm

@jmcabo
Copy link
Contributor Author

jmcabo commented Nov 25, 2013

Please follow the surrounding style. I would skip the braces completely. Also indent properly.

Fixed Primitive.d and added to pull request.
--jm

@jacob-carlborg
Copy link
Owner

Could you please squash these commits in to one. Or perhaps two, one for the Windows unit test file and one for the rest.

@jmcabo
Copy link
Contributor Author

jmcabo commented Nov 26, 2013

No problem, I'll create one branch for the unittest.bat file and
another for the -O -inline fix with Primitive.d also.
--jm

@jmcabo
Copy link
Contributor Author

jmcabo commented Nov 27, 2013

I'm a little busy today, I won't have time to separate the commits until tonight.
Thanks for resolving the other issues!!
--jm

@jacob-carlborg
Copy link
Owner

No rush.

@jacob-carlborg
Copy link
Owner

Are the commits squashed? I would prefer to not have an extra merge commit. Could you please do a rebase instead of a merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants