Skip to content

Reset derived data portably with NVFORTRAN - #394

Open
krystophny wants to merge 1 commit into
mainfrom
agent/nvhpc-empty-constructor
Open

Reset derived data portably with NVFORTRAN#394
krystophny wants to merge 1 commit into
mainfrom
agent/nvhpc-empty-constructor

Conversation

@krystophny

Copy link
Copy Markdown
Member

Summary

  • replace two empty derived-type constructors with assignment from a locally
    default-initialized object;
  • preserve deallocation and default component initialization semantics;
  • make the SPECTRE and JOREK cleanup routines compile with NVFORTRAN 26.3.

The prior constructors omit allocatable components that do not have default
initializers. GFortran accepts that form, while NVFORTRAN rejects it with
NVFORTRAN-F-0155. The replacement is compiler-portable and was also verified
with a focused NVFORTRAN allocation/reset probe.

Verification

  • fo check: build 227 modules; all tests passed in 48.4 s
  • fo fmt --check on both changed files: pass
  • fo lint: zero unused imports; existing 472-warning baseline
  • git diff --check: pass

@slopqueue slopqueue Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review verdict: Approve

Summary: PR #394 replaces structure-constructor resets (data = jorek_restart_t() / data = spectre_data_t()) in two free_* routines with a local default-initialized variable (empty) of the same type, to work around an NVFORTRAN portability issue with deallocating allocatable components via structure-constructor assignment.

Findings:

  1. [minor] src/field/jorek_restart.f90:105-107 / src/spectre/spectre_reader.f90:71-73 — The fix is correct and portable. A local type :: empty variable receives default initialization (scalars to defaults, allocatables unallocated), so data = empty triggers Fortran-2003 automatic deallocation of data's allocatable components. This is functionally equivalent to the old structure constructor but avoids the NVFORTRAN bug where data = type_t() may fail to deallocate allocatable members. No behavioral change for gfortran; the existing tests (test_jorek_restart, test_spectre_reader) already assert allocated() is false after free_*, which directly validates the fix.

No other free_* routines or data = type_t() patterns exist in src/, so the fix is complete. Both types have only scalar and allocatable components (no pointers), so the local-variable approach is safe with no leak risk.

Verdict: Approve — minimal, well-targeted portability fix with adequate existing test coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant