Skip to content

Port tutorial analyses to SimplifiedSpec and reuse SimplifiedUnitAnalysis defaults while preserving unitAnalysis compatibility - #2043

Open
michael-schwarz with Copilot wants to merge 14 commits into
masterfrom
copilot/port-tutorial-analyses
Open

Port tutorial analyses to SimplifiedSpec and reuse SimplifiedUnitAnalysis defaults while preserving unitAnalysis compatibility#2043
michael-schwarz with Copilot wants to merge 14 commits into
masterfrom
copilot/port-tutorial-analyses

Conversation

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor

This PR ports tutorial analyses to the SimplifiedSpec interface while preserving compatibility for analyses that depend on UnitAnalysis.Spec.

In addition, based on review feedback, it introduces reusable default transfer-function behavior in SimplifiedUnitAnalysis and applies it where feasible to reduce boilerplate in tutorial analyses.

What changed

  • Kept unitAnalysis.ml in its original MCPSpec-compatible form.
  • Added a separate simplified tutorial analysis (simplifiedUnitAnalysis.ml) and exported it from src/goblint_lib.ml.
  • Introduced SimplifiedUnitAnalysis.DefaultSpec for shared no-op/default simplified transfer functions.
  • Updated tutorial signs analyses to include SimplifiedUnitAnalysis.DefaultSpec where feasible:
    • src/analyses/tutorials/signs.ml
    • src/analyses/tutorials/solution/signsSol.ml
    • src/analyses/tutorials/solution/signsExtendSol.ml
  • Kept analysis-specific behavior explicit where semantics differ (e.g., context, threadenter).

Validation

  • Attempted make native in this environment (blocked by missing opam/dune).
  • Ran final parallel validation:
    • Code Review: no remaining findings.
    • CodeQL: no analyzable-language findings for these changes.

Agent-Logs-Url: https://github.com/goblint/analyzer/sessions/04ac7a0b-b764-4f8a-b7c0-f96bcb75fc44

Co-authored-by: michael-schwarz <13812333+michael-schwarz@users.noreply.github.com>
Copilot AI changed the title [WIP] Port tutorial analyses over to simplified spec Port tutorial analyses to SimplifiedSpec registration/interface May 23, 2026
Copilot AI requested a review from michael-schwarz May 23, 2026 03:53
Comment thread src/analyses/tutorials/unitAnalysis.ml
Agent-Logs-Url: https://github.com/goblint/analyzer/sessions/4163237d-68cb-41e2-ac46-468ac7eb3df5

Co-authored-by: michael-schwarz <13812333+michael-schwarz@users.noreply.github.com>
Copilot AI changed the title Port tutorial analyses to SimplifiedSpec registration/interface Port tutorial analyses to SimplifiedSpec while preserving unitAnalysis compatibility May 23, 2026
Copilot AI requested a review from michael-schwarz May 23, 2026 05:54
@michael-schwarz

Copy link
Copy Markdown
Member

@copilot: Modules missing from src/goblint_lib.ml: {'SimplifiedUnitAnalysis'}

Agent-Logs-Url: https://github.com/goblint/analyzer/sessions/51579562-8e2a-4d5d-af08-de4f4244ecb5

Co-authored-by: michael-schwarz <13812333+michael-schwarz@users.noreply.github.com>

Copilot AI commented May 23, 2026

Copy link
Copy Markdown
Contributor Author

@copilot: Modules missing from src/goblint_lib.ml: {'SimplifiedUnitAnalysis'}

Fixed in 812244e by adding module SimplifiedUnitAnalysis = SimplifiedUnitAnalysis to /home/runner/work/analyzer/analyzer/src/goblint_lib.ml.

Comment thread src/analyses/tutorials/solution/signsExtendSol.ml

@michael-schwarz michael-schwarz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SimplifiedUnitAnalysis should be used to reduce boilerplate in the other analyses.

Copilot AI commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot: Modules missing from src/goblint_lib.ml: {'SimplifiedUnitAnalysis'}

This is already addressed in commit 812244e: module SimplifiedUnitAnalysis = SimplifiedUnitAnalysis is exported from /home/runner/work/analyzer/analyzer/src/goblint_lib.ml.

@michael-schwarz

Copy link
Copy Markdown
Member

@copilot: The tutorial analysis should, where feasible, not define everything from scratch but build on the SimplifiedUnitAnalysis.

@michael-schwarz

Copy link
Copy Markdown
Member

Should be squashed once the time to merge comes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/analyses/tutorials/simplifiedUnitAnalysis.ml Outdated
@sim642
sim642 self-requested a review May 25, 2026 08:27
@sim642 sim642 added the cleanup Refactoring, clean-up label May 25, 2026
Copilot AI review requested due to automatic review settings July 14, 2026 06:50
@michael-schwarz

Copy link
Copy Markdown
Member

If there's anything blocking this, let me know @sim642!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@michael-schwarz

Copy link
Copy Markdown
Member

Closed by mistake

@sim642

sim642 commented Jul 14, 2026

Copy link
Copy Markdown
Member

I'll try to have a closer look at this next week.

I quickly noticed one thing: this changes a bunch of unused arguments to _ instead. I'm a bit skeptical about that because it doesn't make anything clearer, especially to newcomers who might wonder why a function takes three unused arguments or what they might be. I think OCaml LSP wouldn't even be able to show their types on hover.

Ideally, some of the transfer function arguments might be better with labels to make their order less sensitive, but now is probably not the best time to try such change anyway. Especially because this might introduce unnecessary and annoying conflicts with various architectural additions (e.g. backward constraints and forward-propagating constraints changes).

@michael-schwarz

Copy link
Copy Markdown
Member

@copilot: don't change unused arguments to _ here.

Copilot AI requested review from Copilot and removed request for Copilot July 15, 2026 07:07

Copilot AI commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

@copilot: don't change unused arguments to _ here.

Fixed in the latest commit: all _ standalone argument names have been replaced with descriptive names (man, state/caller_state, lval, f, args, callee_local, etc.) across simplifiedUnitAnalysis.ml, constants.ml, taint.ml, signs.ml, and all solution files. The ignore calls that were added as workarounds have also been removed.

Copilot AI requested a review from michael-schwarz July 15, 2026 07:07
@sim642 sim642 added this to the v2.9.0 milestone Jul 22, 2026
Comment thread src/analyses/tutorials/signs.ml
Comment thread src/analyses/tutorials/constants.ml Outdated
Comment thread src/analyses/tutorials/constants.ml Outdated
Comment thread src/analyses/tutorials/taint.ml
Comment thread src/analyses/tutorials/taint.ml Outdated
Comment thread src/analyses/tutorials/simplifiedUnitAnalysis.ml Outdated
@michael-schwarz
michael-schwarz requested a review from sim642 July 31, 2026 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Refactoring, clean-up

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port tutorial analyses over to simplified spec

4 participants