feat(compiler)!: Refactor enum constructors #1211

ibdafna · 2022-05-09T05:51:04Z

Signed-off-by: Itay Dafna i.b.dafna@gmail.com

Closes #1021
Closes #1502

This PR changes how enum variant constructors work in Grain, making individual variants handled separately from functions. This change allows us to build variant types into the language rather than just the standard library, which will allow us to provide better error messages, allow for disambiguation of variants from multiple types with the same name, and open up opportunities for features like optional arguments. The tradeoff is that the names of variant constructors are no longer functions, but this feature was only seldom used (only appearing once in Grain's entire standard library). Wrapper functions can be used to provide the same functionality.

Before:

List.map(Some, [1, 2, 3])

After:

List.map(x => Some(x), [1, 2, 3])

ospencer · 2022-12-24T02:01:39Z

compiler/src/codegen/compcore.re

-let assertion_error_ident = Ident.create_persistent("AssertionError");
-let assertion_error_closure_ident =
- Ident.create_persistent("GRAIN$EXPORT$AssertionError");
-let index_out_of_bounds_ident =
- Ident.create_persistent("GRAIN$EXPORT$IndexOutOfBounds");
-let index_non_integer_ident =
- Ident.create_persistent("GRAIN$EXPORT$IndexNonInteger");
-let match_failure_ident =
- Ident.create_persistent("GRAIN$EXPORT$MatchFailure");


There are no identifiers or closures for variants anymore.

ospencer · 2022-12-24T02:02:59Z

compiler/src/codegen/compcore.re

@@ -90,6 +81,7 @@ let console_mod = "console";
 let tracepoint_ident = Ident.create_persistent("tracepoint");

 let grain_main = "_gmain";
+let grain_type_metadata = "_gtype_metadata";


Type metadata is now explicitly provided by a special type metadata function in the module. This allows modules to choose to define only types but still have type information loaded.

ospencer · 2022-12-24T02:03:52Z

compiler/src/codegen/compcore.re

- | Runtime_errors.AssertionError => (
- assertion_error_closure_ident,
- Some(get_wasm_imported_name(exception_mod, assertion_error_ident)),
+/** Heap allocations. */


None of this has changed, it's just moved higher so we can do allocations for exceptions (which was previously just calling a function).

ospencer · 2022-12-24T02:05:29Z

compiler/src/codegen/compcore.re

@@ -2484,6 +2388,9 @@ let compile_prim1 = (wasm_mod, env, p1, arg, loc): Expression.t => {
 Expression.Unreachable.make(wasm_mod),
 ],
 )
+ | Assert => failwith("Unreachable case; should never get here: Assert")
+ | BuiltinId =>


This BuiltinId will be how print will be able to know about built-in types.

ospencer · 2022-12-24T02:07:29Z

compiler/src/linking/link.re

-
-let grain_main = "_gmain";
-let grain_start = "_start";
+let hard_dependencies: Hashtbl.t(string, unit) = Hashtbl.create(10);


Hard dependencies are ones that require code to be loaded from the module, whereas soft dependencies just require type information be loaded.

ospencer · 2022-12-24T02:08:27Z

compiler/src/linking/link.re

@@ -301,17 +285,25 @@ let rec globalize_names = (~function_names, ~global_names, ~label_names, expr) =
 };

 let table_offset = ref(0);
-let module_id = ref(0);
+let module_id = ref(Comp_utils.encoded_int32(0));


This was a really sad bug that took away a week of my life.

Can you explain it a bit?

As defined in the data representations doc, module IDs are always tagged and thus GC-able. Except during linking, the linker assigned non-tagged IDS 🤦‍♂️

This wasn't a problem before because the IDs were treated by the runtime as untagged, but some updates while working on this PR caused them to be treated as regular values and made the bug show up.

ospencer · 2022-12-24T02:10:45Z

compiler/src/linking/link.re

- let dependencies =
- Topo.fold((dep, acc) => [dep, ...acc], dependency_graph, []);
+
+ let dependencies = Module_resolution.get_dependencies();


This gives us the full dependency graph, which includes files that might not contain necessary code.

ospencer · 2022-12-24T02:12:25Z

compiler/src/typed/builtin_types.re

@@ -21,12 +21,11 @@ open Path;
 open Types;
 open Btype;

-let builtin_idents = ref([]);
-let builtin_decls = ref([]);
+let builtin_idents = Hashtbl.create(64);


The hash lets us look these up quickly versus having to traverse the list.

ospencer · 2022-12-24T02:18:31Z

stdlib/runtime/string.gr

