Get the wrong variable after module use statement

[Zek-Li]

This is a runtime error of ATM of CESM, and I reduced the case like below:

module public1
  ! This should be the value we get after calling foo() in program use_1_2
  integer, public :: var = 1
  integer, public :: var1 = 11 ! Removing this statement fixes the issue
end

module public2
  integer, public :: var = 2
end

module use_public2
  use public2, only : var
  private ! Removing this fixes the issue
contains
end

program use_1_2
  use public1, only : var1
  use use_public2

  integer :: ret = 0

  ret = foo()
  ! ret shoule be 1
  print *, ret
contains
  function foo() result(ret)
    use public1, only : var

    integer :: ret

    ret = var
  end
end

[Zek-Li]

I investigated the previous commit and found that the problem was caused by 56e924c5b5bd5823dd383b72e5e1416492d49d17.

This line adds an alias of var defined in public2 to the function foo, that leads to the inability to correctly distinguish which var should be used in the function foo.

[Zek-Li]

Hi @michalpasztamobica, do you have any idea to solve this problem?

[kiranchandramohan]

Thanks @Zek-Li for raising the issue. @michalpasztamobica has moved to work on another project. If reverting https://github.com/flang-compiler/flang/commit/56e924c5b5bd5823dd383b72e5e1416492d49d17 fixes the issue then please go ahead and revert. BTW, is this from @bryanpkc's team?

[bryanpkc]

BTW, is this from @bryanpkc's team?

Yes. We are working on a fix and will submit a PR soon.

[michalpasztamobica]

Hi, sorry for low responsiveness. Indeed, I am busy doing something else at the moment. If a fix without revert is possible, that would be better. This commit was proven to fix an issue: https://github.com/flang-compiler/flang/pull/949

[xinliu-hnc]

Hi @Zek-Li , I've submitted a PR #1149 to solve this problem. The above small test case passed. Could you help to check whether this patch fixes the error of ATM of CESM?

[Zek-Li]

Good job! This patch passed the case of CESM. I'm still confused at this line. I think global = SST_SYMG(RHS(3)); may not be the symbol we want, do we need to create the alias? In fact, everything works fine if I delete this line.

[zhaochuanfeng]

I also agree @Zek-Li that this line is useless and need to be removed, what do you think, please? @michalpasztamobica @xinliu-hnc

In this case, when we are processing var in scope foo, global is an alias symbol in scope use_public2, it aliases with var in scope public2. If we create a new global symbol and set it alias with global, it is not correct, because the var in scope foo is an alias of the var in scope public1.

[michalpasztamobica]

Hi @zhaochuanfeng , @Zek-Li , I think this is a risky move In line 313 we make sure that we only enter the section if STYPEG(global) == ST_ALIAS. Then we kind of copy global to newglobal, but the STYPE remains unset (I also checked that insert_sym does not set the STYPE value). So the question is: is it OK for the newglobal variable to have the STYPE unset (or rather set to 0 = ST_UNKNOWN)? Even if we don't see any tests failing without this line, I would leave it intact. It logically makes sense to me and we might get a bug report in a few months which, after weeks of searching, will make us bring this line back - we have seen this before...

[xinliu-hnc]

IMO the function add_use_rename has insufficient code coverage, and when I tried to add test cases to cover it, some of them failed.

(1)

! case-1
module m1
  integer :: var1 = 1
end module

module m2
  use m1, var11 => var1
  private var11
  integer :: var2 = 2
end module

program p
  use m2
  call subr()
contains
  subroutine subr()
    use m2, var11 => var2 ! fail here
    if (var11 == 2) print *, "PASS"
  end subroutine
end program

flang throw error

F90-S-0084-Illegal use of symbol var1 - not public entity of module (case-1.f90: 17)

(2)

! case-2
module m
  implicit none
  interface
    integer function foo(x, y)
      integer, intent(in) :: x, y
    end function
  end interface
  interface operator(.opr.)
    procedure :: foo
  end interface
  integer :: opr = 2
end module m

program p
  use m, operator(.localopr.) => operator(.opr.)
  implicit none
  if (opr ==2) print *, "PASS" 
end program

flang throw error

F90-S-0038-Symbol, opr, has not been explicitly declared (case-2.f90)

I'm still trying to figure out the logic of this part of the code and wondering why newlocal or newglobal is created in add_use_rename instead of apply_use as in other cases.

[xinliu-hnc]

@Zek-Li @zhaochuanfeng @michalpasztamobica The PR #1149 has been updated to fix this issue and the two test cases mentioned in the comment above. Please have a look.

[zhaochuanfeng]

@xinliu-hnc Great! Good to see your patch, thanks!

[pawosm-arm]

Shouldn't it be closed since #1149 was merged?

[kiranchandramohan]

@pawosm-arm if the issue is fixed then please close.

[pawosm-arm]

I'd have to build BigDFT myself if I was the one to close it. I'll try later this week.

[pawosm-arm]

Ok, so I've tried to build BigDFT (version 1.9.0, as before) and the commit that fixes current issue does not break the fix for the issue #889 (which caused the regression signaled in the current issue). Therefore we can assume this bug report can be closed. Sadly, BigDFT still does not build due to other (still opened) issues.