Skip to content

Commit 6863504

Browse files
kLabzkLabz
authored
[macro] fix redefinition of modules with defineType/defineModule (#12001)
* [macro] Disallow redefining module through Context's defineType/defineModule * [macro] defineType / defineModule: allow redefining MSBad modules Note: this hook should be moved somewhere else.. * [tests] small cleanup * [tests] add test * [tests] add more tests * [std] add haxe.macro.CompilationServer.invalidateModule() * Fix defineModule into curmod * [tests] add test using CompilationServer.invalidateModule * Default impl without server * [macro] disallow invalidating loaded module * CompilationServer.invalidate: raise error that macros can catch; add test * Rework implementation Allow redefining when not loaded, but invalidate previous cached module (if any) * [tests] update tests * Use more specific invalidation reasons * [tests] update tests * Let macros catch redefinition errors * [tests] test catching errors * [std] more information about catching errors
1 parent 2833046 commit 6863504

File tree

13 files changed

+322
-25
lines changed

13 files changed

+322
-25
lines changed

‎src/compiler/compilationCache.ml

+15-3
Original file line numberDiff line numberDiff line change
@@ -234,15 +234,27 @@ class cache = object(self)
234234
) cc#get_modules acc
235235
) contexts []
236236

237+
method taint_module m_path reason =
238+
Hashtbl.iter (fun _ cc ->
239+
Hashtbl.iter (fun _ m ->
240+
if m.m_path = m_path then m.m_extra.m_cache_state <- MSBad (Tainted reason)
241+
) cc#get_modules;
242+
Hashtbl.iter (fun _ mc ->
243+
if mc.HxbData.mc_path = m_path then
244+
mc.HxbData.mc_extra.m_cache_state <- match reason, mc.mc_extra.m_cache_state with
245+
| CheckDisplayFile, (MSBad _ as state) -> state
246+
| _ -> MSBad (Tainted reason)
247+
) cc#get_hxb
248+
) contexts
249+
237250
method taint_modules file_key reason =
238251
Hashtbl.iter (fun _ cc ->
239252
Hashtbl.iter (fun _ m ->
240253
if Path.UniqueKey.lazy_key m.m_extra.m_file = file_key then m.m_extra.m_cache_state <- MSBad (Tainted reason)
241254
) cc#get_modules;
242-
let open HxbData in
243255
Hashtbl.iter (fun _ mc ->
244-
if Path.UniqueKey.lazy_key mc.mc_extra.m_file = file_key then
245-
mc.mc_extra.m_cache_state <- match reason, mc.mc_extra.m_cache_state with
256+
if Path.UniqueKey.lazy_key mc.HxbData.mc_extra.m_file = file_key then
257+
mc.HxbData.mc_extra.m_cache_state <- match reason, mc.HxbData.mc_extra.m_cache_state with
246258
| CheckDisplayFile, (MSBad _ as state) -> state
247259
| _ -> MSBad (Tainted reason)
248260
) cc#get_hxb

‎src/core/tPrinting.ml

+3
Original file line numberDiff line numberDiff line change
@@ -637,8 +637,11 @@ module Printer = struct
637637

638638
let s_module_tainting_reason = function
639639
| CheckDisplayFile -> "check_display_file"
640+
| DefineType -> "define_type"
641+
| DefineModule -> "define_module"
640642
| ServerInvalidate -> "server/invalidate"
641643
| ServerInvalidateFiles -> "server_invalidate_files"
644+
| ServerInvalidateModule -> "server_invalidate_module"
642645

643646
let s_module_skip_reason reason =
644647
let rec loop stack = function

‎src/core/tType.ml

+3
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ type module_check_policy =
3232

3333
type module_tainting_reason =
3434
| CheckDisplayFile
35+
| DefineType
36+
| DefineModule
3537
| ServerInvalidate
3638
| ServerInvalidateFiles
39+
| ServerInvalidateModule
3740

3841
type module_skip_reason =
3942
| DependencyDirty of path * module_skip_reason

‎src/macro/macroApi.ml

+12
Original file line numberDiff line numberDiff line change
@@ -2323,6 +2323,18 @@ let macro_api ccom get_api =
23232323
(get_api()).add_module_check_policy filter policy (decode_bool recursive);
23242324
vnull
23252325
);
2326+
"server_invalidate_module", vfun1 (fun p ->
2327+
let mpath = parse_path (decode_string p) in
2328+
let com = ccom() in
2329+
(try
2330+
ignore(com.module_lut#find mpath);
2331+
let msg = "Cannot invalidate loaded module " ^ (s_type_path mpath) in
2332+
let pos = get_api_call_pos() in
2333+
compiler_error (Error.make_error (Custom msg) pos)
2334+
with Not_found ->
2335+
com.cs#taint_module mpath ServerInvalidateModule);
2336+
vnull
2337+
);
23262338
"server_invalidate_files", vfun1 (fun a ->
23272339
let com = ccom() in
23282340
let cs = com.cs in

‎src/typing/macroContext.ml

+18-9
Original file line numberDiff line numberDiff line change
@@ -464,13 +464,21 @@ let make_macro_api ctx mctx p =
464464
| _ -> false
465465
in
466466
let add is_macro ctx =
467-
let mdep = Option.map_default (fun s -> TypeloadModule.load_module ~origin:MDepFromMacro ctx (parse_path s) pos) ctx.m.curmod mdep in
468-
let mnew = TypeloadModule.type_module ctx.com ctx.g ~dont_check_path:(has_native_meta) mpath (ctx.com.file_keys#generate_virtual mpath ctx.com.compilation_step) [tdef,pos] pos in
469-
mnew.m_extra.m_kind <- if is_macro then MMacro else MFake;
470-
add_dependency mnew mdep MDepFromMacro;
471-
add_dependency mdep mnew MDepFromMacroDefine;
472-
ctx.com.module_nonexistent_lut#clear;
473-
in
467+
try
468+
let m = ctx.com.module_lut#find mpath in
469+
let pos = { pfile = (Path.UniqueKey.lazy_path m.m_extra.m_file); pmin = 0; pmax = 0 } in
470+
Interp.compiler_error (make_error ~sub:[
471+
make_error ~depth:1 (Custom "Previously defined here") pos
472+
] (Custom (Printf.sprintf "Cannot redefine module %s" (s_type_path mpath))) p);
473+
with Not_found ->
474+
ctx.com.cs#taint_module mpath DefineType;
475+
let mdep = Option.map_default (fun s -> TypeloadModule.load_module ~origin:MDepFromMacro ctx (parse_path s) pos) ctx.m.curmod mdep in
476+
let mnew = TypeloadModule.type_module ctx.com ctx.g ~dont_check_path:(has_native_meta) mpath (ctx.com.file_keys#generate_virtual mpath ctx.com.compilation_step) [tdef,pos] pos in
477+
mnew.m_extra.m_kind <- if is_macro then MMacro else MFake;
478+
add_dependency mnew mdep MDepFromMacro;
479+
add_dependency mdep mnew MDepFromMacroDefine;
480+
ctx.com.module_nonexistent_lut#clear;
481+
in
474482
add false ctx;
475483
(* if we are adding a class which has a macro field, we also have to add it to the macro context (issue #1497) *)
476484
if not ctx.com.is_macro_context then match tdef with
@@ -496,12 +504,13 @@ let make_macro_api ctx mctx p =
496504
let m = ctx.com.module_lut#find mpath in
497505
if m != ctx.m.curmod then begin
498506
let pos = { pfile = (Path.UniqueKey.lazy_path m.m_extra.m_file); pmin = 0; pmax = 0 } in
499-
raise_typing_error_ext (make_error ~sub:[
507+
Interp.compiler_error (make_error ~sub:[
500508
make_error ~depth:1 (Custom "Previously defined here") pos
501509
] (Custom (Printf.sprintf "Cannot redefine module %s" (s_type_path mpath))) p);
502510
end else
503-
ignore(TypeloadModule.type_types_into_module ctx.com ctx.g m types pos)
511+
ignore(TypeloadModule.type_types_into_module ctx.com ctx.g ctx.m.curmod types pos)
504512
with Not_found ->
513+
ctx.com.cs#taint_module mpath DefineModule;
505514
let mnew = TypeloadModule.type_module ctx.com ctx.g mpath (ctx.com.file_keys#generate_virtual mpath ctx.com.compilation_step) types pos in
506515
mnew.m_extra.m_kind <- MFake;
507516
add_dependency mnew ctx.m.curmod MDepFromMacro;

‎std/haxe/macro/CompilationServer.hx

+11
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,17 @@ class CompilationServer {
6969
});
7070
}
7171

72+
/**
73+
Invalidates a module, removing it from the cache.
74+
75+
If the module has already been loaded in current context, a
76+
`haxe.macro.Expr.Error` compiler error will be raised which can be
77+
caught using `try ... catch`.
78+
**/
79+
static public function invalidateModule(path:String) {
80+
@:privateAccess Compiler.load("server_invalidate_module", 1)(path);
81+
}
82+
7283
/**
7384
Invalidates all files given in `filePaths`, removing them from the cache.
7485
**/

‎std/haxe/macro/Context.hx

+8
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,10 @@ class Context {
663663
/**
664664
Defines a new type from `TypeDefinition` `t`.
665665
666+
If a matching module has already been loaded in current context, a
667+
`haxe.macro.Expr.Error` compiler error will be raised which can be
668+
caught using `try ... catch`.
669+
666670
If `moduleDependency` is given and is not `null`, it should contain
667671
a module path that will be used as a dependency for the newly defined module
668672
instead of the current module.
@@ -695,6 +699,10 @@ class Context {
695699
Defines a new module as `modulePath` with several `TypeDefinition`
696700
`types`. This is analogous to defining a .hx file.
697701
702+
If a matching module has already been loaded in current context, a
703+
`haxe.macro.Expr.Error` compiler error will be raised which can be
704+
caught using `try ... catch`.
705+
698706
The individual `types` can reference each other and any identifier
699707
respects the `imports` and `usings` as usual, expect that imports are
700708
not allowed to have `.*` wildcards or `as s` shorthands.

‎tests/server/src/cases/issues/Issue12001.hx

+129
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package cases.issues;
2+
3+
import utest.Async;
4+
5+
class Issue12001 extends TestCase {
6+
function testDefineType(_) {
7+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
8+
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
9+
var args = ["-main", "Empty", "--macro", "Macro.defineType()"];
10+
runHaxe(args);
11+
assertSuccess();
12+
13+
// Nothing is loading Foo, so no redefinition issue
14+
runHaxe(args);
15+
assertSuccess();
16+
}
17+
18+
function testRedefineTypeCatchError(_) {
19+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
20+
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
21+
var args = ["-main", "Empty", "--macro", "Macro.redefineTypeCatchError()"];
22+
runHaxe(args);
23+
assertSuccess();
24+
25+
runHaxe(args);
26+
assertSuccess();
27+
assertHasPrint("Macro.hx:56: TInst(Foobar,[])");
28+
assertHasPrint("Macro.hx:69: Cannot redefine module Foobar");
29+
}
30+
31+
@:async
32+
@:timeout(3000)
33+
function testRedefineType(async:Async) {
34+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
35+
vfs.putContent("Main.hx", getTemplate("issues/Issue12001/Main.hx"));
36+
var args = ["-main", "Main", "--interp", "--macro", "Macro.defineType()"];
37+
var i = 0;
38+
function test() {
39+
// Was failing with nightlies (HxbFailure)
40+
runHaxe(args, () -> {
41+
assertSuccess();
42+
assertHasPrint("Foo.test() = " + i);
43+
if (++i >= 5) async.done();
44+
else test();
45+
});
46+
}
47+
test();
48+
}
49+
50+
function testDefineModule(_) {
51+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
52+
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
53+
var args = ["-main", "Empty", "--macro", "Macro.defineModule()"];
54+
runHaxe(args);
55+
assertSuccess();
56+
57+
// Nothing is loading Bar, so no redefinition issue
58+
runHaxe(args);
59+
assertSuccess();
60+
}
61+
62+
function testRedefineModuleCatchError(_) {
63+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
64+
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
65+
var args = ["-main", "Empty", "--macro", "Macro.redefineModuleCatchError()"];
66+
runHaxe(args);
67+
assertSuccess();
68+
69+
runHaxe(args);
70+
assertSuccess();
71+
assertHasPrint("Macro.hx:77: TInst(Foobaz,[])");
72+
assertHasPrint("Macro.hx:90: Cannot redefine module Foobaz");
73+
}
74+
75+
@:async
76+
@:timeout(3000)
77+
function testRedefineModule(async:Async) {
78+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
79+
vfs.putContent("Main.hx", getTemplate("issues/Issue12001/Main1.hx"));
80+
var args = ["-main", "Main", "--interp", "--macro", "Macro.defineModule()"];
81+
var i = 0;
82+
function test() {
83+
// Was failing with nightlies (HxbFailure)
84+
runHaxe(args, () -> {
85+
assertSuccess();
86+
assertHasPrint("Bar.test() = " + i);
87+
if (++i >= 5) async.done();
88+
else test();
89+
});
90+
}
91+
test();
92+
}
93+
94+
@:async
95+
@:timeout(3000)
96+
function testRedefineAfterTyping(async:Async) {
97+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
98+
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
99+
var args = ["-main", "Empty", "--interp", "--macro", "Macro.hookRedefine()"];
100+
var i = 0;
101+
function test() {
102+
runHaxe(args, () -> {
103+
assertSuccess();
104+
// Newest version is being included
105+
assertHasPrint("Baz.test() = " + i);
106+
if (++i >= 5) async.done();
107+
else test();
108+
});
109+
}
110+
test();
111+
}
112+
113+
function testInvalidateError(_) {
114+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro1.hx"));
115+
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
116+
var args = ["-main", "Empty", "--interp", "--macro", "Macro.hookInvalidateError()"];
117+
runHaxe(args);
118+
assertErrorMessage("Cannot invalidate loaded module Empty");
119+
}
120+
121+
function testInvalidateCaughtError(_) {
122+
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro1.hx"));
123+
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
124+
var args = ["-main", "Empty", "--interp", "--macro", "Macro.hookInvalidateCatch()"];
125+
runHaxe(args);
126+
assertSuccess();
127+
assertHasPrint("Cannot invalidate loaded module Empty");
128+
}
129+
}

‎tests/server/test/templates/csSafeTypeBuilding/Macro.macro.hx

+5-13
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,19 @@ class Macro {
1111

1212
@:persistent static var generated = new Map<String, Bool>();
1313

14-
#if config.getType
15-
static function isAlive(name:String):Bool {
14+
static function isAlive(name:String, ct:ComplexType, pos:Position):Bool {
1615
// Null check is just there to make it a one liner
1716
// Basically returning true if no exception is caught
17+
#if config.getType
1818
return try Context.getType(name) != null
1919
catch(s:String) {
2020
if (s != 'Type not found \'$name\'') throw s;
2121
false;
2222
};
23-
}
24-
#else
25-
static function isAlive(ct:ComplexType, pos:Position):Bool {
26-
// Null check is just there to make it a one liner
27-
// Basically returning true if no exception is caught
23+
#else
2824
return try Context.resolveType(ct, pos) != null catch(e) false;
25+
#end
2926
}
30-
#end
3127

3228
public static function buildFoo() {
3329
var from = '[${Context.getLocalModule()}] ';
@@ -41,11 +37,7 @@ class Macro {
4137
var ct = TPath({pack: [], name: key});
4238

4339
if (generated.exists(key)) {
44-
#if config.getType
45-
if (isAlive(key)) {
46-
#else
47-
if (isAlive(ct, pos)) {
48-
#end
40+
if (isAlive(key, ct, pos)) {
4941
print('Reusing previously generated type for $key.');
5042
return ct;
5143
}

0 commit comments

Comments
 (0)