Preprocessor defines overrides fortran keyword resulting in scope errors

[gbogopolsky]

Hi,

First thank you for taking up this project and your numerous fixes!

I recently noticed a bug when a single TYPE IS is called in a SELECT TYPE block when an identifier is associated to the selector. The following error is raised in vscode: Invalid parent for "TYPE" declaration.

I am able to reproduce this behavior by removing lines 23 to 26 included in test/test_source/subdir/test_select.f90, yielding:

SUBROUTINE test_select_sub(self)
CLASS(parent), INTENT(inout) :: self
! Select statement with binding
SELECT TYPE(this=>self)
TYPE IS(child1)
  this%a
END SELECT
! Select statement without binding
SELECT TYPE(self)
TYPE IS(child1)
  self%a
END SELECT
END SUBROUTINE test_select_sub

This causes the test/test_server.py::test_comp to fail with TypeError: object of type 'NoneType' has no len().

To my knowledge, such a structure is valid (at least it compiles and works as expected on the Intel classic compiler).

[gnikit]

Hi @gbogopolsky good catch. This is indeed a bug. As you can tell I don't tend to work with type is select constructs and hence this fell through the cracks.

I will fix it ASAP (hopefully it's trivial to fix)

[gnikit]

So I had a look, the error you are getting from pytest is because of the unittest breaks if you remove lines 23-26

if you have a look at the test_comp function you will see this

    file_path = test_dir / "subdir" / "test_select.f90"
    string += comp_request(file_path, 21, 7)
    string += comp_request(file_path, 23, 7)   # <- line 24 in the file and column 8
    string += comp_request(file_path, 25, 7)   # <- line 26 in the file and column 8
    string += comp_request(file_path, 30, 7)

If you remove lines 23-26 the unittest will break because the completion request will return None. That is not to say that there no bug present. I will have a look.

[gnikit]

Okay I had a look and it seems okay to me. You can see that the single type is is actually tested in the same file, lines 26-32.

If you try and run a completion request from test/test_source/test_select.f90 on the modified version of the file as you posted

fortls.py --debug_filepath subdir/test_select.f90 --debug_rootpath . --debug_completion --debug_line 22 --debug_char 8

you should get an output similar to this (I have more Source directories due to unreleased features I am testing).

Testing "initialize" request:
  Root = "."
[INFO - 16:40:58] fortls - Fortran Language Server 2.2.5.dev0+gabd7f39.d20220306 Initialized
  Successful!

  Source directories:
    /home/gn/Code/Python/fortls/test/test_source/docs
    /home/gn/Code/Python/fortls/test/test_source/debug/.vscode
    /home/gn/Code/Python/fortls/test/test_source/subdir
    /home/gn/Code/Python/fortls/test/test_source/pp
    /home/gn/Code/Python/fortls/test/test_source/hover
    /home/gn/Code/Python/fortls/test/test_source/pp/include
    /home/gn/Code/Python/fortls/test/test_source/debug
    /home/gn/Code/Python/fortls/test/test_source/include
    /home/gn/Code/Python/fortls/test/test_source/completion
    /home/gn/Code/Python/fortls/test/test_source
    /home/gn/Code/Python/fortls/test/test_source/signature
    /home/gn/Code/Python/fortls/test/test_source/.vscode
    /home/gn/Code/Python/fortls/test/test_source/diag

Testing "textDocument/completion" request:
  File = "subdir/test_select.f90"
  Line = 22
  Char = 8

  Results:
    6: a -> REAL(8)
    6: n -> INTEGER(4)

Can you post the code that generate this?

The following error is raised in vscode: Invalid parent for "TYPE" declaration

[gbogopolsky]

OK, thanks for a investigation, it seems I was a bit to hasty in my understanding of the tests. And indeed, I do get the same output with your command.

I managed to reproduce my issue in VSCode using the code from test_select.f90, but it is very strange: it seems to depends on whether the type is statement is uppercase or lowercase.

In this example, the second block does not raise any error but the first and the third one do (cf. screenshot)

What do you think? Is it a faulty regex? Or is this not linked with fortls but its implementation?

[gnikit]

So I do think that the error is from fortls but I am unable to replicate myself. Here is my screenshot

The diagnostic error at this%a is from the compiler not fortls.

To get to the bottom of this can you answer the following please.

Firstly post the output of this command

fortls --debug_filepath test_selected.f90 --debug_rootpath . --debug_diagnostics

then

1. What settings are you using for fortls?
2. What Fortran extension are you using in VS Code?
3. What are your VS Code extension settings i.e. settings.json (post the ones relative to Fortran)

[gbogopolsky]

Alright, so in order:

1. Here are the settings in the .fortls config file at my project root (I manually added the PETSc definition, I did not manage to get the automatic include working, especially for the types which require recursive define, which is not supported):

{
    "pp_suffixes": [".f90"],
    "include_dirs": [],
    "pp_defs": {
        "PetscInt": "integer",
        "PetscScalar": "real(pr)",
        "PetscBool": "logical(kind=4)",
        "Mat": "type(tMat)",
        "Vec": "type(tVec)",
        "KSP": "type(tKSP)",
        "PC": "type(tPC)",
        "VecScatter": "type(tVecScatter)",
        "IS": "type(tIS)",
        "PetscLogStage": "integer",
        "PetscErrorCode": "integer"
    }
}

2. I suppose you are asking about Fortran extensions. For this, I am using Modern Fortran v.3.0.2022021000 to be able to use this fortls (deactivating most of my other extensions did not change the reported behavior).
3. Fortran-related options in settings.json:

{
    "fortran-ls.hoverSignature": true,
    "fortran-ls.autocompletePrefix": true,
    "fortran-ls.variableHover": true,
    "fortran-ls.lowercaseIntrinsics": true,
    "fortran-ls.executablePath": "[correctPathToFortls]",
    "fortran.linter.includePaths": [
        "/usr/local/opt/petsc/include"
    ],
    "fortran.linter.compiler": "Disabled"
}

I hope this helps you.

[gnikit]

1. Your fortls settings look good to me. If you want to autoinclde PP definitions (or at least allow fortls to try you have to specify include_dirs e.g. "include_dirs": ["include"], will pass through the preprocessor parser all files encountered matching uppercase Fortran file extensions + ".f90" as per your .fortls file)
2. Modern Fortran v3.0+ is correct
3. You can delete the "fortran-ls" settings and uninstall FORTRAN Intellisense. What that extension did was try and interface with the Fortran Language Server. Since version (pre-release) 3.0+ of Modern Fortran this can be done natively. The default options set to fortls through Modern Fortran are sensible so you should not need anything else. Probably if you are using a custom path to fortls you need to set "fortran.fortls.path":

[gnikit]

And just to check, you are using fortls v 2.2.5?

[gbogopolsky]

1. I had already tried that: putting "include_dirs": ["/usr/local/opt/petsc/include"] does not seem to work with the fortran include of PETSc #include<petsc/finclude/petscksp.h> and the recursive include of other files. Also the PETSc includes have recursive defines for PetscInt for example, which does not work AFAIK. Defining them by hand also allows it to work when I am using the remote SSH server with different include paths.
2. Yep, it was already uninstalled, just remaining options in my settings, my bad. Kudos to you for the default and for simplifying the fortran setup, it was a bit convoluted with the two extensions before.
3. Yep, I am using v. 2.2.5 as of this morning ;)

