fix(query): prevent cycles when analyzing hidden children
**Problem:** `query.c` compares the current analysis state with the
previous analysis state to see if they are equal, so that it can return
early if so. This prevents redundant work. However, the comparison
function here differs from the one used for sorted insertion/lookup in
that it does not check any state data other than the child index. This
is problematic because it leads to infinite analysis when hidden nodes
have cycles.
**Solution:** Remove the custom comparison function, and apply the
insertion/lookup comparison function in place of it.
**NOTE:** This commit also changes the comparison function slightly, so
that some comparisons are reordered. Namely, for performance, it returns
early if the lhs depth is less than the rhs depth. Is this acceptable?
Tests still pass and nothing hangs in my testing, but it still seems
sketchy. Returning early if the lhs depth is greater than the rhs depth
does seem to make query analysis hang, weirdly enough... Keeping the
depth checks at the end of the loop also works, but it introduces a
noticeable performance regression (for queries that otherwise wouldn't
have had analysis cycles, of course).
(cherry picked from commit 6850df969d)
This commit is contained in:
parent
d507a2defb
commit
e7f4dfcd4a
1 changed files with 15 additions and 24 deletions
|
|
@ -928,35 +928,26 @@ static unsigned analysis_state__recursion_depth(const AnalysisState *self) {
|
|||
return result;
|
||||
}
|
||||
|
||||
static inline int analysis_state__compare_position(
|
||||
AnalysisState *const *self,
|
||||
AnalysisState *const *other
|
||||
) {
|
||||
for (unsigned i = 0; i < (*self)->depth; i++) {
|
||||
if (i >= (*other)->depth) return -1;
|
||||
if ((*self)->stack[i].child_index < (*other)->stack[i].child_index) return -1;
|
||||
if ((*self)->stack[i].child_index > (*other)->stack[i].child_index) return 1;
|
||||
}
|
||||
if ((*self)->depth < (*other)->depth) return 1;
|
||||
if ((*self)->step_index < (*other)->step_index) return -1;
|
||||
if ((*self)->step_index > (*other)->step_index) return 1;
|
||||
return 0;
|
||||
}
|
||||
|
||||
static inline int analysis_state__compare(
|
||||
AnalysisState *const *self,
|
||||
AnalysisState *const *other
|
||||
) {
|
||||
int result = analysis_state__compare_position(self, other);
|
||||
if (result != 0) return result;
|
||||
if ((*self)->depth < (*other)->depth) return 1;
|
||||
for (unsigned i = 0; i < (*self)->depth; i++) {
|
||||
if ((*self)->stack[i].parent_symbol < (*other)->stack[i].parent_symbol) return -1;
|
||||
if ((*self)->stack[i].parent_symbol > (*other)->stack[i].parent_symbol) return 1;
|
||||
if ((*self)->stack[i].parse_state < (*other)->stack[i].parse_state) return -1;
|
||||
if ((*self)->stack[i].parse_state > (*other)->stack[i].parse_state) return 1;
|
||||
if ((*self)->stack[i].field_id < (*other)->stack[i].field_id) return -1;
|
||||
if ((*self)->stack[i].field_id > (*other)->stack[i].field_id) return 1;
|
||||
if (i >= (*other)->depth) return -1;
|
||||
AnalysisStateEntry s1 = (*self)->stack[i];
|
||||
AnalysisStateEntry s2 = (*other)->stack[i];
|
||||
if (s1.child_index < s2.child_index) return -1;
|
||||
if (s1.child_index > s2.child_index) return 1;
|
||||
if (s1.parent_symbol < s2.parent_symbol) return -1;
|
||||
if (s1.parent_symbol > s2.parent_symbol) return 1;
|
||||
if (s1.parse_state < s2.parse_state) return -1;
|
||||
if (s1.parse_state > s2.parse_state) return 1;
|
||||
if (s1.field_id < s2.field_id) return -1;
|
||||
if (s1.field_id > s2.field_id) return 1;
|
||||
}
|
||||
if ((*self)->step_index < (*other)->step_index) return -1;
|
||||
if ((*self)->step_index > (*other)->step_index) return 1;
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -1247,7 +1238,7 @@ static void ts_query__perform_analysis(
|
|||
// the states that have made the least progress. Avoid advancing states that have already
|
||||
// made more progress.
|
||||
if (analysis->next_states.size > 0) {
|
||||
int comparison = analysis_state__compare_position(
|
||||
int comparison = analysis_state__compare(
|
||||
&state,
|
||||
array_back(&analysis->next_states)
|
||||
);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue