-
Notifications
You must be signed in to change notification settings - Fork 5
Open
Labels
Description
In the binding specs code we use Hs.ExtRef
data ExtRef = ExtRef {
extRefModule :: Hs.ModuleName
, extRefIdentifier :: Hs.Identifier
}but in most places in hs-bindgen we use Hs.Identifier, without the Hs.ModuleName. This is mostly fine, but it does result in some iffy code, most importantly
hs-bindgen/hs-bindgen/src-internal/HsBindgen/Frontend/Pass/ResolveBindingSpecs/IsPass.hs
Lines 67 to 71 in b4192e9
| extDeclIdPair :: ResolvedExtBinding -> C.DeclIdPair | |
| extDeclIdPair ext = C.DeclIdPair{ | |
| cName = ext.extCDeclId | |
| , hsName = ext.extHsRef.extRefIdentifier | |
| } |
This loss of the module has some consequences; for example, in
| depsOfType :: Type p -> [(ValOrRef, Id p)] |
we currently have
hs-bindgen/hs-bindgen/src-internal/HsBindgen/Frontend/AST/Type.hs
Lines 361 to 368 in b4192e9
| -- TODO <https://github.com/well-typed/hs-bindgen/issues/1467> | |
| -- We could in /principle/ use extBindingId here to implement this case | |
| -- properly. However, one of the use cases of 'depsOfType' is in a check | |
| -- whether a type is defined in the current module; 'extBindingId' currently | |
| -- omits the module, so this could result in potentially incorrect | |
| -- conclusions (if there happens to be a /another/ type of the same name). | |
| -- TypeExtBinding extBinding -> [(ByValue, extBindingId (Proxy @p) extBinding)] | |
| TypeExtBinding _extBinding -> [] |
I don't think this leads to any actual bugs at the moment, but it would be better if we did this more correctly.