Skip to content

Remove implicit save from tests #1202

Description

@jalvesz

Description

I was testing to build stdlib with Lfortran 0.64 the library build but it fails on the tests build process for several reasons among which the fact that Lfortran is very strict with implicit save and also that it doesn't support the exit fork. I think those implicit saves usages should be cleaned.

Draft PR on my fork: jalvesz#14

Cc @certik @jvdp1 @perazz

Here CoPilot's recommendations:

The failing LFortran job is primarily blocked by Fortran constructs that LFortran does not accept in this workflow, not by a runtime test failure.

Fix 1: remove call exit(...) from subprocess code

LFortran flags exit as non-standard in src/system/stdlib_system_subprocess.F90 at lines 256 and 259:

call exit(command_state)
call exit(exit_state)

This is inside the asynchronous fork path. Since the LFortran CI is compiling the whole project, this is enough to poison the build.

Suggested change

Replace the nonstandard calls with standard error stop / stop, or better, avoid compiling this path in the LFortran job.

Minimal code change:

if (asynchronous) then
    if (command_state /= 0) then
        error stop
    else
        stop
    end if
end if

But this loses the exit code.

A cleaner project-level solution is to disable subprocess support for LFortran in the CI or guard this implementation behind compiler conditionals, because this code is relying on fork/exit semantics that LFortran does not handle well yet.


Fix 2: avoid initialized local declarations inside procedures in tests

Several errors come from declarations like these:

LFortran reports:

  • “Assuming implicit save attribute for variable declaration”
  • “add explicit save attribute or parameter attribute or initialize in a separate statement”
  • and for one case, a semantic error around array initialization.

Root cause

The tests declare local variables with initialization in procedure scope, e.g.:

logical(int8)  :: log1(64) = .true.
integer(int64) :: B(2) = [integer(int64) :: 1, 2]
${arraytype}$ :: ... , one = 1

LFortran is stricter here. These should be split into declaration + executable assignment.

Suggested code changes

In test/bitsets/test_stdlib_bitset_large.f90

Change:

logical(int8)  :: log1(64) = .true.
logical(int16) :: log2(31) = .false.
logical(int32) :: log3(15) = .true.
logical(int64) :: log4(33) = .false.
logical(int8)  :: log11(66) = .true.
logical(int16) :: log12(99) = .false.
logical(int32) :: log13(132) = .true.
logical(int64) :: log14(165) = .false.

to:

logical(int8)  :: log1(64)
logical(int16) :: log2(31)
logical(int32) :: log3(15)
logical(int64) :: log4(33)
logical(int8)  :: log11(66)
logical(int16) :: log12(99)
logical(int32) :: log13(132)
logical(int64) :: log14(165)

log1  = .true.
log2  = .false.
log3  = .true.
log4  = .false.
log11 = .true.
log12 = .false.
log13 = .true.
log14 = .false.

Also in test_assignment_array, change:

logical(int8)  :: log1(64) = .true.

to:

logical(int8) :: log1(64)
log1 = .true.
In test/math/test_stdlib_math.fypp

Change declarations like:

${t1}$ :: x(6) = [${t1}$ :: 0, 5, 15, 30, 50, 75]
${t1}$ :: A(1, 3) = reshape([${t1}$ :: 1, 3, 5], [1, 3])
${t1}$ :: B(2) = [${t1}$ :: 1, 2]

to:

${t1}$ :: x(6)
${t1}$ :: A(1, 3)
${t1}$ :: B(2)

x = [${t1}$ :: 0, 5, 15, 30, 50, 75]
A = reshape([${t1}$ :: 1, 3, 5], [1, 3])
B = [${t1}$ :: 1, 2]

This should fix the semantic error seen in generated build/test/math/test_stdlib_math.F90:481.

In test/selection/test_selection.fypp

Change:

${arraytype}$ :: x(N), x_copy(N), mat(Nm), mat_copy(Nm), len1(1), len2(2), &
    kth_smallest, random_vals(Nr), one = 1

to:

${arraytype}$ :: x(N), x_copy(N), mat(Nm), mat_copy(Nm), len1(1), len2(2), &
    kth_smallest, random_vals(Nr), one
one = 1

And similarly:

${arraytype}$ :: x(N), mat(Nm), len1(1), len2(2), random_vals(Nr), one=1

to:

${arraytype}$ :: x(N), mat(Nm), len1(1), len2(2), random_vals(Nr), one
one = 1

Fix 3: keep the LFortran workflow scoped to what it can currently compile

The workflow is here: .github/workflows/ci_lfortran.yml

Right now it attempts a full project build:

- name: Build
  run: cmake --build build --parallel

Given the job banner already says LFortran is “alpha”, the practical fix is to exclude known unsupported targets from this job until the codebase is adjusted.

If you want the quickest path to green CI, combine:

  1. test-source cleanup as above, and
  2. disable subprocess-related targets and/or selected tests for LFortran.

For example, conditionally disable tests/examples that pull in unsupported constructs when using LFortran at CMake level.


Recommended minimal patch set

If your goal is to get this PR green with minimal churn, I’d do this first:

  1. Refactor initialized local variables in tests
    This addresses the hard compilation failures in:

    • test/bitsets/test_stdlib_bitset_large.f90
    • test/math/test_stdlib_math.fypp
    • test/selection/test_selection.fypp
  2. Exclude or guard subprocess code for LFortran
    The call exit(...) usage in src/system/stdlib_system_subprocess.F90 is not LFortran-friendly.

That is the smallest solution directly supported by the failing log lines.

Expected Behaviour

Fixing some of this issues shall get Lfortran to successfully building stdlib

Version of stdlib

Master

Platform and Architecture

Ubuntu

Additional Information

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions