Skip to content

Conversation

@fourcolor
Copy link
Contributor

@fourcolor fourcolor commented Jul 13, 2025

Previously, literals with uppercase 0X prefix (e.g., "0XABC") were misparsed as invalid. This commit fixes that and improves compatibility with C99-style numeric constants.

  • Removed incorrect inclusion of 'x' as a hex digit in is_hex()
  • Updated is_numeric() to skip 0x/0X prefix when validating hex digits
  • Adjusted read_numeric_constant() to consistently handle both 0x and 0X prefixes

Summary by Bito

This pull request enhances numeric literal parsing by adding support for uppercase 0X hexadecimal prefixes, correcting previous misparsing issues. Modifications were made to the is_hex and is_numeric functions for better validation, and the read_numeric_constant function was updated for consistent handling of both prefix styles, improving C99 compatibility. New tests have been included to ensure the accuracy of these changes.

Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Run clang-format -i prior to submit.

@jserv jserv requested review from ChAoSUnItY and DrXiao July 13, 2025 07:29
Previously, literals with uppercase 0X prefix (e.g., "0XABC") were misparsed as invalid.
This commit fixes that and improves compatibility with C99-style numeric constants.

- Removed incorrect inclusion of 'x' as a hex digit in is_hex()
- Updated is_numeric() to skip 0x/0X prefix when validating hex digits
- Adjusted read_numeric_constant() to consistently handle both 0x and 0X prefixes
@fourcolor fourcolor force-pushed the fix/hex-0X-support branch from b6c708a to 49bcd8b Compare July 13, 2025 07:44
Copy link
Collaborator

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Consolidate the test suite to reflect the proposed changes.


bool is_hex(char c)
{
return ((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || c == 'x' ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about adding c == 'X' to the expression instead of removing c == 'x' ?

If so, the proposed changes could be refined accordingly.

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 think we should avoid treating 'x' or 'X' as valid hex digits, since they are part of the prefix (0x or 0X) but not valid hex characters themselves.

Add tests to verify valid and invalid forms of hex literals.
@jserv jserv merged commit 9643530 into sysprog21:master Jul 14, 2025
6 checks passed
@jserv
Copy link
Collaborator

jserv commented Jul 14, 2025

Thank @fourcolor for contributing!

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.

3 participants