camfort / fortran-src

Fortran parsing and static analysis infrastructure
https://hackage.haskell.org/package/fortran-src
Other
48 stars 20 forks source link

Don't inline solo includes #245

Closed RaoulHC closed 1 year ago

RaoulHC commented 1 year ago

When dealing with solo includes we don't have the context to know where its includes are so we probably shouldn't be inlining them.

Happy for this to be named something else. I'm also unsure if there are cases where we want to inline includes still, but I don't think we'll have the context for our use cases, but if you can think of one maybe it will make sense for us to have a function that inlines them and one that doesn't.

RaoulHC commented 1 year ago

(Will update this in fortran-src-extras once this has gone through)

raehik commented 1 year ago

Thanks. I adapted these definitions from older parsing code and didn't stop to think about naming.

That function essentially just lifts a parser error into IO now. With that in mind, how about we remove f77IncIncludes, and export the makeParserFixed F77.includesParser Fortran77Legacy parser so that users can do the IO wrapping themselves? So it would now be defined:

f77IncludesNoTransform = makeParserFixed F77.includesParser Fortran77Legacy
f fn bs = case f77IncludesNoTransform fn bs of
            Left  e   -> liftIO $ throwIO e
            Right bls -> pure bls
-- also export a util for dragging Either to IO and throwing on Left

Casing inline makes the error handling more apparent and we're exporting more pluggable internals (which follow the parser naming conventions).

I'll push my commit to this branch, please edit/comment away.

RaoulHC commented 1 year ago

Yeah that would work, probably makes sense to put that definition in fortran-src-extras then.

raehik commented 1 year ago

Nice, thanks for this. I'll merge this and bump fortran-src-extras.