-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MATLAB switch statement support #634
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Joseph, I think what you've done so far looks really good, but I wanted to ask you to address a couple of minor things.
- The source_refs field appears to be None for some of the CAST pieces. The one I’ve noticed it most frequently in is the ModelIf , there might be other ones. Generally the Gromet generation chokes when it sees None for source_refs as it expects something to be there. At worst you could put some kind of dummy SourceRef if you can’t acquire a real one.
- The out_cast object that is returned in line 64 of skema/program_analysis/matlab2cast.py is wrapped around an additional Python List that it shouldn’t be wrapped around. This makes the actual outputting of the CAST to a file crash. The simplest way to fix that would be to add an additional indexing in line 64 so that you only grab the first element, but it’s worth double checking that there isn’t any additional problems going on.
|
||
def get_model_if(case_node, identifier): | ||
""" return conditional logic representing the case """ | ||
return ModelIf( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, you should add the source_ref field that is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
|
||
def conditional(conditional_node): | ||
""" return a ModelIf struct for the conditional logic node. """ | ||
ret = ModelIf() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll probably also need to add the source_ref that's missing here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FIxed
@@ -78,6 +82,7 @@ def generate_cast(self) -> List[CAST]: | |||
"""Interface for generating CAST.""" | |||
|
|||
# remove comments from tree before processing | |||
|
|||
modules = self.run(remove_comments(self.tree.root_node)) | |||
return [CAST([module], "matlab") for module in modules] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to guess that this line would be a good place to start for resolving the extra list being wrapped around the CAST object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is based on the Fortran TS2CAST which can return multiple CAST modules for a single file. This is then taken into account in multi_file_ingester.py to handle creating Gromet for each module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible I would like to shift this fix to the next PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
## MATLAB switch statement support This PR adds Tree-sitter coverage of MATLAB switch statements. CI tests are also included. ``` matlab switch x case 1 n = 1; j = 1; case {3, three, 'three'} n = 3; case {{1, 2, 3,}, {7, 8, 9}} n = 6; otherwise n = 0; end ``` ## Relevant features - Switch statements are translated into CAST conditional logic - Multiple argument cases are reduced to single list inclusion test - Case arguments may be of differing datatypes (string, number, identifier) ## Related issues - Resolves issue #561 --------- Co-authored-by: Joseph Astier <[email protected]> 391786e
MATLAB switch statement support
This PR adds Tree-sitter coverage of MATLAB switch statements. CI tests are also included.
Relevant features
Related issues