@@ -46,42 +46,33 @@ enum StringList {
 primitive _RUNTIME_TYPE_METADATA_PTR: WasmI32 = "@heap.type_metadata"

 @unsafe
-let findTypeMetadata = (moduleId, typeId) => {
+let findTypeMetadata = typeId => {


This function is now quite a bit less efficient since it traverses all type information. It should be converted to something faster like a binary tree or a hash, though it's tricky to implement because the metadata itself lives in runtime land where arbitrary allocations are not available. Regardless, I felt that optimization didn't need to be included in this PR and could come separately.

phated

Thanks for the comments inline, they helped a ton! I had one question about the parser (and I'll probably spend a little more time looking at snapshots).

compiler/src/parsing/parser.mly

marcusroberts

I had a good read through this - nice to see list constructors cleaned up as well and looks like some nice changes as well as a deep bug fixed. Really nice work!

Hopefully with the new code owner rules I can approve this releasing it as a lot of the changes are beyond my current comprehension but I liked the changes I did understand and the code changes read well.

ospencer · 2023-01-01T14:51:18Z

Note to self: need to update store_type and store_extension in env.re to stop adding constructors as binding names to the environment

Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>

ospencer · 2023-01-09T14:58:26Z

store_type and store_extension updated.

phated

Love this!

Co-authored-by: Oscar Spencer <oscar@grain-lang.org> Closes #1021 Closes #1502

phated marked this pull request as draft May 14, 2022 18:37

ibdafna force-pushed the ibd_comp branch from 6899dd0 to 095b604 Compare July 8, 2022 18:11

ibdafna mentioned this pull request Aug 5, 2022

[WIP] Add Option/Result to builtins #1101

Closed

ibdafna force-pushed the ibd_comp branch from 1712558 to f55e150 Compare August 5, 2022 19:14

phated assigned ospencer and ibdafna Aug 5, 2022

ibdafna force-pushed the ibd_comp branch from 4b6e5e9 to c786941 Compare August 12, 2022 18:42

ospencer force-pushed the ibd_comp branch from 1ca377b to 015029e Compare November 10, 2022 17:50

ospencer force-pushed the ibd_comp branch 4 times, most recently from 149f1df to 2c0dd5b Compare December 24, 2022 01:39

ospencer changed the title ~~[WIP] Implement PExpConstruct/TExpConstruct~~ feat(compiler)!: Refactor enum constructors Dec 24, 2022

ospencer reviewed Dec 24, 2022

View reviewed changes

ospencer marked this pull request as ready for review December 24, 2022 02:19

ospencer requested review from peblair, jozanza, phated and marcusroberts as code owners December 24, 2022 02:19

phated reviewed Dec 25, 2022

View reviewed changes

compiler/src/parsing/parser.mly Show resolved Hide resolved

marcusroberts approved these changes Dec 27, 2022

View reviewed changes

ospencer mentioned this pull request Jan 8, 2023

feat(compiler)!: Module system #1584

Merged

ibdafna and others added 2 commits January 9, 2023 08:50

feat(compiler)!: Refactor enum constructors

47b7ddd

Signed-off-by: Itay Dafna <i.b.dafna@gmail.com>

snapshots

ca6f7e5

ospencer force-pushed the ibd_comp branch from 4f589cc to ca6f7e5 Compare January 9, 2023 14:51

clean up store_type and store_extension

c5324e3

phated approved these changes Jan 10, 2023

View reviewed changes

jozanza approved these changes Jan 10, 2023

View reviewed changes

ospencer merged commit 8d465b7 into grain-lang:main Jan 10, 2023

This was referenced Jan 10, 2023

chore: release Grain v0.6 Emmer #1594

Merged

chore: release main spotandjake/grain#1

Open

marcusroberts pushed a commit that referenced this pull request Jan 14, 2023

feat(compiler)!: Refactor enum constructors (#1211)

cf9c666

Co-authored-by: Oscar Spencer <oscar@grain-lang.org> Closes #1021 Closes #1502

This was referenced Mar 20, 2023

chore: release main alex-snezhko/grain#2

Open

chore: release main alex-snezhko/grain#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(compiler)!: Refactor enum constructors #1211

feat(compiler)!: Refactor enum constructors #1211

ibdafna commented May 9, 2022 •

edited by ospencer

Loading

ospencer Dec 24, 2022

ospencer Dec 24, 2022

ospencer Dec 24, 2022

ospencer Dec 24, 2022

ospencer Dec 24, 2022

ospencer Dec 24, 2022

phated Dec 25, 2022

ospencer Dec 25, 2022

ospencer Dec 24, 2022

ospencer Dec 24, 2022

ospencer Dec 24, 2022

phated left a comment

marcusroberts left a comment

ospencer commented Jan 1, 2023

ospencer commented Jan 9, 2023

phated left a comment

feat(compiler)!: Refactor enum constructors #1211

feat(compiler)!: Refactor enum constructors #1211

Conversation

ibdafna commented May 9, 2022 • edited by ospencer Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phated left a comment

Choose a reason for hiding this comment

marcusroberts left a comment

Choose a reason for hiding this comment

ospencer commented Jan 1, 2023

ospencer commented Jan 9, 2023

phated left a comment

Choose a reason for hiding this comment

ibdafna commented May 9, 2022 •

edited by ospencer

Loading