public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: Gabriel Charette <gchare@google.com>
To: Tom Tromey <tromey@redhat.com>
Cc: Diego Novillo <dnovillo@google.com>,
	Lawrence Crowl <crowl@google.com>,
	       gcc@gcc.gnu.org, Collin Winter <collinwinter@google.com>
Subject: Re: Linemap and pph
Date: Fri, 22 Jul 2011 22:12:00 -0000	[thread overview]
Message-ID: <CAJTZ7L+VPzfrLvnR6ZDmCd29hm2OFHo+taQfLZxDqMBgwqBg1w@mail.gmail.com> (raw)
In-Reply-To: <m3ipquvylt.fsf@fleche.redhat.com>

Thanks for the quick reply Tom,

On Fri, Jul 22, 2011 at 1:39 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Gabriel" == Gabriel Charette <gchare@google.com> writes:
>
> Gabriel> @tromey: We have a question for you: the problem is detailed
> Gabriel> here and our question to you is at the bottom. Thanks!
>
> Sure.  I have not followed PPH progress very closely and after reading
> your message I am not sure I have much to offer.  I'll give it a stab
> anyhow.
>
> Gabriel> This is a problem in the linemap case because when we read the last
> Gabriel> name, we also read it's input_location (line and column) and try to
> Gabriel> replay that location by calling linemap_line_start (so far so
> Gabriel> good....). This issue is that linemap_line_start EXPECTS to be called
> Gabriel> with lines in increasing orders (but since the bindings are backwards
> Gabriel> in the chain we actually call it with lines in a decreasing order),
> Gabriel> the logic in linemap_line_start is such that if the line# is less then
> Gabriel> the previous line# it was called with, it assumes this must be a new
> Gabriel> file and creates a new file entry in the line_table by calling
> Gabriel> linemap_add (which in our case is WRONG!!).
>
> My belief is that linemap was designed to support the typical case for a
> C and C++ compiler.  However, this is not sacrosanct; you could perhaps
> change the implementation to work better for your case.  I suspect,
> though, that this won't be easy -- and also be aware of Dodji's patch
> series, which complicates linemap.
>

We are tying to keep pph as "pluginable" as possible (Diego correct me
if I'm wrong), so changing the actual implementation of the linemap
would be our very last resort I think.

> Gabriel> In the past we have solved similar issues (caused by the backwards
> Gabriel> ordering), by replaying whatever it was in the correct order (before
> Gabriel> doing anything else), and then simply loading references using the
> Gabriel> pph_cache when the real ones were needed. BUT we can't do this with
> Gabriel> source_locations as they are a simple typedef unsigned int, not actual
> Gabriel> structs which are passed by pointer value...
>
> A source_location is a reference to a particular line_map.  It is sort
> of like a compressed pointer.
>

Right, but our cache works in a way that, say you output a refrenece
to a struct, you first remember the pointer value, then output the
struct, if you try to ouput the same reference (same pointer value)
later we simply mark that as a shared reference in the output (and
have logic to link the two on the way in).

However since source_location aren't pointers per se, this wouldn't
work (at least with our current cache implementation, and changing
that is also last resort in my opinion)

> Gabriel> @tromey: I hear you are the person in the know when it comes down to
> Gabriel> linemaps, do you have any hint on how we could rebuild a valid
> Gabriel> line_table given we are obtaining the bindings from the last one in
> Gabriel> the file to the first one (that is, using pph (pre-parsed headers)
> Gabriel> where we are trying to restore a cached version of the parser state
> Gabriel> for a header)?
>
> Can you not just serialize the line map when creating the PPH?
>

We were wondering about that, the problem we thought is that a pph can
be loaded from anywhere in many different C files (which would
generate a different linemap entry in each case I think?). If there
was a way to serialize the linemap entries from the LC_ENTER entry for
the header file to the LC_LEAVE (i.e. ignoring builtins, command line,
etc.), and then directly insert those entries in a given C file
compilation's linemaps entries, that would be great!

> Then, when using the PPH, read the serialized line map, in order, into
> the current global line map.  This will preserve the include chains and
> all the rest.  Then rewrite source locations coming from the PPH to new
> locations from the new line map.  This rewriting could perhaps even be
> done efficiently, say just by adding an offset to all locations --
> though I am not sure, this would require some research.  (It seems
> doable to me, though; perhaps, but not definitely, requiring some
> additional linemap API.)
>

I will investigate in that direction.

> I have no idea if this is practical for you.  I guess it depends on how
> a PPH is read in.
>

A pph is read in by: reading the preprocessor tokens and replaying
them (i.e. redefining them as if they had been pasted in the C file at
the very location of the include triggering the pph). We then load the
bindings attached to the global_namespace defined in the header and
all bindings are recursively loaded in that fashion, the only special
fact to be aware of regarding the linemap issue is that those bindings
(by the very nature of the parser logic happening before we cache the
parser's state into the pph), get to us in the reverse order they were
originally seen in the header file (thus creating this linemap
problem).

Thank you very much,
Gabriel

  reply	other threads:[~2011-07-22 21:15 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-22 20:40 Gabriel Charette
2011-07-22 21:15 ` Tom Tromey
2011-07-22 22:12   ` Gabriel Charette [this message]
2011-07-23  5:46     ` Tom Tromey
2011-07-23 17:54     ` Dodji Seketeli
2011-07-25 19:34       ` Gabriel Charette
2011-07-25 22:55         ` Gabriel Charette
2011-07-26 14:46           ` Dodji Seketeli
2011-07-26 18:49             ` Gabriel Charette
2011-07-26 22:54               ` Gabriel Charette
2011-07-27 10:11                 ` Dodji Seketeli
2011-07-27 14:17                   ` Gabriel Charette
2011-07-27 14:26                     ` Dodji Seketeli
2011-07-27 14:51                       ` Gabriel Charette
2011-07-27 15:45                         ` Dodji Seketeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJTZ7L+VPzfrLvnR6ZDmCd29hm2OFHo+taQfLZxDqMBgwqBg1w@mail.gmail.com \
    --to=gchare@google.com \
    --cc=collinwinter@google.com \
    --cc=crowl@google.com \
    --cc=dnovillo@google.com \
    --cc=gcc@gcc.gnu.org \
    --cc=tromey@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).