Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Erroneous syntax highlighting with subroutine names and preprocessor conditionals #278

Closed
gnikit opened this issue Nov 12, 2021 · 1 comment · Fixed by #509
Closed

Erroneous syntax highlighting with subroutine names and preprocessor conditionals #278

gnikit opened this issue Nov 12, 2021 · 1 comment · Fixed by #509
Assignees
Milestone

Comments

@gnikit
Copy link
Member

gnikit commented Nov 12, 2021

Describe the bug
end subroutine surrounded by preprocessor conditionals is erroneous

Screenshots
image

image

Code sample

  function blasmul_mm(A, B) result(C)
    !!< Use DGEMM to multiply A * B and get C.

    real, dimension(:, :), intent(in) :: A, B
    real, dimension(size(A, 1), size(B, 2)) :: C

    interface
#ifdef DOUBLEP
      SUBROUTINE DGEMM ( TRANSA, TRANSB, M, N, K, ALPHA, A, LDA, B, LDB, &
                         BETA, C, LDC )
#else
      SUBROUTINE SGEMM ( TRANSA, TRANSB, M, N, K, ALPHA, A, LDA, B, LDB, &
                         BETA, C, LDC )
#endif
      CHARACTER*1        TRANSA, TRANSB
      INTEGER            M, N, K, LDA, LDB, LDC
      REAL               ALPHA, BETA
      REAL               A( LDA, * ), B( LDB, * ), C( LDC, * )
#ifdef DOUBLEP
      END SUBROUTINE DGEMM
#else
      END SUBROUTINE SGEMM
#endif
    end interface

    assert(size(A, 2) == size(B, 1))

#ifdef DOUBLEP
    call DGEMM('N', 'N', size(A, 1), size(B, 2), size(A, 2), 1.0, A, size(A, 1), B, size(B, 1), &
               0.0, C, size(A, 1))
#else
    call SGEMM('N', 'N', size(A, 1), size(B, 2), size(A, 2), 1.0, A, size(A, 1), B, size(B, 1), &
               0.0, C, size(A, 1))
#endif 
  end function blasmul_mm

Fortran Form
Free form (F90+)

Build info (please complete the following information):

  • OS: Linux x64
  • Extension Version :2.5.0 internal dev version
  • Visual Studio Code Version: 1.63.0-insider
@gnikit gnikit self-assigned this Nov 12, 2021
@gnikit gnikit added the bug label Nov 12, 2021
@gnikit gnikit added this to the v3.0 milestone Apr 27, 2022
@gnikit
Copy link
Member Author

gnikit commented Jun 3, 2022

This is a much more interesting "bug" than what it seems. Its root is that the regex responsible for opening the scopes matches the end of the match with the name of the subroutine (function, submodule, etc.) so it expects that inner scopes are closed first, followed by outer scopes, e.g.

subroutine scope_1()
  subroutine scope_2()
    subroutine scope_3()
    end subroutine scope_3
  end subroutine scope_2
end subroutine scope_1

However, when preprocessor directives are used the order is upsetted and it no longer makes sense to close the scopes in the same order

subroutine scope_1()
#ifdef DEF
  subroutine scope_2()
#else
  subroutine scope_3()
#endif

#ifdef DEF
  end subroutine scope_2()
#else
  end subroutine scope_3()
#endif
end subroutine scope_1

Now scope 2 and scope 3 are on the same level since they are guarded by a preproc conditional.

Solution

I think it is reasonable to expect our grammar not being responsible for catching coding errors such as using the wrong subroutine,function, etc. when closing a scope. I will edit the scopes to exit upon a C++ variable name (names can be preprocessor macros)

gnikit added a commit that referenced this issue Jun 3, 2022
Previously nested scopes had to be closed in the reverse order than they
were opened e.g.

```f90
subroutine scope_1()
  subroutine scope_2()
    subroutine scope_3()
    end subroutine scope_3
  end subroutine scope_2
end subroutine scope_1
```

However with preprocessor conditionals that would be an issue since

```f90
subroutine scope_1()
#ifdef DEF
  subroutine scope_2()
#else
  subroutine scope_3()
#endif

#ifdef DEF
  end subroutine scope_2()
#else
  end subroutine scope_3()
#endif
end subroutine scope_1
```

`scope_2` and `scope_3`  would be in the same level on the AST, so it is
no longer valid to expect `scope_3` to close before `scope_2`.

The solution to this is removing the strict requirement for scopes to
have matching begin-end names from the regex. The type of scopes
where this requirement is removed are:

- Functions
- Modules
- Programs
- Module Procedures
- Subroutines
- Submodules

Fixes Erroneous syntax highlighting with subroutine names and preprocessor conditionals #278
@gnikit gnikit linked a pull request Jun 3, 2022 that will close this issue
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant