Improvements in convert and format handling #1936

edleno2 · 2021-05-29T03:20:03Z

Description

Added FormatException and code to report formatting errors in Parse routines, Updated versions and interop structure.
Replace some current exceptions with FormatException to be consistent with .net core.
Replaced some other generic exceptions with ArgumentOutOfRangeException to be consistent with .net core.
Some cleanup generated by CLANG format as headers were modified to allow new exception type.
Fixed handling of signed and unsigned Parse strings - allowed "-0" for example. Refactored some code to allow this.
Added errno processing for integers to catch formatting errors from native math functions.
Corrected min/max check logic for integer and float logic.
Correctly handle (.net core compliant) empty strings and/or space preceding string values about to be parsed.
Report cast errors as invalid cast - .net core compliant.
Added System.IO.Ports to STM32F429I-Discovery device in variants.make so it would generate correct includes when running cmake.

Motivation and Context

Fixes UnitTestParseTests.box_unbox_Test_1 fails Home#708

How Has This Been Tested?

Ran unit tests on Win32 with both SUPPORT_ANY_BASE_CONVERSION and without that, then ESP32-WROOM and STM32F429I-Discovery.

Screenshots

Types of changes

Improvement (non-breaking change that improves a feature, code or algorithm)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Config and build (change in the configuration and build system, has no impact on code or features)
Dependencies (update dependencies and changes associated, has no impact on code or features)
Unit Tests (work on Unit Tests, has no impact on code or features)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

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

…nto develop

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.

…c0f-9575-4847-8842-0c97899413f7 Code style fixes for nanoframework/nf-interpreter PR#1928

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.

…nto develop

This reverts commit c29ea88.

Add a FormatException and edits for formatting of int and doubles. Fix #708

…nto develop

Win32 builds worked, but not ESP32 and STM32. Fixed (1) missing API in target cmake-variants.json; (2) moved location of <cerrno> declaration to allow it to be defined before redefines happen for "errno". Fix #708

Some code was simplified by combing the handling of both positive and negative results into one routine that checked for adjusting the sign at the end just before returning a value. Remove the code that was commented out now that all unit testing is complete. Fix #708

edleno2 · 2021-05-29T05:28:17Z

Trying to find another way to use errno - the lwip library has overridden the functionality of errno processing and I'm not sure how to replace it AND have it work. I can fix the build, but then the unit tests fail when run on hardware.

src/CLR/CorLib/corlib_native.cpp

targets/ChibiOS/ST_STM32F429I_DISCOVERY/cmake-variants.json

josesimoes · 2021-05-29T08:56:13Z

src/CLR/Core/CLR_RT_HeapBlock.cpp

