(closes #3392) Store commonblock name inside its interface - #3543
(closes #3392) Store commonblock name inside its interface#3543sergisiso wants to merge 1 commit into
Conversation
|
@arporter @LonelyCat124 This is ready for review, it is the ground work to then address #2554 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3543 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 397 397
Lines 55577 55580 +3
=========================================
+ Hits 55577 55580 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
LonelyCat124
left a comment
LonelyCat124
left a comment
There was a problem hiding this comment.
@sergisiso This mostly makes sense and seems reasonable. I've asked for a couple of things, one is to just test behaviour regarding comments around/in common blocks declaration to check it doesn't also break since it uses hard coded indices.
| # now have a common-block interface. | ||
| try: | ||
| # Loop over every COMMON block defined in this Common_Stmt | ||
| for cb_object in node.children[0]: |
There was a problem hiding this comment.
Since we've been having regular issues when comments appear on/before blocks we've not tested, can you have a go and see what happens if you have comemnts inline or immediately preceding a codeblock with --keep-comments? Am just increasingly wary of our hard coded node.children[0] code.
There was a problem hiding this comment.
Also applies to cb_object[0] - probably need to double check in fparser that its always [0].
| @@ -2904,10 +2904,9 @@ def _process_data_statements(nodes, psyir_parent): | |||
|
|
|||
| @staticmethod | |||
| def _process_common_blocks(nodes, psyir_parent): | |||
There was a problem hiding this comment.
Can you add type hinting to this since it's being modified.
| '''Test we can create an CommonBlockInterface instance and check its | ||
| __str__ value | ||
| '''Test we can create an CommonBlockInterface instance and | ||
| __str__, __eq__, copy, and get its name. |
There was a problem hiding this comment.
"and tests str, eq, copy and can get its name"?
2 participants
This felt as a very natural way to implement the CommonBlocks, I don't know why it wasn't done like this in the first place.