| Message ID | 20110816003729.5C11A1C0F8B@gchare.mtv.corp.google.com |
|---|---|
| State | New |
| Headers | show |
Hello Gabriel, gchare@google.com (Gabriel Charette) a écrit: > Here is the updated patch. > > It nows exposes two libcpp functions to force the source_location for tokens when desired. > > The lexer then checks for a value set by these functions in cpp_reader and acts accordingly when needing a location for a new token (either using the forced_location or calling the linemap as it used to). > > It turns out the fortran library made the same mistake of creating a line_table entry for builtins, I fixed it as well in this patch. > > Tested on x64 for c++,fortran. > > (fyi: I moved the removal of LINEMAP_POSITION_FOR_COLUMN to a separate patch which is checked-in already; thus it doesn't show up in this updated patch obviously. > ) > > Ok for trunk? > > Gabriel > > 2011-08-15 Gabriel Charette <gchare@google.com> > > gcc/c-family/ChangeLog > * c-opts.c (c_finish_options): Force BUILTINS_LOCATION for tokens > defined in cpp_init_builtins and c_cpp_builtins. > > gcc/fortran/ChangeLog > * cpp.c (gfc_cpp_init): Force BUILTINS_LOCATION for tokens > defined in cpp_define_builtins. > > libcpp/ChangeLog > * init.c (cpp_create_reader): Inititalize forced_token_location_p. > * internal.h (struct cpp_reader): Add field forced_token_location_p. > * lex.c (_cpp_lex_direct): Use forced_token_location_p. > (cpp_force_token_locations): New. > (cpp_stop_forcing_token_locations): New. I cannot approve or reject this patch, but FWIW, it looks OK to me. Thanks.
Tom: ok for trunk? fortran@: The fortran change just reflects the fix from libcpp, fortran bootstrap and tests passed. Thanks, Gabriel On Wed, Aug 17, 2011 at 1:04 PM, Dodji Seketeli <dodji@seketeli.org> wrote: > Hello Gabriel, > > gchare@google.com (Gabriel Charette) a écrit: > >> Here is the updated patch. >> >> It nows exposes two libcpp functions to force the source_location for tokens when desired. >> >> The lexer then checks for a value set by these functions in cpp_reader and acts accordingly when needing a location for a new token (either using the forced_location or calling the linemap as it used to). >> >> It turns out the fortran library made the same mistake of creating a line_table entry for builtins, I fixed it as well in this patch. >> >> Tested on x64 for c++,fortran. >> >> (fyi: I moved the removal of LINEMAP_POSITION_FOR_COLUMN to a separate patch which is checked-in already; thus it doesn't show up in this updated patch obviously. >> ) >> >> Ok for trunk? >> >> Gabriel >> >> 2011-08-15 Gabriel Charette <gchare@google.com> >> >> gcc/c-family/ChangeLog >> * c-opts.c (c_finish_options): Force BUILTINS_LOCATION for tokens >> defined in cpp_init_builtins and c_cpp_builtins. >> >> gcc/fortran/ChangeLog >> * cpp.c (gfc_cpp_init): Force BUILTINS_LOCATION for tokens >> defined in cpp_define_builtins. >> >> libcpp/ChangeLog >> * init.c (cpp_create_reader): Inititalize forced_token_location_p. >> * internal.h (struct cpp_reader): Add field forced_token_location_p. >> * lex.c (_cpp_lex_direct): Use forced_token_location_p. >> (cpp_force_token_locations): New. >> (cpp_stop_forcing_token_locations): New. > > I cannot approve or reject this patch, but FWIW, it looks OK to me. > > Thanks. > > -- > Dodji >
On 08/18/2011 06:47 PM, Gabriel Charette wrote: > Tom: ok for trunk? > > fortran@: The fortran change just reflects the fix from libcpp, > fortran bootstrap and tests passed. The Fortran change is OK. Thanks for the patch! Tobias On 08/16/2011 02:37 AM, Gabriel Charette wrote: > Here is the updated patch. > > It nows exposes two libcpp functions to force the source_location for tokens when desired. > > The lexer then checks for a value set by these functions in cpp_reader and acts accordingly when needing a location for a new token (either using the forced_location or calling the linemap as it used to). > > It turns out the fortran library made the same mistake of creating a line_table entry for builtins, I fixed it as well in this patch. > > Tested on x64 for c++,fortran. > Ok for trunk? > > Gabriel > > 2011-08-15 Gabriel Charette<gchare@google.com> > > gcc/c-family/ChangeLog > * c-opts.c (c_finish_options): Force BUILTINS_LOCATION for tokens > defined in cpp_init_builtins and c_cpp_builtins. > > gcc/fortran/ChangeLog > * cpp.c (gfc_cpp_init): Force BUILTINS_LOCATION for tokens > defined in cpp_define_builtins. > > libcpp/ChangeLog > * init.c (cpp_create_reader): Inititalize forced_token_location_p. > * internal.h (struct cpp_reader): Add field forced_token_location_p. > * lex.c (_cpp_lex_direct): Use forced_token_location_p. > (cpp_force_token_locations): New. > (cpp_stop_forcing_token_locations): New. > > diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c > index 3227f7b..49ff80d 100644 > --- a/gcc/c-family/c-opts.c > +++ b/gcc/c-family/c-opts.c > @@ -1306,12 +1306,17 @@ c_finish_options (void) > { > size_t i; > > - cb_file_change (parse_in, > - linemap_add (line_table, LC_RENAME, 0, > - _("<built-in>"), 0)); > + { > + /* Make sure all of the builtins about to be declared have > + BUILTINS_LOCATION has their source_location. */ > + source_location builtins_loc = BUILTINS_LOCATION; > + cpp_force_token_locations (parse_in,&builtins_loc); > > - cpp_init_builtins (parse_in, flag_hosted); > - c_cpp_builtins (parse_in); > + cpp_init_builtins (parse_in, flag_hosted); > + c_cpp_builtins (parse_in); > + > + cpp_stop_forcing_token_locations (parse_in); > + } > > /* We're about to send user input to cpplib, so make it warn for > things that we previously (when we sent it internal definitions) > diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c > index a40442e..9368d89 100644 > --- a/gcc/fortran/cpp.c > +++ b/gcc/fortran/cpp.c > @@ -565,9 +565,17 @@ gfc_cpp_init (void) > if (gfc_option.flag_preprocessed) > return; > > - cpp_change_file (cpp_in, LC_RENAME, _("<built-in>")); > if (!gfc_cpp_option.no_predefined) > - cpp_define_builtins (cpp_in); > + { > + /* Make sure all of the builtins about to be declared have > + BUILTINS_LOCATION has their source_location. */ > + source_location builtins_loc = BUILTINS_LOCATION; > + cpp_force_token_locations (cpp_in,&builtins_loc); > + > + cpp_define_builtins (cpp_in); > + > + cpp_stop_forcing_token_locations (cpp_in); > + } > > /* Handle deferred options from command-line. */ > cpp_change_file (cpp_in, LC_RENAME, _("<command-line>")); [...]
>>>>> "Gabriel" == Gabriel Charette <gchare@google.com> writes:
Gabriel> It nows exposes two libcpp functions to force the
Gabriel> source_location for tokens when desired.
I am not really a fan of this approach, but I see why you did it this
way -- anything else would be very invasive.
I can only approve the libcpp parts.
Gabriel> +void cpp_force_token_locations (cpp_reader *r, source_location *p)
Newline after "void".
Gabriel> +void cpp_stop_forcing_token_locations (cpp_reader *r)
Likewise.
The libcpp parts are ok with those changes.
Tom
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c index 3227f7b..49ff80d 100644 --- a/gcc/c-family/c-opts.c +++ b/gcc/c-family/c-opts.c @@ -1306,12 +1306,17 @@ c_finish_options (void) { size_t i; - cb_file_change (parse_in, - linemap_add (line_table, LC_RENAME, 0, - _("<built-in>"), 0)); + { + /* Make sure all of the builtins about to be declared have + BUILTINS_LOCATION has their source_location. */ + source_location builtins_loc = BUILTINS_LOCATION; + cpp_force_token_locations (parse_in, &builtins_loc); - cpp_init_builtins (parse_in, flag_hosted); - c_cpp_builtins (parse_in); + cpp_init_builtins (parse_in, flag_hosted); + c_cpp_builtins (parse_in); + + cpp_stop_forcing_token_locations (parse_in); + } /* We're about to send user input to cpplib, so make it warn for things that we previously (when we sent it internal definitions) diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c index a40442e..9368d89 100644 --- a/gcc/fortran/cpp.c +++ b/gcc/fortran/cpp.c @@ -565,9 +565,17 @@ gfc_cpp_init (void) if (gfc_option.flag_preprocessed) return; - cpp_change_file (cpp_in, LC_RENAME, _("<built-in>")); if (!gfc_cpp_option.no_predefined) - cpp_define_builtins (cpp_in); + { + /* Make sure all of the builtins about to be declared have + BUILTINS_LOCATION has their source_location. */ + source_location builtins_loc = BUILTINS_LOCATION; + cpp_force_token_locations (cpp_in, &builtins_loc); + + cpp_define_builtins (cpp_in); + + cpp_stop_forcing_token_locations (cpp_in); + } /* Handle deferred options from command-line. */ cpp_change_file (cpp_in, LC_RENAME, _("<command-line>")); diff --git a/libcpp/include/cpplib.h b/libcpp/include/cpplib.h index 55b0f1b..6ad0345 100644 --- a/libcpp/include/cpplib.h +++ b/libcpp/include/cpplib.h @@ -985,4 +985,8 @@ extern void cpp_prepare_state (cpp_reader *, struct save_macro_data **); extern int cpp_read_state (cpp_reader *, const char *, FILE *, struct save_macro_data *); +/* In lex.c */ +extern void cpp_force_token_locations (cpp_reader *, source_location *); +extern void cpp_stop_forcing_token_locations (cpp_reader *); + #endif /* ! LIBCPP_CPPLIB_H */ diff --git a/libcpp/init.c b/libcpp/init.c index 5ba6666..b3d4c8c 100644 --- a/libcpp/init.c +++ b/libcpp/init.c @@ -221,6 +221,9 @@ cpp_create_reader (enum c_lang lang, hash_table *table, /* Initialize table for push_macro/pop_macro. */ pfile->pushed_macros = 0; + /* Do not force token locations by default. */ + pfile->forced_token_location_p = NULL; + /* The expression parser stack. */ _cpp_expand_op_stack (pfile); diff --git a/libcpp/internal.h b/libcpp/internal.h index d2872c4..6c423f0 100644 --- a/libcpp/internal.h +++ b/libcpp/internal.h @@ -499,6 +499,10 @@ struct cpp_reader /* List of saved macros by push_macro. */ struct def_pragma_macro *pushed_macros; + + /* If non-null, the lexer will use this location for the next token + instead of getting a location from the linemap. */ + source_location *forced_token_location_p; }; /* Character classes. Based on the more primitive macros in safe-ctype.h. diff --git a/libcpp/lex.c b/libcpp/lex.c index d460b98..244b14d 100644 --- a/libcpp/lex.c +++ b/libcpp/lex.c @@ -1975,8 +1975,11 @@ _cpp_lex_direct (cpp_reader *pfile) } c = *buffer->cur++; - result->src_loc = linemap_position_for_column (pfile->line_table, - CPP_BUF_COLUMN (buffer, buffer->cur)); + if (pfile->forced_token_location_p) + result->src_loc = *pfile->forced_token_location_p; + else + result->src_loc = linemap_position_for_column (pfile->line_table, + CPP_BUF_COLUMN (buffer, buffer->cur)); switch (c) { @@ -2837,3 +2840,19 @@ cpp_token_val_index (cpp_token *tok) return CPP_TOKEN_FLD_NONE; } } + +/* All tokens lexed in R after calling this function will be forced to have + their source_location the same as the location referenced by P, until + cpp_stop_forcing_token_locations is called for R. */ + +void cpp_force_token_locations (cpp_reader *r, source_location *p) +{ + r->forced_token_location_p = p; +} + +/* Go back to assigning locations naturally for lexed tokens. */ + +void cpp_stop_forcing_token_locations (cpp_reader *r) +{ + r->forced_token_location_p = NULL; +}