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

entity_name of an instantiated unit may be a simple name #70

Closed
c-thaler opened this issue Apr 5, 2023 · 7 comments · Fixed by #71
Closed

entity_name of an instantiated unit may be a simple name #70

c-thaler opened this issue Apr 5, 2023 · 7 comments · Fixed by #71
Labels
bug Something isn't working

Comments

@c-thaler
Copy link
Contributor

c-thaler commented Apr 5, 2023

In a direct entity instantiation, you can also use a simple name as entity_name, without the library prefix (seledted_name). However, EntityInstantiationSymbol uses a SelectedName in its constructor.

class EntityInstantiationSymbol(SelectedName, Symbol):

What's the best way to fix this? Simply set the prefix to work in the simple name case?

@Paebbels
Copy link
Member

Paebbels commented Apr 6, 2023

I know some tools that gave me an error on a simple_name compared to a selected_name. I never questioned that. I also know no example that used just a simple name. Also in configurations a selected_name is used like work.Counter.

I checked that LRM of VHDL-2019 and I can't find any requirement for a selected_name, a simple_name seams to be enough.

OTOH, in the syntax rules *entity_*name is also used in an architecture declaration
architecture identifier of entity_name is. For sure, here a selected name is not possible, but the syntax rules don't forbid it ...

@tgingold can you confime that direct entity instantiation also allows simple names?

@c-thaler if you finding is correct, the model needs to be changed to support "any" name: simple and selected.


@c-thaler should I prioritize names and symbols in my next update? Would it help you (and pyGHDL.dom) in your project?

@Paebbels Paebbels added the question Further information is requested label Apr 6, 2023
@c-thaler
Copy link
Contributor Author

c-thaler commented Apr 6, 2023

Personally, I've never seen a simple name in this place before. I'm used to selected names like work.Counter, too.
However, it is silicon proven code. It went through some synthesis tools in several projects. Quartus works, too.

@c-thaler should I prioritize names and symbols in my next update? Would it help you (and pyGHDL.dom) in your project?
It may help. I'm trying to process all the code in my project. I run into an issue and fix it, then run into the next issue. Hard to tell where the next issue will be.

@Paebbels
Copy link
Member

Paebbels commented Apr 6, 2023

I believe it will help, because in my last rework session of pyVHDLModel and pyGHDL.dom, I needed to disable some symbols and names as I changed the concept. Not all of it is back to its previous feature set.

In the old implementation, there was a concept mistake, so I reworked how symbols and names work together. The changed concept and the necessary modifications need to be applied, so commented lines could be reactivated.

@tgingold
Copy link

tgingold commented Apr 7, 2023 via email

@Paebbels
Copy link
Member

Paebbels commented Apr 7, 2023

@tgingold thanks for clarification. Then I need to fix pyVHDLModel.

In case of: architecture arch of work.ent is ...
is only work... allowed or also any libraries. I remember that secondary units need to be in the same library as their primaries. At least we discussed it somehow years ago for VHDL-2019. Maybe it was just for configurations.

@Paebbels Paebbels added bug Something isn't working and removed question Further information is requested labels Apr 7, 2023
@tgingold
Copy link

tgingold commented Apr 8, 2023 via email

@Paebbels
Copy link
Member

This is also related to ghdl/ghdl#2308.

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

Successfully merging a pull request may close this issue.

3 participants