@@ -893,7 +893,7 @@ HRESULT CLR_RT_HeapBlock::PerformUnboxing(const CLR_RT_TypeDef_Instance &cls)

    if (this->DataType() != DATATYPE_OBJECT)
    {
-        NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
+        NANOCLR_SET_AND_LEAVE(CLR_E_INVALID_CAST);


Are we sure that it's safe to replace this exception? I'm asking because I recall this being used in the generics processing and not sure about the impact at this time... So, just asking.

The current hResult of CLR_E_WRONG_TYPE generates a generic Exception, so any managed code doing a Catch (Exception ex) will continue to work. I could not find any references to the CLR_E_WRONG_TYPE in nanoCLR that was looking at that specific HR and acted different - i.e. all of the references were to setting that hResult. The invalid cast exception IS what is being returned in .net core in the same situation, so that led me to the change after checking if there were references to the old hr.

josesimoes · 2021-05-29T09:33:49Z

@edleno2 just rebased and fix some style issues. As I couldn't push to your fork, I've made this in branch "pr1936".
Please pull from it.

josesimoes · 2021-05-29T09:48:30Z

@edleno2 just fixed the build issue with errno. Pull from pr1936 😉

Fix #708

nfbot · 2021-05-29T20:59:43Z

@edleno2 there are issues with the code style on the source files.
A PR was submitted with the code style fixes. Please click https://github.com/edleno2/nf-interpreter/pull/3, review the changes if you want and merge it.

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.

***NO_CI***

nfbot · 2021-05-30T02:52:26Z

@edleno2 there are issues with the code style on the source files.
A PR was submitted with the code style fixes. Please click https://github.com/edleno2/nf-interpreter/pull/4, review the changes if you want and merge it.

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.

When compiled with devices that use LWIP for networking certain stdlib routines are required to be reentrant. Cleaned up a little left over formatting in nf_errors_exceptions.h Fix #708

nfbot · 2021-05-31T06:34:23Z

@edleno2 there are issues with the code style on the source files.
A PR was submitted with the code style fixes. Please click https://github.com/edleno2/nf-interpreter/pull/5, review the changes if you want and merge it.

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.

edleno2 · 2021-05-31T06:38:35Z

Closing this PR - made a mess of it while trying to rebase. Will open a new one.

edleno2 and others added 16 commits May 19, 2021 15:22

Merge branch 'develop' of https://github.com/edleno2/nf-interpreter i…

a8de032

…nto develop

Fixes for failing units tests

c574697

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

Merge branch 'nanoframework:develop' into develop

44e29f6

Code style fixes

faaf7b7

Automated fixes for code style.

Merge pull request #1 from nanoframework/nfbot/clang-format-fix/a8ad7…

2ac8c41

…c0f-9575-4847-8842-0c97899413f7 Code style fixes for nanoframework/nf-interpreter PR#1928

Use invalid cast exception for bad casts

c29ea88

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.

Merge branch 'develop' of https://github.com/edleno2/nf-interpreter i…

5bc49c9

…nto develop

Resolved conflicts with nfbot code style

c89a6d8

Revert "Use invalid cast exception for bad casts"

63ce22c

This reverts commit c29ea88.

Merge branch 'nanoframework:develop' into develop

d909094

Merge branch 'nanoframework:develop' into develop

3828e31

Fix unit test problems with CorLibrary

0032eee

Add a FormatException and edits for formatting of int and doubles. Fix #708

Merge branch 'develop' of https://github.com/edleno2/nf-interpreter i…

f3857e9

…nto develop

Fix building for ESP32 and STM32

079319a

Win32 builds worked, but not ESP32 and STM32. Fixed (1) missing API in target cmake-variants.json; (2) moved location of <cerrno> declaration to allow it to be defined before redefines happen for "errno". Fix #708

josesimoes reviewed May 29, 2021

View reviewed changes

src/CLR/CorLib/corlib_native.cpp Show resolved Hide resolved

josesimoes reviewed May 29, 2021

View reviewed changes

targets/ChibiOS/ST_STM32F429I_DISCOVERY/cmake-variants.json Outdated Show resolved Hide resolved

josesimoes reviewed May 29, 2021

View reviewed changes

nfbot added Area: Config-and-Build Type: bug Type: enhancement labels May 29, 2021

josesimoes changed the title ~~Fixes to unit tests for CoreLibrary~~ Improvements in convert and format handling May 29, 2021

josesimoes changed the base branch from develop to pr1936 May 29, 2021 09:30

edleno2 added 3 commits May 29, 2021 12:34

update NoReflection checksum for mscorlib

f37abe2

Fix #708

Revert change to cmake-variants.json

9573b54

Undo format changes caused by clang format.

815921f

edleno2 added 2 commits May 29, 2021 13:50

Merge in changes from pr1936 branch

5b8a6ff

reverted cmake-variants.json change

f16d0bc

networkfusion and others added 2 commits May 29, 2021 23:12

Add missing API's to target files (#1937)

f6d0b16

***NO_CI***

Merge branch 'nanoframework:develop' into develop

4811213

Fix strtoll and strtoull to use reentrant versions when needed

d9afbb2

When compiled with devices that use LWIP for networking certain stdlib routines are required to be reentrant. Cleaned up a little left over formatting in nf_errors_exceptions.h Fix #708

edleno2 closed this May 31, 2021

nfbot added invalid and removed Area: Config-and-Build Type: bug Type: enhancement labels May 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements in convert and format handling #1936

Improvements in convert and format handling #1936

edleno2 commented May 29, 2021 •

edited by josesimoes

Loading

edleno2 commented May 29, 2021

josesimoes May 29, 2021

edleno2 May 29, 2021

josesimoes commented May 29, 2021

josesimoes commented May 29, 2021

nfbot commented May 29, 2021

nfbot commented May 30, 2021

nfbot commented May 31, 2021

edleno2 commented May 31, 2021

Improvements in convert and format handling #1936

Improvements in convert and format handling #1936

Conversation

edleno2 commented May 29, 2021 • edited by josesimoes Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots

Types of changes

Checklist:

edleno2 commented May 29, 2021

josesimoes May 29, 2021

Choose a reason for hiding this comment

edleno2 May 29, 2021

Choose a reason for hiding this comment

josesimoes commented May 29, 2021

josesimoes commented May 29, 2021

nfbot commented May 29, 2021

nfbot commented May 30, 2021

nfbot commented May 31, 2021

edleno2 commented May 31, 2021

edleno2 commented May 29, 2021 •

edited by josesimoes

Loading