Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Streamline getDocumentationTryGhc #2539

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

Anton-Latukha
Copy link
Collaborator

@Anton-Latukha Anton-Latukha commented Dec 25, 2021

The main agenda there is to provide argument documentation in a form that is neat for further processing. As to provide the documentation as maps from the name to documentation. Which would allow to properly shipping the function argument show work.

During that, improving on getDocumentation{,s}TryGhc code paths:

  • Introducing initTypecheckEnv to abstract type checking environment initialization between several modes. (It abstracts code that is currently in mainline, further improvements there seem to be possible but they are not the scope of this PR, but are considered future changes).

  • Introducing the getDocsNonInteractive', a function that was used/requested in the source code but not abstracted - a function that is analog to GHCs getDocs but modifies it to work in the non-interactive form. Gets used as an abstraction to open module interfaces to retrieve documentation.

  • Introducing getDocsNonInteractive - a function to get the documentation about a Name from a module interface.

  • Updating getDocsBatch - to share abstractions with getDocsNonInteractive. These updates also make available possibilities of further optimizations as in getDocsBatch for bulk processing #2371.

  • getDocumentationTryGhc (single):

    • as being the current main code path, now uses equivalent process (getDocsNonInteractive), and as so - no longer depends on getDocumentationsTryGhc (plural).
    • because no longer uses getDocumentationsTryGhc (plural) - getDocumentationTryGhc (single) no longer needs to pretend element is a singleton to send it to getDocumentationsTryGhc (plural) to retrieve a head
  • getDocumentationsTryGhc (plural):

    • No longer used. The current patch just shows that this code path is not used in reality, actively doing that also allows working on the batch retrieval if it is found beneficial.

    • Instead of IO [SpanDoc] (& matching Names on [] indexes) - returns proper Map Name SpanDoc type.

    • There was use of Lazy Map in the code, which seemed to imply use/process all elements, considered it is enough obvious to ensure those are Strict Maps, that is a minor thing and can roll it back upon request.

  • This PR is a part of the effort to provision argument documentation into UI. All mentioned above functions are used to pass the argument documentation further into the code. GHC 9.2 started to ship arg docs in the form of the IntMap, which this PR also migrated to. Current changes to functions are seen as an improvement already & further PRs would work on argument documentation provisioning further.

@Anton-Latukha Anton-Latukha changed the title Streamline getDocumentationTryGhc by creating getDocsNonInteractive Streamline getDocumentationTryGhc by creating abstractions Dec 25, 2021
@Anton-Latukha Anton-Latukha force-pushed the 2021-12-25-getDocsTryGhc branch 2 times, most recently from cff7a10 to 4b91cdf Compare December 25, 2021 18:16
This function was "inspired" from GHC code of `getDocs`.

Since `getDocsBatch` is not really used for batch - only for singleton elements,
lets make 1 element processing function & use it.
`getDocsBatch` cuurently (& before) used only for single name retrieval
function. Use of it is in `Documentation` module `getDocumentationTryGhc` where
it wraps arg into singleton & gives to `getDocsBatch` & then recieves a Map with
1 entry & unsafely "lookups" doc in it.

This work is to supply the proper single name retrieval-optimized version to
stop that `getDocsBatch` there.

& further ideally `getDocumentationTryGhc` uses single-retrieval &
`getDocumentationsTryGhc` uses a batch mode & batch mode gets optimized along
the lines of: haskell#2371
@Anton-Latukha Anton-Latukha force-pushed the 2021-12-25-getDocsTryGhc branch 2 times, most recently from a7a00ac to cda18cc Compare February 6, 2022 17:07
@Anton-Latukha Anton-Latukha marked this pull request as ready for review February 6, 2022 18:42
@Anton-Latukha Anton-Latukha changed the title Streamline getDocumentationTryGhc by creating abstractions Streamline getDocumentationTryGhc Feb 6, 2022
@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Feb 10, 2022

Overall, I'd liked to keep explicit return types of getDocumentation{,s}TryGhc of IO (Either GHC.ErrorMessages (Name, Either GetDocsFailure (Maybe HsDocString, Maybe (IntMap HsDocString)))) - it puts those complex return procedure explicitly. Before this, the errors were thrown into a monad & was required to be/was caught immediately up the stack, & during GHC API change of ErrorMessages - I do not know how I'd figured-out it. For now it became Either & allowed me to do safety guaranteed changes working with the code.

Having exceptions explicitly in the types also allowed to figure out how to do rebasing onto GHC 9.2 changes & the API changes of GHC.

Having Map Name (Maybe HsDocString, Maybe (IntMap HsDocString) also would be helpful for further work on arg docs. Before this code pretended to have argdocs always, stubbing the Nothing case, which not helped with thinking how to parse & provision them. First I need to know precisely what IntMap holds in which cases & precisely when Nothing is returned. Having explicit types helps to guide and align the code.

After current changes are figured-out, code may return to trow/catching the exceptions, but I'd highly preferred to have Map Name (Maybe HsDocString, Maybe (IntMap HsDocString) or maybe HashMap Name (Maybe HsDocString, Maybe (IntMap HsDocString).

@pepeiborra pepeiborra added the status: unfinished Status for PRs that have been semi-abandoned label Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: unfinished Status for PRs that have been semi-abandoned
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants