Hello, David Malcolm <dmalc...@redhat.com> a écrit:
> This is an updated version of: > https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00736.html > > Changes in V2 of the patch: > * c_lex_with_flags: don't write through the range ptr if it's NULL > * don't add any fields to the C++ frontend's cp_token for now > * libcpp/lex.c: prevent usage of stale/uninitialized data in > _cpp_temp_token and _cpp_lex_direct. > > This patch adds source *range* information to libcpp's cpp_token, and to > c_token in the C frontend. > > As noted before, to minimize churn, I kept the existing location_t > fields, though in theory these are always just equal to the start of > the source range. Funny; I first overlooked this comment of you, and then when reading the patch, I asked myself "why keep the existing location_t" ? I mean, in here: /* A preprocessing token. This has been carefully packed and should - occupy 16 bytes on 32-bit hosts and 24 bytes on 64-bit hosts. */ + occupy 16 bytes on 32-bit hosts and 24 bytes on 64-bit hosts. + FIXME: the above comment is no longer true with this patch. */ struct GTY(()) cpp_token { source_location src_loc; /* Location of first char of token. */ + source_range src_range; /* Source range covered by the token. */ ENUM_BITFIELD(cpp_ttype) type : CHAR_BIT; /* token type */ unsigned short flags; /* flags - see above */ You could just change the type of src_loc and make it be a source_range. Source range could take a converting constructor, that takes a source_location, so that the existing code that does "cpp_token.src_loc = a_source_location;" keeps working. But then, in the previous patch of the series, I see: +/* A range of source locations. + + Ranges are closed: + m_start is the first location within the range, + m_finish is the last location within the range. + + We may need a more compact way to store these, but for now, + let's do it the simple way, as a pair. */ +struct GTY(()) source_range +{ + source_location m_start; + source_location m_finish; + + void debug (const char *msg) const; + + /* We avoid using constructors, since various structs that + don't yet have constructors will embed instances of + source_range. */ + But what if we define a default constructor for that one (as well as one that takes a source_location)? Wouldn't that work for the embedding case that you talk about in that comment? The reason why I am asking all this is, memory consumption. Maybe you've measured it and it's not relevant, but otherwise, if we could do away with duplicating the start location and still miminize the churn, maybe we should try. -- Dodji