Still, it's strange that you do not have this problem... I will check on my Windows machine, if it changes something, I'll get back to you.

[gbogopolsky]

Same behavior is observed on Windows with fortls v.2.5.5 through conda-forge

[gnikit]

Same behavior is observed on Windows with fortls v.2.5.5 through conda-forge

To be honest, I wouldn't expect conda-forge to be any different.

As a last attempt could you copy-paste the entire file that is giving you the diagnostic error invalid parent of TYPE and I will attempt to replicate locally

I figured it out. You define a preprocessor variable IS which fortls substitutes to the PETSc variable. This results in your type is construct being erroneously substitute in fortls. This is also the reason why the letter case matters, because preprocessor definitions are case sensitive.

[gbogopolsky]

Damn, that's so evident now that you say it...
This means that there can be problem with manual defines and auto includes if preprocessor definitions are matching language intrinsic/keywords. The preprocessor seems fine. Maybe making sure that fortls does not apply preprocessor definition to language keywords? I don't really know if it is possible.

For now, I will remove the IS preprocessor and directly use type(tIS) in my code.

Anyway, thank you very much for your help and your availability. I think I would be interested to help you with the code on this project, but I do not have the time currently thanks to my PhD. But I'll keep it in mind :)

[gnikit]

Yeah preprocessor definitions are a bit flimsy because they are implemented as string substitutions. That means that every instance of the word IS is replaced with type(tis). That is why we can't distinguish between a select construct and a variable named select. This is also the reason why our syntax highlighting (in vscode) is slow sometimes because it has to run through the code multiple times to understand if the word function is the start of a function or a variable named function.

All of this would be so much simpler if Fortran disallowed the use of reserved keywords as variables...

I think the correct way to handle this would be instead of using string substitutions to register preprocessor statements as Fortran variables/functions/subroutines.

However, before I do that I want to increase the code coverage of the preprocessor since the original implementation is the definition of spaghetti code.

I would leave this issue open since it is technically a bug

[gbogopolsky]

Good luck for the preprocessor and thanks for the title update, it was indeed out of date.