kadircet wrote: > I think we need to understand what we want before we can make decisions on > what needs changing, though. Are there invariants we want to introduce, like > the source range for the AST node should encompass the source locations > tracked within the AST node? Or are we fine with the AST node tracking source > locations which exist outside of the source range for the node itself? How do > we want users of the AST to understand what the source range represents?
I think semantically connecting all the requirements of source tools to AST nodes is difficult. > I am currently leaning towards the idea that the source range for an AST node > should be the union of the ranges tracked by the node and its children. I think this makes sense and sounds principled on paper, but achieving that isn't feasible without breaking users every time we take a step towards it. > Someday, if we start tracking source locations for the storage class > specifiers, the range may expand to include those. I think that would be a breaking change for downstream consumers once again. People will rely on those changes not being part of the outer decls, and when we expand them, we'll regress those (e.g. tools that just rewrite types, will start dropping storage specifiers all of a sudden). I really don't know the answer here, and the more I think the harder it feels. Even if we didn't have this "incremental issues result in regressions for the users" issue, as you've also pointed out, I don't think modelling AST nodes as "properly nested" works for C++ :/ Ignoring macros, pragmas, comments; declarator syntax itself means type and name of a declaration might be nested, instead of being siblings. So I'd probably just keep the source range definition for composite definitions as-is today, claim them deprecated and just provide "self" locations for ast nodes, possibly making it multiple locations to model any discontinuities. e.g. a `VarDecl` would only point to its `name`. you'd need to drill down into specific parts of it if your tool is interested in the rest. This sounds like a much simpler contract, and can be incrementally implemented as well. Moreover it will be stable, no matter what changes we make to AST, as long as an AST node is there, source location associated with it will stay there. https://github.com/llvm/llvm-project/pull/133107 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
