Hello,
> 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.
I agree with that thinking, but...
> 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!)
Yes, exactly. I want those crashes, so that we could fix then properly.
> > > 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.
I think this is not worthwhile, because the vests tests won't run
properly.
Note that I have considered the use of get_entities only within the
scope of the VESTS test suite. Maybe you'd like a wider use.
> > 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.
As far as I know, it doesn't happen in VESTS.
> 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.
No problem. You change prevents ghdl from crashing, which is a good
thing.
> > 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.
So the initial message wasn't clear enough. I will investigate that.
Tristan.
_______________________________________________
Ghdl-discuss mailing list
[email protected]
https://mail.gna.org/listinfo/ghdl-discuss
