Skip to content
This repository was archived by the owner on Mar 25, 2025. It is now read-only.

Store all nodes for a specific symbol#934

Merged
pramodk merged 11 commits into
masterfrom
cornu/fix_symbols_nodes
Sep 26, 2022
Merged

Store all nodes for a specific symbol#934
pramodk merged 11 commits into
masterfrom
cornu/fix_symbols_nodes

Conversation

@alkino

@alkino alkino commented Sep 20, 2022

Copy link
Copy Markdown
Member

This way we can support the anti-feature of nrn having a function and a range variable having the same name

This way we can support the anti-feature of nrn having a function and a range variable having the same name

@pramodk pramodk 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.

overall looks fine! as discussed, adding unit test under visitor would be great!

Comment thread src/symtab/symbol.hpp Outdated
@bbpbuildbot

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #74978 (:no_entry:) have been uploaded here!

Status and direct links:

@bbpbuildbot

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #75027 (:white_check_mark:) have been uploaded here!

Status and direct links:

@bbpbuildbot

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #75240 (:white_check_mark:) have been uploaded here!

Status and direct links:

@bbpbuildbot

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #75385 (:white_check_mark:) have been uploaded here!

Status and direct links:

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #934 (e289815) into master (ea1acbd) will increase coverage by 0.10%.
The diff coverage is 97.11%.

@@            Coverage Diff             @@
##           master     #934      +/-   ##
==========================================
+ Coverage   62.22%   62.32%   +0.10%     
==========================================
  Files         194      194              
  Lines       28341    28402      +61     
==========================================
+ Hits        17634    17701      +67     
+ Misses      10707    10701       -6     
Impacted Files Coverage Δ
src/symtab/symbol_table.cpp 72.03% <75.00%> (+0.11%) ⬆️
src/language/templates/pybind/pysymtab.cpp 95.91% <100.00%> (ø)
src/symtab/symbol.cpp 94.44% <100.00%> (+4.44%) ⬆️
src/symtab/symbol.hpp 91.47% <100.00%> (+2.21%) ⬆️
src/visitors/inline_visitor.cpp 92.15% <100.00%> (-0.20%) ⬇️
src/visitors/solve_block_visitor.cpp 97.50% <100.00%> (+0.06%) ⬆️
src/visitors/symtab_visitor_helper.hpp 95.41% <100.00%> (ø)
test/unit/symtab/symbol_table.cpp 100.00% <100.00%> (ø)
src/language/templates/ast/ast.cpp 43.88% <0.00%> (+0.05%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ohm314 ohm314 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.

Generally looks good, but maybe a few minor details to fix.

Comment thread src/symtab/symbol.hpp Outdated
Comment thread src/language/templates/pybind/pysymtab.cpp
Comment thread src/visitors/inline_visitor.cpp
Comment thread src/visitors/solve_block_visitor.cpp Outdated

@pramodk pramodk 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.

few minor nitpicks otherwise good to go! 🚀

Comment thread src/symtab/symbol.hpp Outdated
Comment thread src/symtab/symbol_table.cpp Outdated

@pramodk pramodk 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.

(should have approved it with the previous review)

@bbpbuildbot

Copy link
Copy Markdown
Collaborator

Logfiles from GitLab pipeline #75492 (:white_check_mark:) have been uploaded here!

Status and direct links:

@pramodk pramodk merged commit 68e7086 into master Sep 26, 2022
@pramodk pramodk deleted the cornu/fix_symbols_nodes branch September 26, 2022 11:25
JCGoran pushed a commit to neuronsimulator/nrn that referenced this pull request Mar 12, 2025
…/nmodl#934)

* Until now only first AST node for a given symbol was stored in the AST
* Because of this, it was not possible to find/lookup all nodes associated
  with a given symbol
* This PR now changes the symbol implementation to store all encountered
  AST nodes.
* Add tests for symbol class

NMODL Repo SHA: BlueBrain/nmodl@68e7086
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

5 participants