Thanks for your feedback. > I also think this is to be preferred. I think of location_t as being, for > most of the compiler, an opaque handle to location information. Just as > it can now represent information about different concepts of location in > the presence of macro expansion, so it's entirely reasonable for a > location_t value to represent information about a range of locations > (start and end points) for an expression, or to represent information > about the locations (or ranges of locations) of the operands and operators > of an expression (in the absence of an unfolded AST, where for locations > of operands themselves you could just look one level down in the AST).
OK, I agree that's an interesting/promising approach (using/reserving a bit to add a level of indirection into a separate hash table), I'll investigate what it could take to implement such approach, which would indeed get rid of the duplicate_expr_locations() calls, since this would be implicit as part of protected_set_expr_location() somehow, right? > I would guess that much of the patch would be unchanged with such an > approach - you still need to pass extra location information to various > places, but the details of how you attach it to the expressions might be > different. Right, hopefully the main difference would be the implementation of the new functions in tree.[ch] (possibly moved elsewhere?) to set/retrieve these extra slocs, most of the changes would remain almost identical I suspect. > I would say that a location_t mapping to a set of other locations more > complicated than at present should have some structure to how it maps to > them. That is, rather than just mapping to an array of values with those > values used in different ways for different source code constructs, it > should be possible to tell that a given location_t is mapping to certain > locations as corresponding to first and second operands of an binary > operator (for example). OK, so you mean, instead of knowing the number of locations from the tree kind (e.g. 1 extra sloc for unary exprs, 2 for binary exprs, ...), we would encode this as part of the extra loc info? Note that the number of extra locations is already stored in my current patch, this is the unsigned char len field of sloc_Struct in tree.c. Is that sufficient (knowing the number of slocs), or do you have something more advanced in mind? Note that the structure stored in the hash table is: typedef struct { const_tree node; unsigned char len; location_t locus[1]; } sloc_struct; In other words, we have all the info you mentioned: number of extra slocs (len field) and tree kind (via the node field). If this is not what you had in mind, could you clarify what else? Arno