Hi Josh,

I don't think this addresses the points mine or Zubin's email that is
it not correct to write a HIE file with a different BinHandle. If you
do this then you will end up with a HIE file which contains Names
which don't contain any SrcSpan information.

If I am misunderstanding then please forgive me.

Cheers,

Matt

On Thu, Apr 23, 2020 at 4:06 PM Josh Meredith
<[email protected]> wrote:
>
> Since the current HIE serialisation function writes to a BinHandle, which it 
> initialises itself, the simple way to implement the HIE field would be to 
> change the handle to an argument:
>
> writeHieFile :: FilePath -> HieFile -> IO ()
> writeHieFile hie_file_path hiefile = do
>   bh0 <- openBinMem initBinMemSize
>   -- Actual serialisation work
>
> becomes
>
> writeHieFile :: FilePath -> HieFile -> IO ()
> writeHieFile hie_file_path hiefile = do
>   bh0 <- openBinMem initBinMemSize
>   writeHie hiefile bh0
>
> writeHie :: HieFile -> BinHandle -> IO ()
> writeHie hiefile bh = do
>   -- Actual serialisation work
>
> Then the existing discrete file can continue to use writeHieFile, and the 
> field can reuse the existing implementation via writeHie:
>
> writeIfaceFieldWith :: FieldName -> (BinHandle -> IO ()) -> ModIface -> IO 
> ModIface
>
> writeHieField :: HieFile -> ModIface -> IO ModIface
> writeHieField hiefile iface = writeIfaceFieldWith "ghc/hie" (writeHie 
> hiefile) iface
>
> I hope this clarifies how I intend these more edge cases to work?
>
> Cheers,
> Josh
>
>
> On Thu, 23 Apr 2020 at 20:25, Zubin Duggal <[email protected]> wrote:
>>
>> Yes, Matthew is correct, the symbol table for Names used by HIE files is
>> quite distinct from the usual Iface symbol table for Names. Sharing the HIE
>> symbol table and the Iface symbol table for names will be quite tricky.
>>
>> One crucial difference is that we also save information about local names
>> to the symbol table for HIE files.
>>
>> Here is how names are stored in the HIE symbol table:
>>
>> data HieName
>>   = ExternalName !Module !OccName !SrcSpan
>>   | LocalName !OccName !SrcSpan
>>   | KnownKeyName !Unique
>>
>> And the usual symbol table(KnownKeyNames are implicitly handled as a 
>> separate case):
>>
>> type OnDiskName = (UnitId, ModuleName, OccName)
>>
>> So the merge of HIE and HI files will have to be careful about preserving 
>> these
>> semantics. For instance, the `Binary` instances for HIE file data structures 
>> use
>> putName_, which will have the wrong semantics after this merge. We could 
>> possibly
>> get around this by having ExtensibleFields be allowed to override the 
>> UserData
>> field of the BinHandle.
>>
>> Thanks to Matthew for bringing this up this important point.
>>
>> - Zubin
>>
>> On 20/04/23 09:06, Matthew Pickering wrote:
>> > I also looked at this patch briefly now and it's my understanding that
>> > the intention is to use this for HIE files as well?
>> >
>> > HIE files currently serialises Names differently to normal interface
>> > files (to also include the SrcSpan) and it wasn't clear to me that the
>> > current implementation would allow for this.
>> >
>> > Zubin/Josh could you please comment?
>> >
>> > Otherwise looks like a nice patch.
>> >
>> > Cheers,
>> >
>> > Matt
>> >
>> > On Thu, Apr 23, 2020 at 8:57 AM Ömer Sinan Ağacan <[email protected]> 
>> > wrote:
>> > >
>> > > The "extensible interface files" [1, 2] work has been discussed at GHC 
>> > > calls a
>> > > few times and the conclusion was we were going to document why current
>> > > annotation mechanism (ANN pragmas) are insufficient and we need yet 
>> > > another way
>> > > to put stuff into interfaces.
>> > >
>> > > Unfortunately the MR was merged before that's done and so it's currently
>> > > undocumented and it's still unclear what's insufficient about ANN 
>> > > pragmas or how
>> > > the new mechanism differs.
>> > >
>> > > I'm currently working on an interface files related patch (#16885) so I 
>> > > have to
>> > > maintain both of these features now. It'd be good to know why both is 
>> > > necessary.
>> > > If ANN is no longer useful or needed could we deprecate it in favor of 
>> > > the new
>> > > mechanism and remove it in a few releases? That's help maintaining the 
>> > > code in
>> > > the future.
>> > >
>> > > Could people involved in this design and patch please document the 
>> > > thought
>> > > process here?
>> > >
>> > > Thanks,
>> > >
>> > > Ömer
>> > >
>> > > [1]: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/2948
>> > > [2]: 
>> > > https://gitlab.haskell.org/ghc/ghc/-/wikis/Extensible-Interface-Files
>> > > _______________________________________________
>> > > ghc-devs mailing list
>> > > [email protected]
>> > > http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
_______________________________________________
ghc-devs mailing list
[email protected]
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

Reply via email to