> Sorry for picking on simple stuff, but the switch name seems
No problem, and thanks for your feedback.
> meaningless, and there isn't any documentation.
Ah. I'm open for suggestion on a better name, or I can come up with a
new one.
I'll indeed add documentation as soon as there's some kind of agreement on
the approach.
> Conceptually it looks like you are trying to make up for the absence
> of a proper AST by building an on-the-side hash table to track
> expression locations.
Right, that's the idea.
> The hash table key is the tree structure
> itself.
Yes.
> The thing is, any call into fold-const may give you an
> entirely new tree,
Exactly.
> and at that point you have lost your extra location
> information.
Actually no, see the c-family/c-common.c patch, copied here, which
ensures that folding does preserve such information:
* c-common.c (c_fully_fold_internal): Copy extra locations on new node.
--- c-family/c-common.c (revision 190939)
+++ c-family/c-common.c (working copy)
@@ -1440,7 +1440,10 @@ c_fully_fold_internal (tree expr, bool i
TREE_NO_WARNING (ret) = 1;
}
if (ret != expr)
- protected_set_expr_location (ret, loc);
+ {
+ protected_set_expr_location (ret, loc);
+ duplicate_expr_locations (ret, expr);
+ }
return ret;
}
> And the C/C++ frontends call into fold-const regularly,
> which is why we don't have a proper AST in the first place. So it
> seems to me that this is going to be kind of frustrating, in that we
> will often have the extra location information but sometimes we won't.
That's not the case as per the c-common.c patch, the locations are preserved
across fold, otherwise as you said, the whole approach would be pretty useless.
I should perhaps have mentioned that this patch (and the -fdump-xref
implementation on top of it) has been in production in our (AdaCore) tree for
more than 2 years now, with pretty good results, and certainly most expressions
trees do have extra sloc info available in our experience.
> And whether we have it or not will change as the frontends change.
See above.
Does this address your concern?
Arno
