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

Port should validate that names are sanitary #234

Closed
mkorbel1 opened this issue Dec 21, 2022 · 2 comments · Fixed by #245
Closed

Port should validate that names are sanitary #234

mkorbel1 opened this issue Dec 21, 2022 · 2 comments · Fixed by #245
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@mkorbel1
Copy link
Contributor

Describe the bug

When creating a Port, the expectation is that the name will not change. Since Port extends from Logic, and Logic was updated to prevent names from being unsanitary by renaming them using the Sanitizer, it is now possible that the name will be sanitized silently, leading to bugs down the line.

Usually, it's not a good idea to name a port an unsanitary name ever, but sometimes it makes sense to want to name it something unsanitary if you know it will always be uniquified such that it will not actually be unsanitary on an interface.

To Reproduce

Name a Port a SystemVerilog keyword (e.g. "end"), uniqufiy it so that it's a legal name on an interface, then attempt to access port("end")

Expected behavior

When constructing a Port, it will immediately throw an exception if the name is not sanitary.

Actual behavior

Port name is silently sanitized, making port calls unable to find the port.

Additional: Dart SDK info

No response

Additional: pubspec.yaml

No response

Additional: Context

Exists/introduced in ROHD v0.4.1

@mkorbel1 mkorbel1 added bug Something isn't working good first issue Good for newcomers labels Dec 21, 2022
@quekyj
Copy link
Contributor

quekyj commented Dec 22, 2022

@mkorbel1 let me investigate this.

@quekyj
Copy link
Contributor

quekyj commented Dec 29, 2022

_checkForSafePortName

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants