Skip to content

Add NMODL support for multiple INITIAL blocks - #3589

Merged
JCGoran merged 4 commits into
masterfrom
jelic/fix_multiple_initial_blocks
Sep 4, 2025
Merged

Add NMODL support for multiple INITIAL blocks#3589
JCGoran merged 4 commits into
masterfrom
jelic/fix_multiple_initial_blocks

Conversation

@JCGoran

@JCGoran JCGoran commented Sep 2, 2025

Copy link
Copy Markdown
Collaborator

Fixes #3588.

The implementation of MergeInitialBlocksVisitor basically does the following:

  • check if there is > 1 INITIAL block in the AST. If not, do nothing
  • add the found INITIAL blocks to a container, blocks_to_delete
  • iterate over all of the statements in each INITIAL block found (in the order of their appearance in the mod file), and add them to a container, statements
  • add an additional INITIAL block to the top-level AST, and dump statements in it
  • delete blocks_to_delete from the AST
  • if passes --nmodl-ast is enabled, write a file <modfile>.<number>.merge_initial_block.mod

Some remarks (for future reference):

  • collect_nodes doesn't actually collect Nodes, but rather (a vector of) instances of its parent class, Ast. This means that we can't directly pass it to Program's erase_node. As a workaround, one can use the following idiom (where is_<some_type> is one of the methods here):
auto& blocks = program.get_blocks();
for (const auto& block: blocks) {
   if (block->is_<some_type>()) {
     // logic goes here
  }
}
  • the Ast class doesn't appear to implement operator== so we can't compare values at the AST level, but only at the level of NMODL

@github-actions

github-actions Bot commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

✔️ b09e48a -> artifacts URL

@github-actions

github-actions Bot commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

✔️ 1a91380 -> artifacts URL

@azure-pipelines

Copy link
Copy Markdown

✔️ b09e48a -> Azure artifacts URL

@codecov

codecov Bot commented Sep 2, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.47%. Comparing base (823fb26) to head (83e5638).
⚠️ Report is 42 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3589      +/-   ##
==========================================
+ Coverage   68.46%   68.47%   +0.01%     
==========================================
  Files         685      686       +1     
  Lines      116721   116754      +33     
==========================================
+ Hits        79914    79949      +35     
+ Misses      36807    36805       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JCGoran
JCGoran marked this pull request as ready for review September 2, 2025 17:16
Comment thread src/nmodl/visitors/initial_block_visitor.cpp Outdated
@sonarqubecloud

sonarqubecloud Bot commented Sep 4, 2025

Copy link
Copy Markdown

@azure-pipelines

Copy link
Copy Markdown

✔️ 83e5638 -> Azure artifacts URL

@github-actions

github-actions Bot commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

✔️ 83e5638 -> artifacts URL

@JCGoran
JCGoran merged commit 71cb817 into master Sep 4, 2025
43 checks passed
@JCGoran
JCGoran deleted the jelic/fix_multiple_initial_blocks branch September 4, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NMODL does not support multiple INITIAL blocks in mod files

2 participants