ModuleInfo.minf_rdr_env not exposed

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

ModuleInfo.minf_rdr_env not exposed

fommil
Hi all,

Is there a reason why minf_rdr_env (a field in ModuleInfo) is not
exposed?

It's possible to reconstruct it in 8.4.4 (the only version I'm looking
at) with a TypecheckedModule via

  let (tc_gbl_env, _) = GHC.tm_internals_ tmod
      minf_rdr_env = tcg_rdr_env tc_gbl_env


It's a useful thing to have for editor tooling (e.g. to get the correct
qualified imported symbols that may be autocompleted).

--
Best regards,
Sam


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

signature.asc (199 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ModuleInfo.minf_rdr_env not exposed

fommil
Hello,

I would like to submit a patch to ghc 8.8 adding a function that exposes a field that is useful for tooling authors and can already be reached, albeit in a very awkward way.

The process documented at
https://gitlab.haskell.org/ghc/ghc/wikis/working-conventions/adding-features
seems very heavyweight for the patch that I was planning to propose,
which is just adding the following to GHC.hs (and exporting it):

  modInfoRdrEnv :: ModuleInfo -> Maybe GlobalRdrEnv
  modInfoRdrEnv = minf_rdr_env

Without this accessor, we must reparse and typecheck the file to get the GlobalRdrElts, which is very inefficient and uses fields named "internal" so I'm guessing that's not a good thing to build a tool on top of.

My colleague already wrote a tool using a workaround, see https://gitlab.com/tseenshe/hsinspect/blob/503cd48faba5b308be29ed44a12f7d6b22105f2b/exe/Main.hs#L53-61

How should I go about submitting this? Do I need to write a test or is this trivial enough not to bother?

Sam Halliday <[hidden email]> writes:

> Hi all,
>
> Is there a reason why minf_rdr_env (a field in ModuleInfo) is not
> exposed?
>
> It's possible to reconstruct it in 8.4.4 (the only version I'm looking
> at) with a TypecheckedModule via
>
>   let (tc_gbl_env, _) = GHC.tm_internals_ tmod
>       minf_rdr_env = tcg_rdr_env tc_gbl_env
>
>
> It's a useful thing to have for editor tooling (e.g. to get the correct
> qualified imported symbols that may be autocompleted).
>
> --
> Best regards,
> Sam
--
Best regards,
Sam


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

signature.asc (199 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: ModuleInfo.minf_rdr_env not exposed

Artem Pelenitsyn
Hey Sam,

I think the thing you propose hardly qualifies as a new feature (in the sense of page you referenced), so not much of a hassle should be involved.
As long as it is a 3-lines change, I'd say go ahead and create an Issue and an MR on Gitlab: you might be better off with getting feedback there.

--Best, Artem

On Tue, 6 Aug 2019 at 15:52, Sam Halliday <[hidden email]> wrote:
Hello,

I would like to submit a patch to ghc 8.8 adding a function that exposes a field that is useful for tooling authors and can already be reached, albeit in a very awkward way.

The process documented at
https://gitlab.haskell.org/ghc/ghc/wikis/working-conventions/adding-features
seems very heavyweight for the patch that I was planning to propose,
which is just adding the following to GHC.hs (and exporting it):

  modInfoRdrEnv :: ModuleInfo -> Maybe GlobalRdrEnv
  modInfoRdrEnv = minf_rdr_env

Without this accessor, we must reparse and typecheck the file to get the GlobalRdrElts, which is very inefficient and uses fields named "internal" so I'm guessing that's not a good thing to build a tool on top of.

My colleague already wrote a tool using a workaround, see https://gitlab.com/tseenshe/hsinspect/blob/503cd48faba5b308be29ed44a12f7d6b22105f2b/exe/Main.hs#L53-61

How should I go about submitting this? Do I need to write a test or is this trivial enough not to bother?

Sam Halliday <[hidden email]> writes:

> Hi all,
>
> Is there a reason why minf_rdr_env (a field in ModuleInfo) is not
> exposed?
>
> It's possible to reconstruct it in 8.4.4 (the only version I'm looking
> at) with a TypecheckedModule via
>
>   let (tc_gbl_env, _) = GHC.tm_internals_ tmod
>       minf_rdr_env = tcg_rdr_env tc_gbl_env
>
>
> It's a useful thing to have for editor tooling (e.g. to get the correct
> qualified imported symbols that may be autocompleted).
>
> --
> Best regards,
> Sam

--
Best regards,
Sam
_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
Reply | Threaded
Open this post in threaded view
|

Re: ModuleInfo.minf_rdr_env not exposed

fommil
Thanks Artem,

I created a merge request at

  https://gitlab.haskell.org/ghc/ghc/merge_requests/1541

I wanted to target the 8.8 branch but I couldn't find anything except
master and that seems to be 8.9 already. If the reviewers are happy with
this, could they please cherry pick it to wherever it needs to go? I'd
also love to see it in 8.6 if there are any more releases as it would
mean no more workarounds in the downstream tool :-D


Artem Pelenitsyn <[hidden email]> writes:

> Hey Sam,
>
> I think the thing you propose hardly qualifies as a new feature (in the
> sense of page you referenced), so not much of a hassle should be involved.
> As long as it is a 3-lines change, I'd say go ahead and create an Issue and
> an MR on Gitlab: you might be better off with getting feedback there.
>
> --Best, Artem
>
> On Tue, 6 Aug 2019 at 15:52, Sam Halliday <[hidden email]> wrote:
>
>> Hello,
>>
>> I would like to submit a patch to ghc 8.8 adding a function that exposes a
>> field that is useful for tooling authors and can already be reached, albeit
>> in a very awkward way.
>>
>> The process documented at
>>
>> https://gitlab.haskell.org/ghc/ghc/wikis/working-conventions/adding-features
>> seems very heavyweight for the patch that I was planning to propose,
>> which is just adding the following to GHC.hs (and exporting it):
>>
>>   modInfoRdrEnv :: ModuleInfo -> Maybe GlobalRdrEnv
>>   modInfoRdrEnv = minf_rdr_env
>>
>> Without this accessor, we must reparse and typecheck the file to get the
>> GlobalRdrElts, which is very inefficient and uses fields named "internal"
>> so I'm guessing that's not a good thing to build a tool on top of.
>>
>> My colleague already wrote a tool using a workaround, see
>> https://gitlab.com/tseenshe/hsinspect/blob/503cd48faba5b308be29ed44a12f7d6b22105f2b/exe/Main.hs#L53-61
>>
>> How should I go about submitting this? Do I need to write a test or is
>> this trivial enough not to bother?
>>
>> Sam Halliday <[hidden email]> writes:
>>
>> > Hi all,
>> >
>> > Is there a reason why minf_rdr_env (a field in ModuleInfo) is not
>> > exposed?
>> >
>> > It's possible to reconstruct it in 8.4.4 (the only version I'm looking
>> > at) with a TypecheckedModule via
>> >
>> >   let (tc_gbl_env, _) = GHC.tm_internals_ tmod
>> >       minf_rdr_env = tcg_rdr_env tc_gbl_env
>> >
>> >
>> > It's a useful thing to have for editor tooling (e.g. to get the correct
>> > qualified imported symbols that may be autocompleted).
>> >
>> > --
>> > Best regards,
>> > Sam
>>
>> --
>> Best regards,
>> Sam
>> _______________________________________________
>> ghc-devs mailing list
>> [hidden email]
>> http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
>>
--
Best regards,
Sam


_______________________________________________
ghc-devs mailing list
[hidden email]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

signature.asc (199 bytes) Download Attachment