Fixes in System.Convert #1928
Fixes in System.Convert #1928
Conversation
Removed call to GetIntegerPart since it couldn't support UInt64 conversions and edit for invalid values. Wrote new string parse loop based on code that is being used for SUPPORT_ANY_BASE_CONVERSION. Moved some code so it could be used by both SUPPORT_ANY... and the radix 10/16 logic. Possible problem change in Double parsing - used pow() method to better generate the IEEE 574 compliant double in storage. pow() is considered to be slow, but without a more precise float multiplication the resulting double will not match what is used by the Roslyn compiler when creating and comparing doubles. Fixes #715
Cleaned up brackets in system convert to be consistent with rest of file. Added "signed" to char for cast operation - win32 treats char as signed, but gcc does not. Adding signed makes this un-ambiguous. Fix #715
Automated fixes for code style.
|
@edleno2 there are issues with the code style on the source files. Make sure to follow the project code style. Check the details here on how it works and the tools required to help you with that. |
|
I've fixed the code style in the original code file so that only the real changes show up in your file. Please pull these. |
|
These changes are OK but the code separation with the ifdef for supporting all base conversions needs to be kept where it is, otherwise it's purpose is lost and the smaller targets wont build because of the math code are added. Please refractor to keep that one there. It's OK to move declarations around, of course. |
…c0f-9575-4847-8842-0c97899413f7 Code style fixes for nanoframework/nf-interpreter PR#1928
|
@edleno2 in case you've missed it, there are conflicts after the merge from the code style fix. |
Unit tests of boxing/unboxing are expecting invalid cast exception. Code was returning generic System.Exception with HR of CLR_E_WRONG_TYPE. Changed code since invalid cast exception is more descriptive of the problem.
This reverts commit c29ea88.
|
@josesimoes I intentionally moved logic that was in the ifndef to be shared for both with/without SUPPORT_ANY_BASE_CONVERSION. For example, checking for min/max is now done for both cases, and some string operations to set up for negative results is moved ahead of the if/else-defined so it applies to both. Tested on win32 with both conditions to make sure the code work. |
|
@edleno2 your intention is perfectly clear! 😃 What you need to make sure (as you add changes) is that only the absolute math API (from CRT) is pulled in for the targets that support only partial conversion. This is because we have to make sure that each image has the smallest possible code size. (that's the intention of the SUPPORT_ANY_BASE_CONVERSION option: support all or partial conversions). |
|
@josesimoes - completed all of the testing. Ran with and without SUPPORT_ANY_BASE_CONVERSION on win32 and made sure both code interpretations got the expected results, and that no extra math API are being included. Also built firmware on stm32f429 and confirmed that all tests passed on that platform (it is a SUPPORT_ANY_BASE_CONVERSION platform). |
josesimoes
changed the title
Description
Reworked parts of System.Convert native code to reuse more of the logic already there for SUPPORT_ANY_BASE_CONVERSION. Removed use of shared GetIntegerPart since it couldn't handle all of the exception cases for signed integers, instead put the code into the NativeToInt64 so it could do all exception checking and handle signed and unsigned values. In NativeToDouble added call to Pow to take 10 to a power since that is more accurate - the original code would be a little faster, but had significant offset from the IEEE 574 standard for generation of double into a 64bit address (ex: min and max values were off from the standard). Code now passes with the standard values.
In nanoprint.c added comments regarding the algorithm used for converting floating numbers and the range of numbers that it can process. Also added "signed" to char cast operation - in win32 a char is treated as signed, but not in GCC. Added "signed" attribute to get the same test results in all platforms.
Motivation and Context
How Has This Been Tested?
Ran NFUnitTestConversions using the nanoFramework Test Framework on Win32, STM32F429I-Discovery, and ESP32-WROOM-32. All tests are passing.
Screenshots
Types of changes
Checklist: