Re: [PATCH 12/27] New file: gcc/jit/jit-recording.h

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2014-11-03 at 14:27 -0700, Jeff Law wrote:
> On 10/31/14 11:02, David Malcolm wrote:
> > This file declares the gcc::jit::recording internal API, so that
> > libgccjit.c can record the calls that are made to the public API, for
> > later playback by the dummy frontend.
> >
> > gcc/jit/
> > 	* jit-recording.h: New.
> > ---
> >   gcc/jit/jit-recording.h | 1593 +++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 1593 insertions(+)
> >   create mode 100644 gcc/jit/jit-recording.h
> >
> > diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
> > new file mode 100644
> > index 0000000..bb1a2ee
> > --- /dev/null
> > +++ b/gcc/jit/jit-recording.h
> [ ... ]
> 
> > +
> > +private:
> > +  void validate ();
> So give the complexities in interfacing with the guts of GCC, would it 
> make sense to expose the validate method?

Most of the error-checking in the API happens in the API calls in
libgccjit.c, testing that the individual pieces are sane.

The validate method tests the things that can only be verified "as a
whole", when the context is about to be compiled: are there unreachable
blocks?  is every block terminated?  etc

I can't quite see why client code might want to perform the latter kind
of validation without actually doing a compile, so I don't plan to
expose this at this time.  It's trivial to do so if someone needs it.

> > +/* or just use std::string? */
> > +class string : public memento
> Is there some reason not to use std::string?  I really like using 
> standard components rather than rolling our own.

I think I was trying to avoid std::string for some reason, but I'm not
quite sure why, perhaps out of a misremembered idea that libstdc++ was
verboten (I currently use std:: in one place, in jit-playback.h, where a
playback::context has a:
  vec<std::pair<tree, location *> > m_cached_locations;
).

In any case recording::string is an implementation detail hidden within
the library.  It is a recording::memento and hence has the same lifetime
as the recording::context.  I can't think of a reason off the top of my
head why a std::string wouldn't work instead, but the existing code
works, and has been through a fair amount of debugging.

One thing I do make use of is that it's a pointer, for this field within
recording::memento:

  string *m_debug_string;

so that I can use NULL for the common case of "no debug string has been
built for this thing yet" - though I suspect I could use a std::string *
for that.

> OK for the trunk.

Thanks.