Making the compiler independent from syntactic constraints on ident capitalization #7743
Comments
|
Comment author: @lpw25 I have a patch lying around somewhere from my work on namespacing that fixes this properly. It just adds a new constructor to Path.t to handle the case of projecting a constructor type from a type. However, I would warn you that there are plenty of places that assume things about identifiers that are preserved by the parser but not checked by the type system. For example, the capitalization of module identifiers is relied on in a few places. These should all be removed as they are clearly terrible software engineering, but it will require some work. |
|
Comment author: turpin Isn't adding a constructor to Path quite invasive ? I haven't played (yet) with uncapitalized module / modtype names (as I do with virtually all other kinds of idents), but I will remember the warning, thanks ! |
|
Comment author: @lpw25
Not particularly, and really the constructor was already added to Path.t and just given a bizarre ascii-based encoding. Any additional places where we will now be forced to consider the new case are places where we could easily have a bug currently. |
|
This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc. |
|
I agree this would be nice, but it belongs to the "code cleanup" category. I'm not sure if we want to keep those open. Resilence to AST with unexpected forms in the typechecker is now relatively good. |
|
Yeah, I'll close the issue. But we really need to clean up these parts of the system, they are bugs waiting to happen (and IIRC a couple of bugs that already happened). |
Original bug ID: 7743
Reporter: turpin
Status: new
Resolution: open
Priority: normal
Severity: feature
Version: 4.06.1
Category: typing
Monitored by: @yallop
Bug description
Since the introduction of inline records in OCaml 4.03, assumptions have appeared in the typing part of the compiler, on the capitalization of identifiers. In fact, inline types are named (very naturally) as their parent datatype constructor, and the capitalization is sometimes used to decide where to retrieve type information. This happens in function Env.find_type_full, which "classifies" paths using Path.constructor_typath. This works perfectly for OCaml code, prevents using generated ASTs which violate such assumptions by using, for example, capitalized type constructor names.
Avoiding to rely on syntactic assumptions in the type-checker is not only a good design choice from modularity perspective, I think it also makes sense, given the powerful AST transformation capabilities that OCaml provides.
From my (incomplete) understanding of the compiler typing code, I believe that those assumptions are not essential, and could be avoided with reasonable work, though I don't think I understand enough to do it properly (I may try again if my knowledge gets better). I have a small patch to do things slightly differently: instead of relying on capitalization, Env.find_type_full always searches for an inline records if the "normal" lookup fails. But this is a hack, it is incorrect if both exist. And I'm unsure about the Ext/LocalExt cases.
Apart from that, I have not encountered any other places where the compiler breaks if OCaml syntax capitalization rules are violated, which is really nice !
Best regards,
Tiphaine Turpin
MathWorks
The text was updated successfully, but these errors were encountered: