Declare every tooling builder view so an implementation can read state

[thedavidmeister]

Closes#92

IParserToolingV1 and ISubParserToolingV1 declared their three builders
pure. Solidity forbids an override from loosening mutability, so that was a
hard ceiling: no parser or sub parser could build its pointer table from storage
or from an immutable — the normal Rain pattern where a sub parser embeds its own
or an extern's address into the bytecode it emits. Their two siblings,
IIntegrityToolingV1 and IOpcodeToolingV1, already declared theirs view.

All five builders across the four interfaces are now view. The three
LibCodeGen wrappers that were pure only because their interface was are now
view. ToolingMock — which had to opt out of the interfaces entirely and
document the ceiling in a @dev paragraph — now inherits all four, so the
compiler checks every builder's name, arguments, return type and mutability
against the interface that declares it.

The test is the mock's inheritance

There is no runtime behaviour to assert here: the wrappers emit byte-identical
strings before and after. The defect is a compile-time one, so the test is a
compile-time one — ToolingMock reads all five answers from storage and declares
is IOpcodeToolingV1, IParserToolingV1, ISubParserToolingV1, IIntegrityToolingV1.
Under the old declarations that does not build. It is also the compile-time proof
of implementability that #74 asks for.

Failing, before the fix

ToolingMock inheriting the four interfaces, interfaces untouched,
nix develop -c forge test:

Compiling 6 files with Solc 0.8.25
Error: Compiler run failed:
Error (6959): Overriding function changes state mutability from "pure" to "view".
--> test/concrete/ToolingMock.sol:48:5:
Note: Overridden function is here:
--> src/interface/IParserToolingV1.sol:28:5:
Error (6959): Overriding function changes state mutability from "pure" to "view".
--> test/concrete/ToolingMock.sol:53:5:
Note: Overridden function is here:
--> src/interface/IParserToolingV1.sol:18:5:
Error (6959): Overriding function changes state mutability from "pure" to "view".
--> test/concrete/ToolingMock.sol:58:5:
Note: Overridden function is here:
--> src/interface/ISubParserToolingV1.sol:17:5:

Three errors, on exactly the three declarations the issue names.

Passing, after the fix

Ran 16 test suites in 1.43s (12.48s CPU time): 134 tests passed, 0 failed, 0 skipped (134 total tests)

Same 134 tests as on main at 935c725; none were added, changed or removed.

Mutation matrix

Every mutant flips exactly one view this change touched back to pure and runs
the whole suite. M0 is the unmutated baseline, run first, so a later KILLED is a
real difference rather than a harness that never ran. Each mutant asserts the
target line's exact content before and after editing it, so a silently-missed
edit aborts instead of reading as SURVIVED. cache/fuzz/failures is deleted
before every run so no counterexample is replayed from a previous mutant. The
tree was clean (git status --short empty) after the run.

#	mutation	suite	verdict
M0	none (baseline)	134 passed, 0 failed	n/a
M1	IParserToolingV1.buildOperandHandlerFunctionPointersview→pure	Error (6959)	KILLED
M2	IParserToolingV1.buildLiteralParserFunctionPointersview→pure	Error (6959)	KILLED
M3	ISubParserToolingV1.buildSubParserWordParsersview→pure	Error (6959)	KILLED
M4	IOpcodeToolingV1.buildOpcodeFunctionPointersview→pure	Error (6959)	KILLED
M5	IIntegrityToolingV1.buildIntegrityFunctionPointersview→pure	Error (6959)	KILLED
M6	LibCodeGen.literalParserFunctionPointersConstantStringview→pure	Error (2527)	KILLED
M7	LibCodeGen.operandHandlerFunctionPointersConstantStringview→pure	Error (2527)	KILLED
M8	LibCodeGen.subParserWordParsersConstantStringview→pure	Error (2527)	KILLED

8/8 killed. M4 and M5 are the two interfaces this change did not flip: they show
the mock's inheritance guards all four, not only the two that moved.

Error (6959) is Overriding function changes state mutability from "pure" to "view". Error (2527) is Function declared as pure, but this expression (potentially) reads from the environment or state and thus requires "view".

interfaceId does not move

The issue's note on this is garbled, so it was measured rather than reasoned
about. A throwaway probe logged type(I…).interfaceId and every builder selector
on the unmutated tree before and after the change:

interface	before	after
IOpcodeToolingV1	0x514b5d4f	0x514b5d4f
IParserToolingV1	0x1a2c8edd	0x1a2c8edd
ISubParserToolingV1	0x336284d4	0x336284d4
IIntegrityToolingV1	0xb92d7553	0xb92d7553

Every builder selector is also unchanged: buildOperandHandlerFunctionPointers
0xd6d8c9a8, buildLiteralParserFunctionPointers0xccf44775,
buildSubParserWordParsers0x336284d4, buildOpcodeFunctionPointers
0x514b5d4f, buildIntegrityFunctionPointers0xb92d7553.

An interface id is the XOR of its function selectors, and a selector is the first
four bytes of keccak256 over the canonical signature — name(argTypes). State
mutability is not in that string, so it cannot reach the selector, and an id built
from unchanged selectors is unchanged. 0xd6d8c9a8 ^ 0xccf44775 == 0x1a2c8edd
reproduces IParserToolingV1's id by hand from the two measured selectors.

What that means for already-deployed contracts: nothing. Every deployed
RainlangParser, RaindexV6SubParser and extern advertising 0x1a2c8edd /
0x336284d4 keeps answering the same ids, and a consumer probing an id computed
from this source keeps getting true. This is not a new interface, so no V2
is minted and #74's proposed id pins are still the pre-change literals.

The probe was not committed. #74 owns the permanent id pin, and duplicating it
here would collide with that PR.

Where the issue's proposed fix was wrong

1. It says the change is "source-breaking for any existing pure implementer's
   override". It is not — measured, see below.
2. It proposes flipping only the three pure declarations. This PR also adds the
   mutability rationale to the two that were already view, because the finding
   is that all four split their mutability "with no stated rationale" — leaving
   two of five declarations unexplained just relocates the finding.

Downstream: nothing breaks, measured rather than argued

Solidity allows an override to tighten mutability — view may be overridden by
pure — so a pure implementation of a view declaration compiles. Probed in
this repo on this branch with a throwaway contract inheriting all four interfaces
and declaring all five builders external pure override:

Compiler run successful!
[PASS] testPureImplementerConforms() (gas: 134903)

Not committed — it is #74's ConformingToolingMock in all but name.

rain.interpreter at HEAD (main, shallow clone) was compiled twice, before and
after patching its vendored rain-sol-codegen-0.1.0 copy exactly the way this PR
patches the source — three interface declarations pure→view plus the three
LibCodeGen wrappers:

	nix develop -c forge build
unpatched dependencies/rain-sol-codegen-0.1.0	Compiling 512 files with Solc 0.8.25 / Compiler run successful with warnings
patched to view	Compiling 512 files with Solc 0.8.25 / Compiler run successful with warnings

512 files is src plus test plus script, so this covers RainlangParser and
RainlangReferenceExtern's five external pure override builders, the six
pure test sub-parsers, and script/Build.sol's calls into the three widened
wrappers. Nothing had to change on the consumer side.

rain.flare and raindex were checked by reading their call sites rather than
built. Both hold the same shape and are unaffected for the same reason:

• Their sub parser builders are external pure — FlareFtsoSubParser.sol:63,78,86
  and RaindexV6SubParser.sol:102,107,193 — which is a legal tightening.
• The one construct that could break is a pure caller reaching a builder
  through the now-view interface type. raindex has two external pure test
  functions calling builders — RaindexV6SubParser.pointers.t.sol:47,54 — but
  they call through the concrete RaindexV6SubParser type, whose own declarations
  stay pure, so the call site's mutability is unchanged.
• Both script/Build.sol call sites into the widened wrappers sit in non-view
  functions (raindex:49 buildRaindexSubParserPointers() internal, rain.flare
:15 buildFlareFtsoWordsPointers() internal), which deploy contracts.

No consumer anywhere calls a builder through the interface type from a pure
context. LibCodeGen's three wrappers were the only such callers, and they are
widened here.

Also changed, beyond the diff the issue drafted

The @dev paragraph on ToolingMock recorded the workaround rather than the
defect. It is replaced by a paragraph describing what the mock now is.

QA

• Discriminating tests: test/concrete/ToolingMock.sol declaring
  is IOpcodeToolingV1, IParserToolingV1, ISubParserToolingV1, IIntegrityToolingV1
  while reading all five answers from storage — a compile-time test, since the
  defect is a compile-time one. Verified failing on base: with that inheritance
  applied and the interfaces untouched, nix develop -c forge test fails to
  compile with three Error (6959) on ToolingMock.sol:48,53,58, quoted in full
  above. The whole 134-test suite is the thing that goes from not-building to
  passing.
• Mutations applied: 8 mutants, all KILLED, matrix above.
  src/interface/IParserToolingV1.sol:21view→pure → whole suite fails to
  compile, Error (6959) at ToolingMock.sol (M1); :34 likewise (M2);
  src/interface/ISubParserToolingV1.sol:20 likewise (M3);
  src/interface/IOpcodeToolingV1.sol:21 likewise (M4);
  src/interface/IIntegrityToolingV1.sol:22 likewise (M5);
  src/lib/LibCodeGen.sol:145, :171, :195view→pure → Error (2527) in
  LibCodeGen.sol itself (M6, M7, M8). Baseline M0 ran first and passed 134/134,
  so the kills are differences rather than a harness that never ran.
• Oracle: the Solidity language rule, not this repo's code — an override may only
  tighten mutability, and a pure function may not read state. The compiler is
  the independent judge of both, and it reports them as Error (6959) and
  Error (2527) respectively; both error codes were named in the issue from
  probes taken before this change. The interfaceId claim is judged against the
  ABI selector definition (keccak256("name(argTypes)"), mutability absent) and
  cross-checked by measuring the ids on both trees, plus 0xd6d8c9a8 ^ 0xccf44775 == 0x1a2c8edd by hand.
• Category check: the issue asks for (A) the three pure interface declarations
  made view, (B) the three LibCodeGen wrappers widened to view, (C) the
  @dev workaround paragraph deleted and ToolingMock inheriting all four
  interfaces, (D) the ABI/interfaceId and downstream-implementer impact
  confirmed against rain.interpreter. Covered A, B, C, D. Beyond them, the two
  already-view declarations get the same mutability rationale, because the
  finding is that all four split their mutability with none stated.
• nix develop -c forge test on the RED tree (mock inherits, interfaces still
  pure): Error: Compiler run failed with three Error (6959), quoted above.
• nix develop -c forge test on the GREEN tree: Ran 16 test suites in 1.43s (12.48s CPU time): 134 tests passed, 0 failed, 0 skipped (134 total tests).
• Mutation matrix, 8 mutants + an unmutated baseline: 8/8 KILLED, table above,
  full transcript kept at
  /home/gildlab/artifacts/work/rain.sol.codegen-fix-92-mutations.txt.
• nix develop -c forge fmt --check: exit 0, no files listed.
• type(I…).interfaceId probed before and after on the unmutated tree: all four
  ids and all five selectors byte-identical.
• nix develop -c forge build in a fresh rain.interpreter clone, before and
  after patching its vendored rain-sol-codegen the way this PR patches source:
  Compiling 512 files with Solc 0.8.25 / Compiler run successful with warnings both times.
• A throwaway contract on this branch inheriting all four interfaces with five
  external pure override builders: Compiler run successful!, test passes.
• CodeRabbit reported Review limit reached — @thedavidmeister's PR review
  limit — so its green check is an absence of review, not a passed one. Its
  unresolved-thread count over GraphQL is zero for the same reason.

[coderabbitai]

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in:33 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4a18a44c-bd5e-4d3b-ab0f-b7820bd42399

📥 Commits

Reviewing files that changed from the base of the PR and between 1bcf2f9 and 8419581.

📒 Files selected for processing (6)

• src/interface/IIntegrityToolingV1.sol
• src/interface/IOpcodeToolingV1.sol
• src/interface/IParserToolingV1.sol
• src/interface/ISubParserToolingV1.sol
• src/lib/LibCodeGen.sol
• test/concrete/ToolingMock.sol

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}