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

Improvements in convert and format handling #1938

Merged
merged 37 commits into from
May 31, 2021
Merged

Improvements in convert and format handling #1938

merged 37 commits into from
May 31, 2021

Conversation

edleno2
Copy link
Contributor

@edleno2 edleno2 commented May 31, 2021

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/code-style of lists in exceptions header file.
  • 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.

Motivation and Context

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.

edleno2 and others added 30 commits May 19, 2021 15:22
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.
…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.
Add a FormatException and edits for formatting of int and doubles.

Fix #708
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
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
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.
Automated fixes for code style.
Add a FormatException and edits for formatting of int and doubles.

Fix #708
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 and others added 2 commits May 29, 2021 19:49
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
Automated fixes for code style.
@nfbot
Copy link
Member

nfbot commented May 31, 2021

@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/6, 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.

…02b-9a25-4604-bbc0-010311b101e3

Code style fixes for nanoframework/nf-interpreter PR#1938
@@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe these to be safe changes - the CLR_E_WRONG_TYPE returns a generic Exception type, so any try/catch in C# code will work the same. I could not find any instances of CPP code specifically looking for CLR_E_WRONG_TYPE - everything I could find was just setting the value.

Copy link
Member

Choose a reason for hiding this comment

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

OK. Let's take those. If any changes are required in generics, I'll deal with those.

@edleno2 edleno2 marked this pull request as ready for review May 31, 2021 07:09
Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

Awesome!!

Automated fixes for code style.
@nfbot
Copy link
Member

nfbot commented May 31, 2021

@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/7, 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.

…11c-95bf-4d84-9de8-8fa8f2275231

Code style fixes for nanoframework/nf-interpreter PR#1938
@josesimoes josesimoes enabled auto-merge (squash) May 31, 2021 07:55
@josesimoes josesimoes merged commit c14ec56 into nanoframework:develop May 31, 2021
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.

UnitTestParseTests.box_unbox_Test_1 fails
3 participants