Skip to content

Conversation

LKedward
Copy link
Member

As discussed in #155, this uses the error_t type to return and propagate errors from the new source parsing routines.
Adds a test suite for the parsing routines - any difficult parsing edge cases can be added in there as they are found.

There are other regions from #155 that still require test coverage.

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Looks good. I have one minor suggestion for the file_parse_error. Since we have the line number already, it might be more verbose to print the line of the source file as well, maybe, in case we know, even highlight the offending part in some way.

I did something similar for the TOML parsing with a context from the tokenizer to provide some visual aid together with the error message:
https://github.com/toml-f/toml-f/blob/d7371fc8/src/tomlf/error.f90#L215-L243

Allows optional line number, line string value and line column output.
@LKedward
Copy link
Member Author

Thanks @awvwgk 👍

.. it might be more verbose to print the line of the source file as well, maybe, in case we know, even highlight the offending part in some way.

Great idea! That's a neat implementation too!

I've now added more context output, though in most cases indicating the offending column may have to wait until a proper parsing library is used.

end function parse_c_source


function split_n(string,delims,n,stat) result(substring)
Copy link
Member

@certik certik Sep 14, 2020

Choose a reason for hiding this comment

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

We should move these into an fpm_string.f90 module, and eventually get these into stdlib.

But let's first merge this PR, then we can refactor later.

Copy link
Member

@everythingfunctional everythingfunctional left a comment

Choose a reason for hiding this comment

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

This looks great. I know writing tests is often a thankless chore, but it looks like you've really shored this part up with it, so thanks! :)

@milancurcic milancurcic merged commit e6f8785 into fortran-lang:master Sep 15, 2020
@LKedward LKedward deleted the parsing-tests branch September 20, 2020 16:25
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.

5 participants