-
Notifications
You must be signed in to change notification settings - Fork 56
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
Error in AlertLogPkg with latest ghdl #53
Comments
AFAIK this error was found some days ago by another user, which reported in the Gitter chat: https://gitter.im/OSVVM/Lobby?at=5f5f0795df4af236f9070b10 A possible current solution is to use the Dev branch of OSVVM which includes a fix / workaround: https://github.com/OSVVM/OSVVM/commits/Dev |
I see. Just wanted to notify about this issue. Closing it, as already aware of. |
VHDL-2008 made code of this sort legal. If someone would like to make a bug report against GHDL, that would be appreciated |
The updates have now been pushed to master. I am also working toward getting GHDL running in the OSVVM scripting environment so I can run the full regression suite on it. |
I think GHDL is correct here.
The inner GetReqId has the same parameter and result type profile (see 4.5.1).
As a consequence (see 12.3) they are homograph.
And the inner function declaration hides the outer one.
So the outer one cannot be directly called.
Because parameters are missing to call the inner one, it is an error.
What am I missing ?
|
The call does not meet the call profile of the inner procedure since it has only one parameter. |
You missed my point. The outer procedure is hidden by the inner procedure as they have the same profile.
|
Their profiles do not lexicographically conform. So how can it hide it? It would only hide the calls that match its exact profile - calling with all parameters. If called with less, it should call the outer one. Right? With respect to importance, this one is low as there is a simple work around. However, I have numerous ones wrt records with unconstrained arrays, ports, using subtype with such ports, ... that are much more important to me. I have not been able to submit a bug report as I am not good enough at GHDL yet. Also all my build scripts are currently TCL based - which is ok if you have an environment that allows you to run tclsh. |
Their profiles do not lexicographically conform. So how can it hide it?
Lexicographical conformance is not involved. If two functions have the same number of parameters, and the corresponding parameters have the same base type and the results have the same base type then they are homograph.
It would only hide the calls that match its exact profile - calling with all parameters. If called with less, it should call the outer one. Right?
No. As soon as the outer declaration is hidden, it is not considered in the overload resolution.
|
There seem to be some inconsistencies in this area. However, 4.5.1 the rule for ambiguity, it is clear that to call is unambiguous: I think that rules of 12.3 are erroneous and should account for initialized parameters and needs to be updated. If 12.3 accounted for this, then the call would be unambiguous. Mentor and Aldec seem to agree with my interpretation. |
I have submitted issue: https://gitlab.com/IEEE-P1076/VHDL-Issues/-/issues/20 against the LRM in this area. Any reason that we should not make the code legal? |
There seem to be some inconsistencies in this area.
no, not really.
However, 4.5.1 the rule for ambiguity, it is clear that to call is unambiguous:
You need also to consider 12.5 The context of overload resolution, which clearly states that 'all the visible declarations are considered'. As the outer one is not visible, it is not considered.
|
Any reason that we should not make the code legal?
Why do we need to change the LRM ? If you really want to call the outer function, just use an expanded name.
|
Why change the LRM? Does the current solution provide any safety benefit? If no, then does the current solution make these type of situations easier or harder for a programmer? This sort of hiding is inconsistent with the way ambiguity is determined and eliminates more than required to make subprogram calls unambiguous in the context. The only hiding that is required is a call to the subprogram with all parameters specified. There is a reason to not have default values on an inner layer like this when a wrapper layer, like the one in AlertLogPkg is created. If the job of the outer layer is to fill in the parameters and then call the inner layer, if the inner layer has defaults and as you are writing the outer layer, one forgets to map the defaults, it takes some time and simulation effort to find - first hand experience. So I removed the defaults from the inner layer. |
Why change the LRM? Does the current solution provide any safety benefit? If no, then does the current solution make these type of situations easier or harder for a programmer?
If you change the LRM, that would be incompatible with the previous LRM versions. That would therefore be much less safe.
|
Currently I have tested on 2 of the 4 commercial vendors and neither take issue with the code, only GHDL. Hopefully later this year I will be able to do testing with the other two. |
Obviously any change to the LRM makes it different than the previous versions. The current metric when changing the LRM is try to not break old code - if you break old code, it better be a really important feature (which obviously this is not). |
Hi,
Using osvvm 2020.08 I get following error when using latest nighty (as for today) ghdl under archlinux. It compiles with no issue with 2020.07.
Not sure if this is an osvvm or a ghdl issue @tgingold, though.
Thanks,
The text was updated successfully, but these errors were encountered: