On Thu, Aug 11, 2011 at 10:14 AM, Richard Guenther <richard.guent...@gmail.com> wrote: > On Thu, Aug 11, 2011 at 6:54 PM, Gabriel Charette <gch...@google.com> wrote: >> On Thu, Aug 11, 2011 at 12:27 AM, Richard Guenther >> <richard.guent...@gmail.com> wrote: >>> On Wed, Aug 10, 2011 at 8:22 PM, Gabriel Charette <gch...@google.com> wrote: >>>> 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. >>> >>> DECL_IS_BUILTIN is almost never the appropriate thing to use, instead >>> you should use DECL_BUILT_IN (and grepping, I see some suspicious uses >>> ...). >> >> Why don't all builtins have BUILTINS_LOCATION as their location? It >> doesn't make sense to me that we need to create a line_table entry for >> builtins as they don't have line/col information. > > Because we want the source location of the actual declaration if there was > any, > like when including math.h. >
Ah ok in this case I could see why a builtin would have a source_location other than BUILTINS_LOCATION, but I don't think that justifies having a <built-in> entry in the line_table (which was only used by the builtins created in cpp_init_builtins which is what I'm patching as I think those builtins should also have BUILTINS_LOCATION). The builtins created in math.h I would assume are associated with a source_location in math.h, and that's OK I think (although this might be harder for us to handle in pph, I don't think we want to change this behavior in trunk, Diego?) In pph we probably still want to serialize those builtins that are instantiated from includes and not by default anyways. The only builtins that are constant across every compilation from my understanding are those which have BUILTINS_LOCATION has their source_location (and the ones instantiated by cpp_init_builtins should be part of this set I think). Gabriel
