From patchwork Wed Aug 10 18:22:53 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gab Charette X-Patchwork-Id: 109422 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id DFA67B7090 for ; Thu, 11 Aug 2011 04:23:16 +1000 (EST) Received: (qmail 7382 invoked by alias); 10 Aug 2011 18:23:14 -0000 Received: (qmail 7371 invoked by uid 22791); 10 Aug 2011 18:23:12 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RP_MATCHES_RCVD, SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.67) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 10 Aug 2011 18:22:57 +0000 Received: from hpaq7.eem.corp.google.com (hpaq7.eem.corp.google.com [172.25.149.7]) by smtp-out.google.com with ESMTP id p7AIMtDA004067; Wed, 10 Aug 2011 11:22:55 -0700 Received: from gchare.mtv.corp.google.com (gchare.mtv.corp.google.com [172.18.111.122]) by hpaq7.eem.corp.google.com with ESMTP id p7AIMrxC006394; Wed, 10 Aug 2011 11:22:53 -0700 Received: by gchare.mtv.corp.google.com (Postfix, from userid 138564) id 285201C0F8B; Wed, 10 Aug 2011 11:22:53 -0700 (PDT) To: reply@codereview.appspotmail.com, crowl@google.com, dnovillo@google.com, tromey@redhat.com, dodji@seketeli.org, gcc-patches@gcc.gnu.org Subject: Linemap force location and remove LINEMAP_POSITION_FOR_COLUMN (issue4801090) Message-Id: <20110810182253.285201C0F8B@gchare.mtv.corp.google.com> Date: Wed, 10 Aug 2011 11:22:53 -0700 (PDT) From: gchare@google.com (Gabriel Charette) X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org There was a bug where c_finish_options would create some builtins and assign them source_locations in the linemap other than BUILTINS_LOCATION == 1. Thus, when calling DECL_IS_BUILTIN to know if a decl is a builtin, some of them would return false as they had a source_location other than BUILTINS_LOCATION within the line_map entry that was incorrectly created in c_finish_options. The problem to fix this is that _cpp_lex_direct just gets the location of the currently lexed token by calling linemap_position_for_column. It has no notion of whether this token is a builtin or whatever else it may be. My solution is to add a forced_location field to the line_table which when set is returned by default by linemap_position_for_column instead of getting a new loc from the line_table. Furthermore, this mechanism will allow us to inject locations for a similar problem we have in pph when lexing replayed pre-processor definitions (which as it is now are getting assigned new source_locations which is creating problems for us). I'm open to other solutions if anyone sees another way to do it. We could also leave linemap_position_for_column as is and have a separate inline function, say linemap_pos_for_col_or_forced, which makes the forced_location check and simply calls linemap_position_for_column when no forced_location. I also removed LINEMAP_POSITION_FOR_COLUMN, it did the EXACT same thing as linemap_position_for_column, so maintaining both in parallel seems like overkill to me. The only thing I can think of is that it's more optimal as it's inlined (but if that's really needed we can always make linemap_position_for_column an inline function). Gabriel 2011-08-10 Gabriel Charette * c-opts.c (c_finish_options): Don't create built-in line_table entry; instead force BUILTINS_LOCATION when creating builtins. * include/line-map.h (struct line_maps): Add field forced_location. (LINEMAP_POSITION_FOR_COLUMN): Remove. * line-map.c (linemap_init): Init forced_location to 0. (linemap_position_for_column): Return forced_location by default if set --- This patch is available for review at http://codereview.appspot.com/4801090 diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c index 3227f7b..1af8e7b 100644 --- a/gcc/c-family/c-opts.c +++ b/gcc/c-family/c-opts.c @@ -1306,13 +1306,15 @@ c_finish_options (void) { size_t i; - cb_file_change (parse_in, - linemap_add (line_table, LC_RENAME, 0, - _(""), 0)); + /* Make sure all of the builtins about to be declared have + BUILTINS_LOCATION has their source_location. */ + line_table->forced_location = BUILTINS_LOCATION; cpp_init_builtins (parse_in, flag_hosted); c_cpp_builtins (parse_in); + line_table->forced_location = 0; + /* We're about to send user input to cpplib, so make it warn for things that we previously (when we sent it internal definitions) told it to not warn. diff --git a/gcc/go/gofrontend/lex.cc b/gcc/go/gofrontend/lex.cc index 9f26911..167c7dd 100644 --- a/gcc/go/gofrontend/lex.cc +++ b/gcc/go/gofrontend/lex.cc @@ -518,9 +518,7 @@ Lex::require_line() source_location Lex::location() const { - source_location location; - LINEMAP_POSITION_FOR_COLUMN(location, line_table, this->lineoff_ + 1); - return location; + return linemap_position_for_column (line_table, this->lineoff_ + 1); } // Get a location slightly before the current one. This is used for @@ -529,9 +527,7 @@ Lex::location() const source_location Lex::earlier_location(int chars) const { - source_location location; - LINEMAP_POSITION_FOR_COLUMN(location, line_table, this->lineoff_ + 1 - chars); - return location; + return linemap_position_for_column (line_table, this->lineoff_ + 1 - chars); } // Get the next token. diff --git a/libcpp/directives-only.c b/libcpp/directives-only.c index e19f806..c6772af 100644 --- a/libcpp/directives-only.c +++ b/libcpp/directives-only.c @@ -142,7 +142,7 @@ _cpp_preprocess_dir_only (cpp_reader *pfile, flags |= DO_LINE_COMMENT; else if (!(flags & DO_SPECIAL)) /* Mark the position for possible error reporting. */ - LINEMAP_POSITION_FOR_COLUMN (loc, pfile->line_table, col); + loc = linemap_position_for_column (pfile->line_table, col); break; diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h index f1d5bee..d14528e 100644 --- a/libcpp/include/line-map.h +++ b/libcpp/include/line-map.h @@ -95,6 +95,11 @@ struct GTY(()) line_maps { /* If non-null, the allocator to use when resizing 'maps'. If null, xrealloc is used. */ line_map_realloc reallocator; + + /* If non-zero, linemap_position_for_column automatically returns + the value stored at this memory location, instead of caclulating + a new source_location. */ + source_location forced_location; }; /* Initialize a line map set. */ @@ -165,23 +170,6 @@ extern const struct line_map *linemap_lookup /* Nonzero if the map is at the bottom of the include stack. */ #define MAIN_FILE_P(MAP) ((MAP)->included_from < 0) -/* Set LOC to a source position that is the same line as the most recent - linemap_line_start, but with the specified TO_COLUMN column number. */ - -#define LINEMAP_POSITION_FOR_COLUMN(LOC, SET, TO_COLUMN) do { \ - unsigned int to_column = (TO_COLUMN); \ - struct line_maps *set = (SET); \ - if (__builtin_expect (to_column >= set->max_column_hint, 0)) \ - (LOC) = linemap_position_for_column (set, to_column); \ - else { \ - source_location r = set->highest_line; \ - r = r + to_column; \ - if (r >= set->highest_location) \ - set->highest_location = r; \ - (LOC) = r; \ - }} while (0) - - extern source_location linemap_position_for_column (struct line_maps *set, unsigned int to_column); diff --git a/libcpp/lex.c b/libcpp/lex.c index d29f36d..d460b98 100644 --- a/libcpp/lex.c +++ b/libcpp/lex.c @@ -1975,8 +1975,8 @@ _cpp_lex_direct (cpp_reader *pfile) } c = *buffer->cur++; - LINEMAP_POSITION_FOR_COLUMN (result->src_loc, pfile->line_table, - CPP_BUF_COLUMN (buffer, buffer->cur)); + result->src_loc = linemap_position_for_column (pfile->line_table, + CPP_BUF_COLUMN (buffer, buffer->cur)); switch (c) { diff --git a/libcpp/line-map.c b/libcpp/line-map.c index dd3f11c..31ab672 100644 --- a/libcpp/line-map.c +++ b/libcpp/line-map.c @@ -41,6 +41,7 @@ linemap_init (struct line_maps *set) set->highest_line = RESERVED_LOCATION_COUNT - 1; set->max_column_hint = 0; set->reallocator = 0; + set->forced_location = 0; } /* Check for and warn about line_maps entered but not exited. */ @@ -245,6 +246,9 @@ linemap_line_start (struct line_maps *set, linenum_type to_line, source_location linemap_position_for_column (struct line_maps *set, unsigned int to_column) { + if (set->forced_location) + return set->forced_location; + source_location r = set->highest_line; if (to_column >= set->max_column_hint) {