Nailen Matschke

Results 11 comments of Nailen Matschke

@lpw25 it does indeed accept that. Will think more about how to fix it.

@lpw25 I have implemented the fix as discussed (offline) and added a couple test cases to recursive_modules.ml in the testsuite.

@yallop I'm happy to modify the behavior to something else, but I'd argue that this is consistent with how `let` bindings work in structures. Note that your example would be...

Sorry, it took a while to get back to this. I've implemented the suggested change, to instead use `matches` to check whether the primitive's type could be instantiated to that...

@Octachron could you take a look at this PR, please, at your convenience? Thanks!

Thanks @yallop. 1. Yes, that essentially matches my understanding. Pedantically, I think it is acceptable when the signature ascription `struct external p : t = ... end : sig val...

@yallop > The third point (attributes on aliases) also seems mostly fine, although if attributes on the alias have no effect then I'd think that we ought to at least...

Awesome! By the way, @yallop, the CI is complaining about the "Parsetree Updated: comment-and-label" job. My impression is that this requires adding a label to the PR, which I'm not...

Thanks for the review @yallop. I pushed some changes that (I think) resolve the issues you raised. @Octachron ah yeah, ~~I see how it's tricky to get the `non_aliasable` check...

We intend to move `Nonempty_list` into `Base` but it's not at the top of our stack. We'll update this issue if and when that happens.