:list:member declares ArgModeOutput but Decide handles a bound first argument
- Dominant language
- Go
- Stars
- 3k
- Forks
- 157
- PR merge metrics
- No merged PRs in 30d
Description
## Summary
`:list:member`'s declared mode is narrower than its implementation. The analyser refuses a bound
first argument that `Decide` would have handled, so a membership *test* is rejected while the code
to perform it is present and reachable.
## Where
`builtin/builtin.go`, both on `main` at the time of writing:
```go
// line 54 — the declaration
symbols.ListMember: {ast.ArgModeOutput, ast.ArgModeInput},
```
```go
// line ~445 — the evaluator, which handles the bound case
evaluatedMember := atom.Args[0]
memberVar, memberIsVar := evaluatedMember.(ast.Variable)
if memberIsVar && subst != nil {
evaluatedMember = subst.Get(memberVar)
_, memberIsVar = evaluatedMember.(ast.Variable)
}
if !memberIsVar { // We are looking for a member
res, err := functional.EvalExpr(
ast.ApplyFn{symbols.ListContains, []ast.BaseTerm{evaluatedArg, evaluatedMember}}, nil)
...
}
```
The `if !memberIsVar` branch and its comment are the bound case, delegating to `:list:contains`.
`ArgModeOutput` means analysis never lets a program reach it.
## Reproduction
Against unpatched `main`:
```
seen(/a).
ok(X) :- seen(X), :list:member(X, [/a, /b]).
```
```
ANALYSIS ERROR: for goal ":list:member(X,fn:list(/a,/b))" expected X (arg 0) to be a free variable
```
Four shapes tried; three are refused:
| shape | result |
|---|---|
| `seen(X), :list:member(X, [/a, /b])` | **refused** |
| `seen(X), approved(L), :list:member(X, L)` | **refused** |
| `X = /a, :list:member(X, [/a, /b])` | **refused** |
| `seen(X), approved(L), !:list:member(X, L)` | accepted |
The negated form being accepted is worth noting: my first attempt at a reproduction used it, found
nothing, and would have had me conclude there was no issue.
Changing the declaration to `{ast.ArgModeInputOutput, ast.ArgModeInput}` accepts all four, and the
existing `builtin` tests pass unchanged.
## What I am not proposing
I am not sending a patch. We carried that one-token change in a fork and have now removed it,
because we do not use the capability — our one call site uses the generator form, which needs no
change. Reporting it rather than patching it seemed the right way round.
Two things I have not established, and would defer to you on:
- whether the restriction is deliberate, with the evaluator branch kept for a caller I have not
found; and
- whether `InputOutput` is the right widening, as against a separate predicate.
Filed here rather than on Codeberg because I have no account there; happy to move it.
Context: this is the same reporter as #92 and #93. Both of those rested on claims I could not
reproduce and I have withdrawn #93 accordingly. This one I did reproduce, and the shapes and the
error text above are what that looks like — it seemed only fair to show the work this time.
Contributor guide
Research direction
Start in builtin/builtin.go by comparing the :list:member declaration near line 54 with the bound-argument branch in Decide near line 445. Reproduce the four argument shapes from the issue and run the existing builtin tests. Done means the intended mode restriction or widening is decided and the reproduced behavior is covered by tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 64/100