| Message ID | 20110412202845.164371DA1BE@topo.tor.corp.google.com |
|---|---|
| State | New |
| Headers | show |
This change is not source compatible with existing code using the callbacks (which may not be in the gcc svn). Perhaps a new callback is needed? On 4/12/11, Diego Novillo <dnovillo@google.com> wrote: > > During pph processing, when we find an included file that we are going > to instantiate from an image, we don't want libcpp to stack and read > it. > > I've implemented this by allowing the 'include' callback to return a > boolean value. If it returns true, then we call _cpp_stack_include. > Otherwise, the file is ignored. > > Tom, I believe this is the right approach, but I'm not sure. It does > what I want, though. > > > Thanks. Diego. > > libcpp/ChangeLog.pph > 2011-04-12 Diego Novillo <dnovillo@google.com> > > * directives.c (do_include_common): If the callback > pfile->cb.include returns falose do not call > _cpp_stack_include. > * include/cpplib.h (struct cpp_callbacks): Change return > type of field 'include' to bool. > > gcc/cp/ChangeLog.pph > 2011-04-12 Diego Novillo <dnovillo@google.com> > > * pph.c (pth_include_handler): Return true. > (pph_include_handler): If the header file exists as a PPH > image, return false. > > diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c > index 74d1d50..6584e72 100644 > --- a/gcc/cp/pph.c > +++ b/gcc/cp/pph.c > @@ -1984,9 +2008,10 @@ pph_read_file (const char *filename) > error ("Cannot open PPH file for reading: %s: %m", filename); > } > > + > /* Record a #include or #include_next for PTH. */ > > -static void > +static bool > pth_include_handler (cpp_reader *reader ATTRIBUTE_UNUSED, > location_t loc ATTRIBUTE_UNUSED, > const unsigned char *dname, > @@ -2012,11 +2037,14 @@ pth_include_handler (cpp_reader *reader > ATTRIBUTE_UNUSED, > > state->new_angle_brackets = angle_brackets; > state->new_iname = name; > + > + return true; > } > > + > /* Record a #include or #include_next for PPH. */ > > -static void > +static bool > pph_include_handler (cpp_reader *reader, > location_t loc ATTRIBUTE_UNUSED, > const unsigned char *dname, > @@ -2025,6 +2053,7 @@ pph_include_handler (cpp_reader *reader, > const cpp_token **tok_p ATTRIBUTE_UNUSED) > { > const char *pph_file; > + bool read_text_file_p; > > if (flag_pph_debug >= 1) > { > @@ -2034,9 +2063,15 @@ pph_include_handler (cpp_reader *reader, > fprintf (pph_logfile, "%c\n", angle_brackets ? '>' : '"'); > } > > + read_text_file_p = true; > pph_file = query_pph_include_map (name); > if (pph_file != NULL && !cpp_included_before (reader, name, > input_location)) > - pph_read_file (pph_file); > + { > + pph_read_file (pph_file); > + read_text_file_p = false; > + } > + > + return read_text_file_p; > } > > > diff --git a/libcpp/directives.c b/libcpp/directives.c > index 7812b57..09c7686 100644 > --- a/libcpp/directives.c > +++ b/libcpp/directives.c > @@ -788,15 +788,19 @@ do_include_common (cpp_reader *pfile, enum > include_type type) > cpp_error (pfile, CPP_DL_ERROR, "#include nested too deeply"); > else > { > + bool do_stack_include; > + > /* Get out of macro context, if we are. */ > skip_rest_of_line (pfile); > > + do_stack_include = true; > if (pfile->cb.include) > - pfile->cb.include (pfile, pfile->directive_line, > - pfile->directive->name, fname, angle_brackets, > - buf); > + do_stack_include = pfile->cb.include (pfile, pfile->directive_line, > + pfile->directive->name, > + fname, angle_brackets, buf); > > - _cpp_stack_include (pfile, NULL, fname, angle_brackets, type); > + if (do_stack_include) > + _cpp_stack_include (pfile, NULL, fname, angle_brackets, type); > } > > XDELETEVEC (fname); > diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h > index 3dc4139..c9f3dfb 100644 > --- a/libcpp/include/cpplib.h > +++ b/libcpp/include/cpplib.h > @@ -480,7 +480,9 @@ struct cpp_callbacks > void (*file_change) (cpp_reader *, const struct line_map *); > > void (*dir_change) (cpp_reader *, const char *); > - void (*include) (cpp_reader *, unsigned int, const unsigned char *, > + /* Called when processing a #include directive. If the handler > + returns false, the file will not be read. */ > + bool (*include) (cpp_reader *, unsigned int, const unsigned char *, > const char *, int, const cpp_token **); > void (*define) (cpp_reader *, unsigned int, cpp_hashnode *); > void (*undef) (cpp_reader *, unsigned int, cpp_hashnode *); > > -- > This patch is available for review at http://codereview.appspot.com/4388057 >
On Tue, Apr 12, 2011 at 20:41, Lawrence Crowl <crowl@google.com> wrote: > This change is not source compatible with existing code using > the callbacks (which may not be in the gcc svn). Perhaps a new > callback is needed? Well, it only changes the return value for the callback. Existing users do not really need to be changed. I don't think we want a new callback. The callback would do exactly what cb.include does. Diego.
>>>>> "Diego" == Diego Novillo <dnovillo@google.com> writes:
Diego> During pph processing, when we find an included file that we are going
Diego> to instantiate from an image, we don't want libcpp to stack and read
Diego> it.
Diego> I've implemented this by allowing the 'include' callback to return a
Diego> boolean value. If it returns true, then we call _cpp_stack_include.
Diego> Otherwise, the file is ignored.
Diego> Tom, I believe this is the right approach, but I'm not sure. It does
Diego> what I want, though.
It seems reasonable enough to me.
Tom
>>>>> "Diego" == Diego Novillo <dnovillo@google.com> writes:
Lawrence> This change is not source compatible with existing code using
Lawrence> the callbacks (which may not be in the gcc svn). Perhaps a new
Lawrence> callback is needed?
Diego> Well, it only changes the return value for the callback. Existing
Diego> users do not really need to be changed.
Diego> I don't think we want a new callback. The callback would do exactly
Diego> what cb.include does.
My grep shows 2 places that set this: c-family/c-ppoutput.c and
fortran/cpp.c. It seems like it would be simple to just update those
two functions to account for the type change.
Tom
On Wed, Apr 13, 2011 at 12:21, Tom Tromey <tromey@redhat.com> wrote: >>>>>> "Diego" == Diego Novillo <dnovillo@google.com> writes: > > Lawrence> This change is not source compatible with existing code using > Lawrence> the callbacks (which may not be in the gcc svn). Perhaps a new > Lawrence> callback is needed? > > Diego> Well, it only changes the return value for the callback. Existing > Diego> users do not really need to be changed. > > Diego> I don't think we want a new callback. The callback would do exactly > Diego> what cb.include does. > > My grep shows 2 places that set this: c-family/c-ppoutput.c and > fortran/cpp.c. It seems like it would be simple to just update those > two functions to account for the type change. Oh, right. Here I was thinking of users outside of gcc, but of course there aren't those. I'll fix the calls in the existing front ends. Diego.
diff --git a/gcc/cp/pph.c b/gcc/cp/pph.c index 74d1d50..6584e72 100644 --- a/gcc/cp/pph.c +++ b/gcc/cp/pph.c @@ -1984,9 +2008,10 @@ pph_read_file (const char *filename) error ("Cannot open PPH file for reading: %s: %m", filename); } + /* Record a #include or #include_next for PTH. */ -static void +static bool pth_include_handler (cpp_reader *reader ATTRIBUTE_UNUSED, location_t loc ATTRIBUTE_UNUSED, const unsigned char *dname, @@ -2012,11 +2037,14 @@ pth_include_handler (cpp_reader *reader ATTRIBUTE_UNUSED, state->new_angle_brackets = angle_brackets; state->new_iname = name; + + return true; } + /* Record a #include or #include_next for PPH. */ -static void +static bool pph_include_handler (cpp_reader *reader, location_t loc ATTRIBUTE_UNUSED, const unsigned char *dname, @@ -2025,6 +2053,7 @@ pph_include_handler (cpp_reader *reader, const cpp_token **tok_p ATTRIBUTE_UNUSED) { const char *pph_file; + bool read_text_file_p; if (flag_pph_debug >= 1) { @@ -2034,9 +2063,15 @@ pph_include_handler (cpp_reader *reader, fprintf (pph_logfile, "%c\n", angle_brackets ? '>' : '"'); } + read_text_file_p = true; pph_file = query_pph_include_map (name); if (pph_file != NULL && !cpp_included_before (reader, name, input_location)) - pph_read_file (pph_file); + { + pph_read_file (pph_file); + read_text_file_p = false; + } + + return read_text_file_p; } diff --git a/libcpp/directives.c b/libcpp/directives.c index 7812b57..09c7686 100644 --- a/libcpp/directives.c +++ b/libcpp/directives.c @@ -788,15 +788,19 @@ do_include_common (cpp_reader *pfile, enum include_type type) cpp_error (pfile, CPP_DL_ERROR, "#include nested too deeply"); else { + bool do_stack_include; + /* Get out of macro context, if we are. */ skip_rest_of_line (pfile); + do_stack_include = true; if (pfile->cb.include) - pfile->cb.include (pfile, pfile->directive_line, - pfile->directive->name, fname, angle_brackets, - buf); + do_stack_include = pfile->cb.include (pfile, pfile->directive_line, + pfile->directive->name, + fname, angle_brackets, buf); - _cpp_stack_include (pfile, NULL, fname, angle_brackets, type); + if (do_stack_include) + _cpp_stack_include (pfile, NULL, fname, angle_brackets, type); } XDELETEVEC (fname); diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h index 3dc4139..c9f3dfb 100644 --- a/libcpp/include/cpplib.h +++ b/libcpp/include/cpplib.h @@ -480,7 +480,9 @@ struct cpp_callbacks void (*file_change) (cpp_reader *, const struct line_map *); void (*dir_change) (cpp_reader *, const char *); - void (*include) (cpp_reader *, unsigned int, const unsigned char *, + /* Called when processing a #include directive. If the handler + returns false, the file will not be read. */ + bool (*include) (cpp_reader *, unsigned int, const unsigned char *, const char *, int, const cpp_token **); void (*define) (cpp_reader *, unsigned int, cpp_hashnode *); void (*undef) (cpp_reader *, unsigned int, cpp_hashnode *);