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

Feat: Tokenize tool directives #188

Merged
merged 4 commits into from
Sep 9, 2023

Conversation

Schottkyc137
Copy link
Contributor

Closes #179

Tokenize tool directives of the form

`identifier { arbitrary text }

In the current implementation status, this PR only tokenizes the tool directives. Any further processing is omitted for now. In the future, standard tool directives like the protect tool directive could be used for analysis

@kraigher
Copy link
Member

kraigher commented Sep 8, 2023

Hi @Schottkyc137 I have some more time now after summer vacation so I will start to process your PRs. Sorry for the delay.

@kraigher
Copy link
Member

kraigher commented Sep 8, 2023

The code looks good and I have no problem merging it. One follow up question I have is if tool directives always are a single line by standard definition? Or is parsing to the end of line just a heurestic you have assumed? Will encrypted IP just have a single line for the encrypted blob?

@Schottkyc137
Copy link
Contributor Author

Schottkyc137 commented Sep 9, 2023

Hi @Schottkyc137 I have some more time now after summer vacation so I will start to process your PRs. Sorry for the delay.

Good to see you back, I hope you had a nice vacation!

The code looks good and I have no problem merging it. One follow up question I have is if tool directives always are a single line by standard definition? Or is parsing to the end of line just a heurestic you have assumed? Will encrypted IP just have a single line for the encrypted blob?

The standard (LRM 15.11) states that

A tool directive starts with a grave accent character and extends up to the end of the line.

I hope that this is how the encrypted IP will behave. The examples that I have seen in the issues weren't only occupying one line but this could just as well be a copy-and paste artifact somewhere. At the moment, I can't test this with Vivado but I will probably be able to do so later today or tomorrow. So if you want to, you can postpone merging until this is clear.

@kraigher kraigher merged commit 6681f48 into VHDL-LS:master Sep 9, 2023
8 checks passed
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.

[Bug] Encryption envelopes flagging many errors despite being valid VHDL
2 participants