perf(treesitter): bound has-ancestor? for highlighter

[tomtomjhj]

Problem:
has-ancestor? predicate can take long when the match is deep in the tree, e.g., long else if chain in C. This may slow down highlighting and thus redrawing significantly.
See #24965.

Solution:
Add option for limiting has-ancestor?'s search length, and use it for highlighter.
The limit 15 is arbitrary, but seems to be good enough:

• Existing use of has-ancestor? in highlight queries are quite local. For example in cpp, it searches for a call expression above a quantified identifier to highlight the function name.
• Scrolling is reponsive enough in the long else if chain example.

See #24965 for other approaches.

[clason]

I'm not a fan of arbitrary magic constants (we had similar PRs killed by that).

Can't we expose the number instead by, e.g., having #has-ancestor? @foo 15 (with some default which is then not as important)? Bonus points: deprecate #has-parent? @foo in favor of #has-ancestor @foo 1.

[lewis6991]

I don't think we should do this as it breaks correctness.

Ideally the grammars should be rewritten so they don't create nested structures for if-else statements.

[tomtomjhj]

Ideally the grammars should be rewritten so they don't create nested structures for if-else statements.

Filed an issue there. tree-sitter/tree-sitter-c#198.

note: other languages (e.g., rust) have similar issue.

[tomtomjhj]

Can't we expose the number instead by, e.g., having #has-ancestor? @foo 15 (with some default which is then not as important)? Bonus points: deprecate #has-parent? @foo in favor of #has-ancestor @foo 1.

#24965 (comment)
#24965 (comment)

I don't think we should do this as it breaks correctness.

I think distance limit is good enough even if it can't be totally correct since the worst thing that could happen is just some highlights going slightly off. Also, this would be very rare because the most uses of #has-ancestor don't intend to go all the way up to the root of the tree.

[clason]

I still think a global limit is not acceptable (even though I mentioned that initially; the analogy with match_limit is exactly why I no longer want that since that also triggered "but I want a different limit!" issues).

What's the argument against making this explicit in the directive?

[tomtomjhj]

I'm not against per-directive limits, and I'm happy to implement that.

[clason]

That's a different approach than the one I suggested (which would allow per-language and per-pattern configuration, and also consolidate #has-parent?). Can you explain why you went that route instead?

[tomtomjhj]

Oh, sorry for confusion. I haven't worked on it yet, and the force push was just for resolving the conflicts.

But I think we haven't reached consensus on whether there should be any kind of arbitrary limits, be it global or the per-pattern. @lewis6991 What do you think about per-pattern limits like #has-ancestor? @foo 15?

By the way, there is an ongoing work on changing the grammar itself: tree-sitter/tree-sitter-c#199

[lewis6991]

This might not be needed with tree-sitter/tree-sitter#3214 (review) which should provide even more optimization opportunities.

[theHamsta]

#has-ancestor shouldn't exist. It was a hack to get around certain limitations of tree-sitter that couldn't be expressed via native queries. E.g.

(_
     (nested
        (subtree
           (here))))

(foo
  (...      <---- arbitrary nesting here
     (nested
        (subtree
           (here)))))

#has-ancestor was there before :parent() got it's parent cache removed. We planned to reduce it's usage in nvim-treesitter and eventually fade it out.

I thought that we removed has-ancestor from the C family nesting limited copy&paste. But I haven't been a around for a long time and it is now actively used again.

Ideally, tree-sitter should answer whether this is a common usecase that should be expressible by the queries or whether this is something that users should avoid. With tree-sitter/tree-sitter#3214 (review) tree-sitter could send the message that requesting parent is something you can do.

[tomtomjhj]

Closing, since (1) the other solution is much nicer, (2) the linear factor on the depth of the node is not avoidable, and (3) this factor can be bounded by max_start_depth.