fd2771d51dfa6526d51778a1a3e2e553aa75290c braney Tue Sep 1 10:37:16 2026 -0700 trackDbConditions: follow a setting to where its value is used, refs #37908 The first pass only saw a condition when it enclosed the read. That misses the commonest shape in the drawing code: the setting is read plainly at the top of a loader, carried into a struct, and used far below behind a test of a different setting. bamColorTag came back unconditional even though it does nothing unless bamColorMode is tag. So follow the value. A name that holds exactly one setting across a file is taken to carry it, including into a struct field of the same name, which is how the value usually travels. Then find where the value is used and intersect the conditions guarding those uses. Two distinctions do the work. A mention at paren depth zero is one side of an assignment or an element of an initializer list, which only moves the value somewhere else, so it is not a use; inside a call it is an argument and it is. Counting the struct initializer as a use put an unguarded site in the set and emptied every intersection. And matching drops the field prefix, so the test written sameString(colorMode, ...) in the loader is the same condition as sameString(btd->colorMode, ...) in the drawer. These are reported as "when used" and kept apart from "always". The every-path conditions are necessary by construction; a use-site condition is only as good as the set of uses found, so it is a strong hint rather than a claim, and it keeps the strict key rather than the loose one for that reason. 43 settings in the render scope are read plainly and used only under a condition, 21 of them documented. Among them: bamColorTag needs bamColorMode=tag, pairSearchRange needs pairEndsByName, speciesCodonDefault needs mafChain and frames, and speciesOrder, speciesGroups and speciesDefaultOff turn out to gate each other. diff --git src/hg/utils/trackDbConditions/harvestConditions.py src/hg/utils/trackDbConditions/harvestConditions.py index 0ad2abf29eb..dcec4055008 100755 --- src/hg/utils/trackDbConditions/harvestConditions.py +++ src/hg/utils/trackDbConditions/harvestConditions.py @@ -299,81 +299,93 @@ j = skipSpace(s, i + len(word)) if depth == 0 and j < n and s[j] == "(" and word not in NOT_A_CALL: 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: 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 + 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 = [] + 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] == "_")): word = m.group(1) j = skipSpace(s, i + len(word)) if j < n and s[j] == "(": close = matchParen(s, j) cond = re.sub(r"\s+", " ", src[j+1:close-1]).strip() end = stmtEnd(s, close) guards.append((close, end, word, cond, line[i])) + condSpans.append((j, close)) p = skipSpace(s, end) if s[p:p+4] == "else" and not (p+4 < n and (s[p+4].isalnum() or s[p+4] == "_")): q = skipSpace(s, p + 4) guards.append((q, stmtEnd(s, q), "else", "NOT (%s)" % cond, line[p])) elif word == "if" and LEAVES.match(src[close:end]): # the guarded statement leaves, so the rest of the function # is only reached when the condition is false for fs, fe, _ in funcs: if fs <= i < fe: 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]): - derived = sorted({fromSetting[(func, ident)] - for ident in re.findall(r"[A-Za-z_][A-Za-z0-9_]*", part) - if (func, ident) in fromSetting}) + 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}) 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 @@ -401,30 +413,82 @@ # depth > 0 keeps a definition or a prototype from counting as a call calls.append({"callee": word, "caller": enclosing(i), "file": rel, "line": line[i], "conds": condsAt(i)}) i += len(word) continue if not isCall and depth > 0 and re.fullmatch(r"[a-z][A-Za-z0-9_]*", word): # only a real use as a value, which is how a track method is # installed: tg->drawItems = bedDrawItems; or &someFunc before = src[max(0, i-2):i].strip() after = s[skipSpace(s, i + len(word)):][:1] if (before.endswith("=") or before.endswith("&") or before.endswith(",")) \ and after in (";", ",", ")"): addrTaken.add(word) i += len(word) + # Where the value goes. A setting is often read plainly and only used + # under a test, which is the bamColorTag shape: the value is read at the top + # of the loader and used far below, behind a test of bamColorMode. So for + # each name that carries a setting, find where the value is used and what + # guards those uses. + inCond = lambda off: any(a <= off < b for a, b in condSpans) + + def isTransfer(off): + """The value being handed on, not consumed. + + The test is how deep in parentheses the mention sits, counted from the + start of its statement. Inside a call it is an argument, so something + is doing work with it: bamGetTagString(bam, btd->userTag, ...) is a use. + At depth zero it is one side of an assignment or an element of an + initializer list, which only moves the value somewhere else: + struct bamTrackData btd = {tg, pairHash, colorMode, userTag, ...} + carries the setting into a field without using it. Counting that as a + use puts an unguarded site in the set and empties every intersection. + """ + start = max(s.rfind(";", 0, off), s.rfind("{", 0, off), s.rfind("}", 0, off)) + depth = 0 + for ch in s[start + 1:off]: + if ch in "([": + depth += 1 + elif ch in ")]": + depth -= 1 + return depth <= 0 + + useConds = {} + for var, setting in carries.items(): + sites = [] + for m in re.finditer(r"\b%s\b" % re.escape(var), s): + off = m.start() + if off in assignAt[var] or inCond(off) or enclosing(off) is None: + continue + if isTransfer(off): + continue + sites.append(condsAt(off)) + if not sites: + continue + common = {useKey(c): c for c in sites[0]} + for one in sites[1:]: + keys = {useKey(c) for c in one} + common = {k: v for k, v in common.items() if k in keys} + if not common: + break + # a test of the setting's own value says only that it is set + useConds[setting] = [c for c in common.values() + if c.get("derivedFrom") != [setting]] + for read in reads: + read["useConds"] = useConds.get(read["name"], []) if read["tdb"] else [] + return {"file": rel, "funcs": [f[2] for f in funcs], "reads": reads, "calls": calls, "addrTaken": sorted(addrTaken)} def splitConjuncts(text): """A && B is two conditions. Splitting it is sound; splitting || is not.""" parts, depth, cur = [], 0, "" i = 0 while i < len(text): c = text[i] if c in "([": depth += 1 elif c in ")]": depth -= 1 if depth == 0 and text[i:i+2] == "&&": @@ -432,30 +496,43 @@ cur = "" i += 2 continue cur += c i += 1 parts.append(cur) parts = [p.strip() for p in parts if p.strip()] return parts if len(parts) > 1 else [text] def condKey(cond): """Two conditions are the same when they say the same thing, spacing aside.""" return (re.sub(r"\s+", "", cond["text"]), cond["kind"]) +def useKey(cond): + """Looser key, for matching a test of a value against a test of the field holding it. + + The same test is written sameString(colorMode, ...) in the loader and + sameString(btd->colorMode, ...) in the drawer, because the value was carried + into a struct on the way. Dropping the field prefix makes those one + condition. Only the use-site pass uses this; the every-path analysis keeps + the strict key, since there a wrong match would turn a guess into a claim. + """ + text = re.sub(r"\b\w+\s*->\s*", "", cond["text"]) + return (re.sub(r"\s+", "", text), cond["kind"]) + + UI_NAME = re.compile(r"(CfgUi|Ui|UiSection|Option|Options|Menu|Dropdown|Section|Cfg)$") class Graph: """The call graph of one scope, and the conditions it can prove.""" def __init__(self, units, scopeFiles, dropUiCallers=False): self.defined = set() self.addrTaken = set() for unit in units: self.defined.update(unit["funcs"]) self.addrTaken.update(unit["addrTaken"]) self.external = self.addrTaken & self.defined self.calls = [] for unit in units: