Conversation
…d_types_in_driver
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2706 +/- ##
=======================================
Coverage 99.89% 99.89%
=======================================
Files 359 359
Lines 51073 51114 +41
=======================================
+ Hits 51021 51062 +41
Misses 52 52 ☔ View full report in Codecov by Sentry. |
hiker
added
LFRic
…d_types_in_driver
…d_types_in_driver
…d_types_in_driver
…d_types_in_driver
|
To clarify: this does not support passing derived types to subroutines (upcoming PR for 2830_support_user_defined_type_arguments), it only handles the case that a subroutine somewhere uses a member |
…d_types_in_driver
hiker
added
PSyKE
arporter
left a comment
arporter
left a comment
There was a problem hiding this comment.
Thanks for this Joerg. Functionally it all looks OK but I do have questions about array accesses in structures and structure accesses of greater depths.
Aside from that, I'm not entirely comfortable about the Signature <-> PSyIR interface and think it should perhaps be the other way around. It's not a biggie for this PR though.
I'll launch the integration tests next time (unless you fancy doing it when you arrive at the office in the morning?).
| from psyclone.psyir.nodes import Reference, StructureReference | ||
|
|
||
| if self.is_structure: | ||
| ref = StructureReference.create(symbol, list(self._signature[1:])) |
There was a problem hiding this comment.
What about arrays?
I have to admit that having a Signature method take a Symbol and give back PSyIR feels like the wrong way around. I think StructureReference should perhaps have a new create_from_signature method? That way, we might not need the imports either.
| :py:class:`psyclone.psyir.symbols.Symbol`, | ||
| :py:class:`psyclone.psyir.symbols.Symbol`]] | ||
| :py:class:`psyclone.psyir.symbols.Symbol`, | ||
| :py:class:`psyclone.core.signature.Signature`]] |
There was a problem hiding this comment.
Could this be a Reference instead - that way we use PSyIR consistently?
| datatype=sym.datatype) | ||
| if module_name: | ||
| post_tag = f"{name}{postfix}@{module_name}" | ||
| if module_name and hasattr(sym.datatype, "interface"): |
There was a problem hiding this comment.
A TypedSymbol always has an interface so (hopefully) you don't need the hasattr?
| post_name = f"{flat_name}_{module_name}{postfix}" | ||
| mod_man = ModuleManager.get() | ||
| mod_info = mod_man.get_module_info(module_name) | ||
| datatype = mod_info.get_symbol(sym.datatype.name) |
There was a problem hiding this comment.
A bit picky but for clarity I suggest s/datatype/sym/ or something as it is a symbol, not a datatype.
| if isinstance(datatype, DataTypeSymbol): | ||
| # This is a structure. We need to create a flattened name | ||
| # and find the base type of the member involved | ||
| datatype = datatype.datatype |
There was a problem hiding this comment.
If you had a Reference here, this would just be ref.datatype. That would also allow for array subsections which I don't think you cover here?
| # ------------------------------------------------------------------------- | ||
| @staticmethod | ||
| def _create_output_var_code(name, program, is_input, read_var, | ||
| def _create_output_var_code(signature, program, is_input, read_var, |
There was a problem hiding this comment.
The docstring for the name argument needs to be updated.
| ("routine", None, "unknown_subroutine")] | ||
| ) | ||
| ("routine", None, "unknown_subroutine"), | ||
| ('unknown', 'module_with_var_mod', 'user_var%member_read_write'), |
There was a problem hiding this comment.
Now that I look at this, would this work for arbitrary-depth references, e.g. user_var%member%this_one%read_write?
2 participants
Adds support for the drivers to properly read in derived type (using non-derived Fortran variables based on the actual basic type of the members).