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

remap offsets from bytes to chars during source map conversion #2661

Closed
wants to merge 2 commits into from

Conversation

bohendo
Copy link
Contributor

@bohendo bohendo commented Feb 5, 2025

Remaps byte offsets to char offsets in the _convert_source_mapping method of the Source class.

This method now:

  • uses utf-8 encoding to convert the source code string to a bytes array
  • applies the byte offsets
  • decodes back to a normal string
  • uses the length of the resulting string to determine new char-denominated offsets

Assumptions:

  • this method is only ever used to ingest output from the solc compiler, which infamously outputs byte-denominated src map offsets.
  • other platforms (eg foundry, hardhat) already provide char-denominated src map offsets

Problem: if f not in sourceUnits then we don't have reliable access to the source filename or code so we can't use the aforementioned method to convert to byte offsets. It's not clear to me when or why this logic branch would be taken, however.

@bohendo bohendo force-pushed the fix-unicode-src-mappings branch from 4d7f623 to 5940ef8 Compare February 5, 2025 20:36
@bohendo bohendo force-pushed the fix-unicode-src-mappings branch from 5940ef8 to 3404b76 Compare February 5, 2025 20:37
Comment on lines 154 to +156
def _convert_source_mapping(
offset: str, compilation_unit: "SlitherCompilationUnit"
) -> Source: # pylint: disable=too-many-locals
) -> Source:
Copy link
Contributor

Choose a reason for hiding this comment

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

[blackfmt] reported by reviewdog 🐶

Suggested change
def _convert_source_mapping(
offset: str, compilation_unit: "SlitherCompilationUnit"
) -> Source: # pylint: disable=too-many-locals
) -> Source:
def _convert_source_mapping(offset: str, compilation_unit: "SlitherCompilationUnit") -> Source:

@bohendo
Copy link
Contributor Author

bohendo commented Feb 7, 2025

This PR demonstrates a deep misunderstanding of the underlying problem

@bohendo bohendo closed this Feb 7, 2025
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.

1 participant