Skip to content

Conversation

@Pirulax
Copy link
Contributor

@Pirulax Pirulax commented Mar 10, 2021

The new parser didn't deal correctly with NaN, under/overflows and signed to unsigned conversions.
Now it will SetBadArgument if the read number is:

  • negative and T is unsigned
  • NaN (previously this was only done for T = double/float)
  • inf

NOTE/TODO: We really should use C++20's std::in_range here
For now this doesn't do all the safety checks, but this should be "good enough" [until we switch to C++20]

@Pirulax Pirulax requested a review from sbx320 March 10, 2021 21:02
@Pirulax Pirulax force-pushed the hotfix/new-parser-unsigned-number branch from 5d96857 to 86de824 Compare March 10, 2021 21:48
@patrikjuvonen patrikjuvonen added the enhancement New feature or request label Mar 17, 2021
@Pirulax Pirulax marked this pull request as draft April 8, 2021 17:47
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2022

This draft pull request is stale because it has been open for at least 90 days with no activity. Please continue on your draft pull request or it will be closed in 30 days automatically.

@github-actions github-actions bot added the stale Inactive for over 90 days, to be closed label Jan 7, 2022
@github-actions github-actions bot closed this Feb 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2022

This draft pull request was closed because it has been marked stale for 30 days with no activity.

@Pirulax Pirulax reopened this Sep 11, 2022
@Pirulax Pirulax marked this pull request as ready for review September 11, 2022 16:11
@patrikjuvonen patrikjuvonen removed the stale Inactive for over 90 days, to be closed label Dec 25, 2022
@Pirulax Pirulax changed the title Detect integral under/overflow and NaN in new parser Detect integral underflow, NaN and inf values in the new parser Jul 2, 2023
@sbx320 sbx320 merged commit d07baa3 into multitheftauto:master Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants