Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ocaml compiler lib asttypes #5658

Closed
vicuna opened this issue Jun 19, 2012 · 19 comments
Closed

ocaml compiler lib asttypes #5658

vicuna opened this issue Jun 19, 2012 · 19 comments

Comments

@vicuna
Copy link

vicuna commented Jun 19, 2012

Original bug ID: 5658
Reporter: @bobzhang
Status: acknowledged (set by @damiendoligez on 2013-07-12T15:48:46Z)
Resolution: open
Priority: normal
Severity: feature
Category: configure and build/install
Tags: patch
Monitored by: mehdi @glondu

Bug description

when I derive code mechanically code for ocaml compiler itself.
I have found that asttypes.mli only have interface file without implementation.
It's fine when compiling. but I get an linking error.

Is there any technical difficulties that just rename asttypes.mli to asttypes.ml?
Many thanks

File attachments

@vicuna
Copy link
Author

vicuna commented Jun 19, 2012

Comment author: @bobzhang

I attached the patch. and copied asttypes.ml from asttypes.mli. Maybe you can find one way to keep only one copy.
Many thanks

@vicuna
Copy link
Author

vicuna commented Jun 20, 2012

Comment author: @alainfrisch

Why do you want to link Asttypes? If it doesn't contain any implementation, you should not need to link it.

@vicuna
Copy link
Author

vicuna commented Jun 20, 2012

Comment author: @glondu

For example, compiling:

module Foo = struct
include Asttypes
end

results in the following error:

File "toto.ml", line 1, characters 0-1:
Error: Error while linking toto.cmo:
Reference to undefined global `Asttypes'

@vicuna
Copy link
Author

vicuna commented Jun 20, 2012

Comment author: @garrigue

module rec Foo : module type of Asttypes = Foo

@vicuna
Copy link
Author

vicuna commented Jun 20, 2012

Comment author: @glondu

module Bar1 (A : module type of Asttypes) = struct
type t = A.constant
let f (x : t) = x
end

module Bar2 = Bar1(Foo)
let bar2 = Bar2.f (Asttypes.Const_int 5)

module Bar3 = Bar1(Asttypes)
let bar3 = Bar3.f (Asttypes.Const_int 5)

@vicuna
Copy link
Author

vicuna commented Jun 20, 2012

Comment author: @bobzhang

Thanks for the work around. But is there any bad effect that renaming asttypes.mli into asttypes.ml?

@vicuna
Copy link
Author

vicuna commented Jun 20, 2012

Comment author: @bobzhang

my workaround(it's really ugly!!)
module rec Foo: module type of Parsetree with
type core_type = Parsetree.core_type and
type core_type_desc = Parsetree.core_type_desc and
type package_type = Parsetree.package_type and
type core_field_type = Parsetree.core_field_type and
type row_field = Parsetree.row_field and
type 'a class_infos = 'a Parsetree.class_infos and
type pattern = Parsetree.pattern and
type pattern_desc = Parsetree.pattern_desc and
type expression = Parsetree.expression and
type expression_desc = Parsetree.expression_desc and
type value_description = Parsetree.value_description and
type type_declaration = Parsetree.type_declaration and
type type_kind = Parsetree.type_kind and
type exception_declaration = Parsetree.exception_declaration and
type class_type = Parsetree.class_type and
type class_type_desc = Parsetree.class_type_desc and
type class_signature = Parsetree.class_signature and
type class_type_field = Parsetree.class_type_field and
type class_type_field_desc = Parsetree.class_type_field_desc and
type class_description = Parsetree.class_description and
type class_type_declaration = Parsetree.class_type_declaration and
type class_expr = Parsetree.class_expr and
type class_expr_desc = Parsetree.class_expr_desc and
type class_structure = Parsetree.class_structure and
type class_field = Parsetree.class_field and
type class_field_desc = Parsetree.class_field_desc and
type class_declaration = Parsetree.class_declaration and
type module_type = Parsetree.module_type and
type module_type_desc = Parsetree.module_type_desc and
(* type signature = Parsetree.signature and )
type signature_item = Parsetree.signature_item and
type signature_item_desc = Parsetree.signature_item_desc and
type modtype_declaration = Parsetree.modtype_declaration and
type with_constraint = Parsetree.with_constraint and
type module_expr = Parsetree.module_expr and
type module_expr_desc = Parsetree.module_expr_desc and
(
type structure = Parsetree.structure and *)
type structure_item = Parsetree.structure_item and
type structure_item_desc = Parsetree.structure_item_desc and
type toplevel_phrase = Parsetree.directive_argument
= Foo

@vicuna
Copy link
Author

vicuna commented Jun 20, 2012

Comment author: @bobzhang

I don't see any harm that renaming parsetree.mli and asttypes.mli to .ml. It's really helpful to make the change.
Many thanks

@vicuna
Copy link
Author

vicuna commented Jun 21, 2012

Comment author: @alainfrisch

When a module contains only static components (types), there is indeed a choice between:

  1. having only an .mli file
  2. having only an .ml file
  3. having both the .ml and the .mli files (identical)

(3) is not very nice (at least, the build system should be responsible for copying one file to the other, but then there is always the risk to edit the wrong one and lose the change when recompiling).

I believe that (1) is considered superior to (2), because generally speaking, it is a good idea to have .mli files (and I think this is better for using ocamldoc to generate documentation).

Now, there is one technical problem in that using (1) prevents from rebinding the module (module X = Y). I'm still not sure why you want to do that, but this could be solved quite easily in the compiler, by not creating a reference to an external module when only static components are imported. Here is the patch:

===================================================================
--- bytecomp/translmod.ml	(revision 12623)
+++ bytecomp/translmod.ml	(working copy)
@@ -234,7 +234,9 @@
 
 let rec transl_module cc rootpath mexp =
   match mexp.mod_desc with
-    Tmod_ident (path,_) ->
+  | Tmod_ident _ when Mtype.no_code_needed mexp.mod_env mexp.mod_type ->
+      Lconst (Const_pointer 0)
+  | Tmod_ident (path,_) ->
       apply_coercion cc (transl_path path)
   | Tmod_structure str ->
       transl_struct [] cc rootpath str

Do you think it would solve your problem?

(Note that this patch might change the semantics when compiling without -linkall if the referenced module has side effects.)

@vicuna
Copy link
Author

vicuna commented Jun 21, 2012

Comment author: @bobzhang

Thanks. It's pretty cool. plz apply the patch.
Why I need to extend the module is
when the code generator scan the directory for all .cmi files, it generate a new module M which extends module M with some utility functions, for example,pretty printer. Suppose the type definitions in N.cmi contains some type (M.t), the code generator will call M.pp_print_t, so the code generator should be applied to M.cmi which should extend module M.
With this patch, I don't need to have control for the directory parsing/ typing/

@vicuna
Copy link
Author

vicuna commented Jun 21, 2012

Comment author: @alainfrisch

I'm checking with other OCaml developers if they are fine with this solution.

@vicuna
Copy link
Author

vicuna commented Jun 27, 2012

Comment author: @bobzhang

is it possible to make the change? I found it's a common idiom to just write mli files for the type definitions among some caml programmers. When I derive the code for dypgen, it's also the same case, they only write parse_tree.mli.
Please apply the patch if possible, many thanks.

@vicuna
Copy link
Author

vicuna commented Jul 12, 2012

Comment author: @alainfrisch

is it possible to make the change?

Unfortunately, there is no consensus between OCaml developers that the change of semantics is harmless. (Namely, with the patch, a simple reference "module M = X" to a module X with an empty signature does not force X to be linked anymore, if X is part of a library. This can change the semantics if X has side effects.)

@vicuna
Copy link
Author

vicuna commented Jul 12, 2012

Comment author: @bobzhang

thanks, it makes sense. maybe in the long term it would be nice to add some meta-data to highlight functors which have side effect. It's possible to rename all the .mli files which include only type definitions in the directory typing and parsing to .ml files?

I was designing a quotation expander for parsetree and typedtree, it's fine for me to write those equalities by hand, but I may need to sync up when compiler changes which is not interesting.
Sorry for my poor English.
Another question: given -ppx flag is merged into trunk, is it possible that the compile can accept typedtree? I have heard that after -bin-anot merged, it's possible to recover the source information by typedtree, so it also makes sense to do meta-programming in the typedtree level.

@vicuna
Copy link
Author

vicuna commented Jul 12, 2012

Comment author: @alainfrisch

It's possible to rename all the .mli files which include only type definitions in the directory typing and parsing to .ml files?

I'll let other developers decide on this point, I've no opinion.

it also makes sense to do meta-programming in the typedtree level.

This is really different from rewriting parsetrees, because people writing the "extension" need to understand very well the data structures manipulated with the type-checker, its invariants, and the APIs. It's easy to break type soundness. An intermediate proposal is OCamlTemplates by Fabrice Le Fessant. IIRC, this is about allowing "quotations" in the parsetree, to be rewritten by dynlinked plugins during type-checking into parse tree fragments. This means the "extension" can use type information, but generated fragments will be type-checked by the regular type-checker. This is a little bit fragile because the extension can still break internal data structures of the type-checker, but still much more robust than allowing it to generate directly (potentially wrong) typed trees.

Anyway, this is rather out of scope for this ticket. Let's discuss that somewhere else!

@vicuna
Copy link
Author

vicuna commented Jul 12, 2013

Comment author: @damiendoligez

FTR, I don't think it's a good idea to have a .ml without a .mli, and I'd like to avoid this in the compiler sources. I'd rather have both files with identical contents.

@vicuna
Copy link
Author

vicuna commented Jul 14, 2013

Comment author: @lpw25

Couldn't we just add the equivalent of:

ocamlc -c -impl asttypes.mli

to the build instructions?

That seems to work fine on some toy examples.

@ejgallego
Copy link

Hi all, I stumbled upon this issue and it seems to me that it should be closed, right?

@github-actions
Copy link

github-actions bot commented Sep 7, 2020

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.

@github-actions github-actions bot added the Stale label Sep 7, 2020
@trefis trefis closed this as completed Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants