fd380f3ae9c678f8e92b4728d5f614a71764fecd braney Tue Sep 1 12:56:18 2026 -0700 trackDbConditions: four fixes found by walking the results against the code, refs #37908 Walking the worklist row by row is what found these; none of them was visible from the summary counts. A read wrapped in another call was invisible to the value map, because it stopped at the first call name. cloneString(trackDbSetting(...)) and atoi(cartOrTdbString(...)) are both common. This is why hideEmptySubtracksSourcesUrl came back as merely composite-only when it in fact also needs hideEmptySubtracks and hideEmptySubtracksMultiBedUrl. A macro in a condition was read as an unknown word, so cartVarExistsAnyLevel(cart, tdb, FALSE, MAF_CHAIN_VAR) did not resolve. That hid the fact that irows is consulted only when mafChain is absent from the cart. Same indirection trap as the chained defines, in a different place. The variable-to-setting map kept only the last assignment. wigFetchMinMaxYWithCart assigns defaultViewLimits from defaultViewLimits and then, if that came back NULL, from viewLimits, so the test in between looked like viewLimits testing itself. It is now resolved at the position of the test, which turns an artifact into the real finding: viewLimits is read only when defaultViewLimits is absent. Two classes of noise removed. "Read the trackDb value when the cart has none" is how every setting with a default resolves, and it was a third of the worklist. A guard on the trackDb type line having words is a sanity check that is true of every track, and it was the whole of what the scan had to say about chainNormScoreAvailable, lollyMaxSize and lollyNoStems. diff --git src/hg/utils/trackDbConditions/harvestConditions.py src/hg/utils/trackDbConditions/harvestConditions.py index 02312851ca0..b13b79e89ae 100755 --- src/hg/utils/trackDbConditions/harvestConditions.py +++ src/hg/utils/trackDbConditions/harvestConditions.py @@ -105,30 +105,35 @@ # read while never having seen one of them. So scan the libraries whole and let # reachability decide which side of the browser each read belongs to. SHARED = ["hg/lib/*.c", "hg/cgilib/*.c"] # A read reached with none of these is unguarded. Anything matching is noise # rather than a condition on the setting: it says the code got far enough to # run, not that the setting only applies sometimes. NOISE = re.compile( r"^NOT \((?:[\w>.\-]+(?:\s*->\s*\w+)* == NULL)" r"(?:\s*\|\|\s*[\w>.\-]+(?:\s*->\s*\w+)* == NULL)*\)$" r"|^errCatchStart\b" r"|^\w+ != NULL$" r"|^NOT \(isEmpty\(" r"|^NOT \(!\w+\)$" r"|^\w+ = \w+; \w+ != NULL" # for (x = y; x != NULL; ...) + # the words of the trackDb type line. A track always has a type, so a guard + # on the count being positive is a sanity check, not a condition: it was + # showing up as a "data" condition on chainNormScoreAvailable, lollyMaxSize + # and lollyNoStems, and it means nothing on any of them. + r"|^NOT \(word(Count|Ct) <= 0\)$|^word(Count|Ct) > 0$" ) def blankOut(src): """Comments and string bodies become spaces, so offsets and lines still line up.""" out = list(src) i, n = 0, len(src) while i < n: c = src[i] if c == "/" and i + 1 < n and src[i+1] == "/": while i < n and src[i] != "\n": out[i] = " " i += 1 elif c == "/" and i + 1 < n and src[i+1] == "*": out[i] = out[i+1] = " " @@ -307,43 +312,61 @@ end = matchParen(s, j) p = skipSpace(s, end) if p < n and s[p] == "{": funcs.append((p, stmtEnd(s, p), word)) i += len(word) def enclosing(off): return next((f[2] for f in funcs if f[0] <= off < f[1]), None) # char *scoreMinStr = trackDbSettingClosestToHome(tdb, GRAY_LEVEL_SCORE_MIN); # A later test of scoreMinStr is a test of the setting, not housekeeping, so # remember which local holds which setting. fromSetting = {} fileValue = collections.defaultdict(set) # name -> settings it ever holds assignAt = collections.defaultdict(list) # name -> offsets of its assignments - for m in re.finditer(r"([A-Za-z_][A-Za-z0-9_]*)\s*=\s*([A-Za-z_][A-Za-z0-9_]*)\s*\(", s): - reader = m.group(2) - if reader not in READERS: + for m in re.finditer(r"([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(?=[A-Za-z_])", s): + # The read is often wrapped: cloneString(trackDbSetting(...)) and + # atoi(cartOrTdbString(...)) are both common, and stopping at the first + # call name loses them. hideEmptySubtracksSourcesUrl needing + # hideEmptySubtracksMultiBedUrl was missed for exactly this reason. + pos, reader, open_ = m.end(), None, None + for _ in range(4): + call = re.match(r"([A-Za-z_][A-Za-z0-9_]*)\s*\(", s[pos:]) + if not call: + break + here = pos + call.end() - 1 + if call.group(1) in READERS: + reader, open_ = call.group(1), here + break + pos = here + 1 + if reader is None: continue - open_ = s.index("(", m.end(2)) close = matchParen(s, open_) args = splitArgs(src[open_ + 1:close - 1]) idx = READERS[reader] if idx >= len(args): continue name = settingNameOf(args[idx], defines) if name and not name.startswith("{"): - fromSetting[(enclosing(m.start(1)), m.group(1))] = name + # Keep every assignment with its offset. wigFetchMinMaxYWithCart + # assigns defaultViewLimits from defaultViewLimits and then, if that + # came back NULL, from viewLimits. Remembering only the last one + # made the test in between look like viewLimits testing itself, and + # hid a real relationship between two settings. + fromSetting.setdefault((enclosing(m.start(1)), m.group(1)), []).append( + (m.start(1), name)) fileValue[m.group(1)].add(name) assignAt[m.group(1)].append(m.start(1)) # A name is only taken to carry a setting when it carries exactly one, and # is long enough not to be a word like type, name or vis that means # something else three functions away. carries = {var: sorted(names)[0] for var, names in fileValue.items() if len(names) == 1 and len(var) >= 5} # guards guards, condSpans = [], [] i = 0 while i < n: m = re.match(r"\b(if|while|for|switch)\b", s[i:]) if m and (i == 0 or not (s[i-1].isalnum() or s[i-1] == "_")): @@ -372,33 +395,48 @@ if fs <= i < fe: if line[i] - line[fs] <= PRECONDITION_LINES: guards.append((end, fe, "guard", "NOT (%s)" % cond, line[i])) break i = close continue i += 1 def condsAt(off): out = [] for g in guards: if g[0] <= off < g[1]: func = enclosing(g[0]) for part in splitConjuncts(g[3]): idents = re.findall(r"[A-Za-z_][A-Za-z0-9_]*", part) - derived = sorted({fromSetting[(func, ident)] for ident in idents - if (func, ident) in fromSetting} - | {carries[ident] for ident in idents if ident in carries}) + derived = set() + for ident in idents: + # A macro in the test is the setting it expands to. + # cartVarExistsAnyLevel(cart, tdb, FALSE, MAF_CHAIN_VAR) + # is a test of mafChain, and reading it as an unknown + # word hid that irows is only consulted when mafChain is + # absent from the cart. + if re.fullmatch(r"[A-Z][A-Z0-9_]*", ident) and ident in defines: + derived.add(defines[ident]) + continue + writes = fromSetting.get((func, ident)) + if writes: + # the assignment in force where the test is written + before = [w for w in writes if w[0] < g[0]] + derived.add((before[-1] if before else writes[0])[1]) + elif ident in carries: + derived.add(carries[ident]) + derived = sorted(derived) out.append({"kind": g[2], "text": part, "line": g[4], "file": rel, "derivedFrom": derived}) return out # reads and calls, both only inside a function body reads, calls, addrTaken = [], [], set() depth, i = 0, 0 while i < n: c = s[i] if c == "{": depth += 1 i += 1 continue if c == "}": depth -= 1