Skip to content

Commit 6e782ee

Browse files
phatedpeblair
andauthored
fix(graindoc): Improve location lookup so re-exports do not crash it (#1280)
chore(compiler): Add export ident to export_declarations Co-authored-by: Philip Blair <philip@pblair.org>
1 parent df55898 commit 6e782ee

File tree

5 files changed

+99
-31
lines changed

5 files changed

+99
-31
lines changed

compiler/graindoc/docblock.re

Lines changed: 89 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,69 @@ let () =
3232
}
3333
});
3434

35+
let enumerate_exports = stmts => {
36+
let id_tbl = ref(Ident.empty);
37+
38+
let rec pattern_ids = ({pat_desc, pat_loc}: Typedtree.pattern) => {
39+
switch (pat_desc) {
40+
| TPatVar(id, _) => [(id, pat_loc)]
41+
| TPatAlias(subpat, id, _) => [(id, pat_loc), ...pattern_ids(subpat)]
42+
| TPatTuple(pats)
43+
| TPatArray(pats)
44+
| TPatConstruct(_, _, pats) => List.concat(List.map(pattern_ids, pats))
45+
| TPatRecord(elts, _) =>
46+
List.concat(List.map(((_, _, pat)) => pattern_ids(pat), elts))
47+
| _ => []
48+
};
49+
};
50+
51+
module ExportIterator =
52+
TypedtreeIter.MakeIterator({
53+
include TypedtreeIter.DefaultIteratorArgument;
54+
55+
let enter_toplevel_stmt =
56+
({ttop_desc, ttop_attributes}: Typedtree.toplevel_stmt) => {
57+
switch (ttop_desc) {
58+
| TTopData(decls) =>
59+
List.iter(
60+
({data_id, data_loc}: Typedtree.data_declaration) => {
61+
id_tbl := Ident.add(data_id, data_loc, id_tbl^)
62+
},
63+
decls,
64+
)
65+
| TTopExport(decls) =>
66+
List.iter(
67+
({tex_id, tex_loc}: Typedtree.export_declaration) => {
68+
id_tbl := Ident.add(tex_id, tex_loc, id_tbl^)
69+
},
70+
decls,
71+
)
72+
| TTopForeign({tvd_id, tvd_loc}) =>
73+
id_tbl := Ident.add(tvd_id, tvd_loc, id_tbl^)
74+
| TTopLet(_, _, vbinds) =>
75+
List.iter(
76+
({vb_pat}: Typedtree.value_binding) => {
77+
List.iter(
78+
((id, loc)) => {id_tbl := Ident.add(id, loc, id_tbl^)},
79+
pattern_ids(vb_pat),
80+
)
81+
},
82+
vbinds,
83+
)
84+
| _ => ()
85+
};
86+
};
87+
});
88+
89+
List.iter(ExportIterator.iter_toplevel_stmt, stmts);
90+
91+
id_tbl^;
92+
};
93+
94+
let location_for_ident = (~exports, ident) => {
95+
snd(Ident.find_name(Ident.name(ident), exports));
96+
};
97+
3598
let module_name_of_location = (loc: Grain_parsing.Location.t) => {
3699
Grain_utils.Filepath.String.filename_to_module_name(
37100
loc.loc_start.pos_fname,
@@ -83,12 +146,12 @@ let lookup_type_expr = (~idx, type_exprs) => {
83146
};
84147

85148
let for_value_description =
86-
(~comments, ~ident: Ident.t, vd: Types.value_description) => {
87-
let module_name = module_name_of_location(vd.val_loc);
149+
(~comments, ~ident: Ident.t, ~loc, vd: Types.value_description) => {
150+
let module_name = module_name_of_location(loc);
88151
let name = title_for_api(~module_name, ident);
89152
let type_sig = Printtyp.string_of_value_description(~ident, vd);
90153
let comment =
91-
Comments.Doc.ending_on(~lnum=vd.val_loc.loc_start.pos_lnum - 1, comments);
154+
Comments.Doc.ending_on(~lnum=loc.loc_start.pos_lnum - 1, comments);
92155

93156
let (description, attributes) =
94157
switch (comment) {
@@ -118,15 +181,12 @@ let for_value_description =
118181
};
119182

120183
let for_type_declaration =
121-
(~comments, ~ident: Ident.t, td: Types.type_declaration) => {
122-
let module_name = module_name_of_location(td.type_loc);
184+
(~comments, ~ident: Ident.t, ~loc, td: Types.type_declaration) => {
185+
let module_name = module_name_of_location(loc);
123186
let name = title_for_api(~module_name, ident);
124187
let type_sig = Printtyp.string_of_type_declaration(~ident, td);
125188
let comment =
126-
Comments.Doc.ending_on(
127-
~lnum=td.type_loc.loc_start.pos_lnum - 1,
128-
comments,
129-
);
189+
Comments.Doc.ending_on(~lnum=loc.loc_start.pos_lnum - 1, comments);
130190

131191
let (description, attributes) =
132192
switch (comment) {
@@ -138,35 +198,37 @@ let for_type_declaration =
138198
};
139199

140200
let for_signature_item =
141-
(~env: Env.t, ~comments, sig_item: Types.signature_item) => {
201+
(
202+
~comments,
203+
~exports: Ident.tbl(Grain_parsing.Location.t),
204+
sig_item: Types.signature_item,
205+
) => {
142206
switch (sig_item) {
143-
| TSigValue(ident, ovd) =>
144-
// Fetch original location as signatures don't contain real locations
145-
let vd = Env.find_value(ovd.val_fullpath, env);
146-
let val_loc = vd.val_loc;
147-
let docblock =
148-
for_value_description(~comments, ~ident, {...ovd, val_loc});
207+
| TSigValue(ident, vd) =>
208+
let loc = location_for_ident(~exports, ident);
209+
let docblock = for_value_description(~comments, ~ident, ~loc, vd);
149210
Some(docblock);
150-
| TSigType(ident, otd, _rec) =>
151-
// Fetch original location as signatures don't contain real locations
152-
let td = Env.find_type(otd.type_path, env);
153-
let type_loc = td.type_loc;
154-
let docblock =
155-
for_type_declaration(~comments, ~ident, {...otd, type_loc});
211+
| TSigType(ident, td, _rec) =>
212+
let loc = location_for_ident(~exports, ident);
213+
let docblock = for_type_declaration(~comments, ~ident, ~loc, td);
156214
Some(docblock);
157215
| _ => None
158216
};
159217
};
160218

161219
let signature_item_in_range =
162-
(~env: Env.t, sig_item: Types.signature_item, range: Grain_utils.Range.t) => {
220+
(
221+
~exports: Ident.tbl(Grain_parsing.Location.t),
222+
sig_item: Types.signature_item,
223+
range: Grain_utils.Range.t,
224+
) => {
163225
switch (sig_item) {
164226
| TSigValue(ident, vd) =>
165-
let vd = Env.find_value(vd.val_fullpath, env);
166-
Grain_utils.Range.inRange(vd.val_loc.loc_start.pos_lnum, range);
227+
let loc = location_for_ident(~exports, ident);
228+
Grain_utils.Range.inRange(loc.loc_start.pos_lnum, range);
167229
| TSigType(ident, td, _rec) =>
168-
let td = Env.find_type(td.type_path, env);
169-
Grain_utils.Range.inRange(td.type_loc.loc_start.pos_lnum, range);
230+
let loc = location_for_ident(~exports, ident);
231+
Grain_utils.Range.inRange(loc.loc_start.pos_lnum, range);
170232
| _ => false
171233
};
172234
};

compiler/graindoc/graindoc.re

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ let generate_docs =
7878
(~current_version, ~output=?, program: Typedtree.typed_program) => {
7979
let comments = Comments.to_ordered(program.comments);
8080

81-
let env = program.env;
81+
let exports = Docblock.enumerate_exports(program.statements);
82+
8283
let signature_items = program.signature.cmi_sign;
8384

8485
let buf = Buffer.create(0);
@@ -156,7 +157,7 @@ let generate_docs =
156157
};
157158

158159
let add_docblock = sig_item => {
159-
let docblock = Docblock.for_signature_item(~env, ~comments, sig_item);
160+
let docblock = Docblock.for_signature_item(~comments, ~exports, sig_item);
160161
switch (docblock) {
161162
| Some(docblock) =>
162163
Buffer.add_buffer(
@@ -197,7 +198,7 @@ let generate_docs =
197198
);
198199
List.iter(
199200
sig_item =>
200-
if (Docblock.signature_item_in_range(~env, sig_item, range)) {
201+
if (Docblock.signature_item_in_range(~exports, sig_item, range)) {
201202
add_docblock(sig_item);
202203
},
203204
signature_items,

compiler/src/typed/typedtree.re

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,7 @@ type import_declaration = {
509509

510510
[@deriving sexp]
511511
type export_declaration = {
512+
tex_id: Ident.t,
512513
tex_path: Path.t,
513514
[@sexp_drop_if sexp_locs_disabled]
514515
tex_loc: Location.t,

compiler/src/typed/typedtree.rei

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ type import_declaration = {
472472

473473
[@deriving sexp]
474474
type export_declaration = {
475+
tex_id: Ident.t,
475476
tex_path: Path.t,
476477
[@sexp_drop_if sexp_locs_disabled]
477478
tex_loc: Location.t,

compiler/src/typed/typemod.re

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,10 @@ let type_module = (~toplevel=false, funct_body, anchor, env, sstr /*scope*/) =>
602602
};
603603
let name = Identifier.IdentName(name);
604604
let (p, {val_fullpath} as desc) = Env.lookup_value(name, env);
605-
(TSigValue(id, desc), {tex_path: val_fullpath, tex_loc: loc});
605+
(
606+
TSigValue(id, desc),
607+
{tex_id: id, tex_path: val_fullpath, tex_loc: loc},
608+
);
606609
},
607610
exports,
608611
);

0 commit comments

Comments
 (0)