Explicitely disallow redefining module level fields #11232
Conversation
| | statics, Some c -> | ||
| raise_typing_error_ext (make_error ~sub:[ | ||
| make_error ~depth:1 (Custom "Previously defined here") c.cl_pos | ||
| ] (Custom (Printf.sprintf "Cannot add module level fields for module %s" (s_type_path m.m_path))) loadp); |
There was a problem hiding this comment.
The phrasing somehow suggests that this could be allowed for a different module.
There was a problem hiding this comment.
"Module level fields are already defined for module %s" ?
There was a problem hiding this comment.
Actually this makes me realize that we're not addressing the real problem here. This should be about adding module level fields to existing modules, whether or not they are already defined should not make a difference...
There was a problem hiding this comment.
Adding module level fields to an existing module that does not have module level fields already works. It's only when it already has module level fields that we're having issues.
There was a problem hiding this comment.
That doesn't sound right, I'll have to take a closer look at the original issue in that case...
There was a problem hiding this comment.
Yeah, repro from the issue is actually bad code since it's checking Context.getType with a module level field instead of a type, so always running defineModule.
It runs fine the first time because Test.hx has no module level fields, but obviously fails the next times because it is redefining the module level fields.
Edit: fixed link u_u
|
Do you know what the status here (and in #10374) is now? |
|
As said on slack, we actually need to make it a bit more strict so this PR needs to be updated |
Closes #10374