On Sun, 2013-12-29 at 10:22 +0100, [email protected] wrote:
> Hello,
>
> > It's really great to see all of this activity!
> > Thanks Tristan!
>
> Well, thank you for reloading ghdl!
>
> > A couple of questions if you don't mind;
> > ------- from 77dde4 ---------------------------
> > sem_assocs: add missing set_base_name on formal.
> > Did you just notice this was missing (I'd never have seen it!) or is
> > it part of another commit?
>
> In fact, I'd like to rework change set 156 & 157: crashes in
> Get_Kind means there is an issue before.
Aaah, so this will bring compilation to a halt before Get_Kind(Null_Iir)
for one of those test cases. I haven't
> (I will revert the Get_Kind change, as it isn't anymore necessary
> for the reported issues.)
My thinking was, there may be more paths that return Null_Iir given
incorrect VHDL, so (if errors had already been detected) halting
compilation there was reasonable. And though it's on a critical path,
ghdl is already so fast compared with the rest of gcc that one "null"
test there seemed tolerable.
Also my limited understanding of the source means I wasn't seeing the
right place to fix each individual case. I had a fix (I wasn't really
happy with) for 20597 but removed it when fixing sr2553
If we revert that change, more cases may come along which have to be
fixed (but you probably want to fix them properly, so that may not be a
bad thing!)
> > The comment looks out of date?
> Yes.
Heh - not any more, I see...
>
> > ------- from 1f3238 and c19ce6 ---------------------------
> >
> > Adding an "ENTITY" option to various tests ...
> It is up to you. I have modified get_entities (should now be renamed ?)
> to print the last entity of the file, which is the one at the top of
> the design.
Aaah. I didn't want to exclude multiple top-level (=portless) entities
in a file (since a test suite should be torture!), the plan was to
elaborate and run them all. But if you don't think that's worthwhile,
that's OK.
> And I added ENTITY when the design must be elaborated from
> a configuration.
Also useful when top level entity and architecture are in different
files. I'll continue with it a bit longer, but it may come to nothing.
> I also plan to revert changeset 135: in fact there was a missing range
> check during sem. The range is an integer range, and the right bound
> is clearly out of range.
OK if there's a better way to do it. Again, I don't fully understand the
overall structure of the compiler so I just do the best I can.
> I also have a question about one of your change:
>
> --- a/sem_names.adb
> +++ b/sem_names.adb
> @@ -1980,8 +1980,8 @@ package body Sem_Names is
> end;
> if Res = Null_Iir then
> Error_Msg_Sem
> - ("prefix is neither a function name "
> - & "nor can it be sliced or indexed", Name);
> + ("No overloaded subprogram found matching "
> + & Disp_Node (Prefix_Name), Name);
> I wonder wether the message is an improvement or not, as the prefix may
> not be a function. But I suppose you'd like to display the prefix to
> make the message clearer ?
The original error message appeared in the code twice, and seemed to
make perfect sense in the other location (about 10 lines further on).
I encountered it when the OSVVM demo was failing (an overloaded
procedure wasn't found - in that case because of another bug) and it was
really misleading in that circumstance (the prefix is a subprogram name,
and the context is overload resolution).
But if you can see circumstances where my replacement is misleading,
then we need a new wording.
Regards,
- Brian
_______________________________________________
Ghdl-discuss mailing list
[email protected]
https://mail.gna.org/listinfo/ghdl-discuss
