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

[macro] fix redefinition of modules with defineType/defineModule #12001

Open
wants to merge 15 commits into
base: development
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions src/compiler/compilationCache.ml
Original file line number Diff line number Diff line change
Expand Up @@ -234,15 +234,27 @@ class cache = object(self)
) cc#get_modules acc
) contexts []

method taint_module m_path reason =
Hashtbl.iter (fun _ cc ->
Hashtbl.iter (fun _ m ->
if m.m_path = m_path then m.m_extra.m_cache_state <- MSBad (Tainted reason)
) cc#get_modules;
Hashtbl.iter (fun _ mc ->
if mc.HxbData.mc_path = m_path then
mc.HxbData.mc_extra.m_cache_state <- match reason, mc.mc_extra.m_cache_state with
| CheckDisplayFile, (MSBad _ as state) -> state
| _ -> MSBad (Tainted reason)
) cc#get_hxb
) contexts

method taint_modules file_key reason =
Hashtbl.iter (fun _ cc ->
Hashtbl.iter (fun _ m ->
if Path.UniqueKey.lazy_key m.m_extra.m_file = file_key then m.m_extra.m_cache_state <- MSBad (Tainted reason)
) cc#get_modules;
let open HxbData in
Hashtbl.iter (fun _ mc ->
if Path.UniqueKey.lazy_key mc.mc_extra.m_file = file_key then
mc.mc_extra.m_cache_state <- match reason, mc.mc_extra.m_cache_state with
if Path.UniqueKey.lazy_key mc.HxbData.mc_extra.m_file = file_key then
mc.HxbData.mc_extra.m_cache_state <- match reason, mc.HxbData.mc_extra.m_cache_state with
| CheckDisplayFile, (MSBad _ as state) -> state
| _ -> MSBad (Tainted reason)
) cc#get_hxb
Expand Down
3 changes: 3 additions & 0 deletions src/core/tPrinting.ml
Original file line number Diff line number Diff line change
Expand Up @@ -637,8 +637,11 @@ module Printer = struct

let s_module_tainting_reason = function
| CheckDisplayFile -> "check_display_file"
| DefineType -> "define_type"
| DefineModule -> "define_module"
| ServerInvalidate -> "server/invalidate"
| ServerInvalidateFiles -> "server_invalidate_files"
| ServerInvalidateModule -> "server_invalidate_module"

let s_module_skip_reason reason =
let rec loop stack = function
Expand Down
3 changes: 3 additions & 0 deletions src/core/tType.ml
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ type module_check_policy =

type module_tainting_reason =
| CheckDisplayFile
| DefineType
| DefineModule
| ServerInvalidate
| ServerInvalidateFiles
| ServerInvalidateModule

type module_skip_reason =
| DependencyDirty of path * module_skip_reason
Expand Down
12 changes: 12 additions & 0 deletions src/macro/macroApi.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2323,6 +2323,18 @@ let macro_api ccom get_api =
(get_api()).add_module_check_policy filter policy (decode_bool recursive);
vnull
);
"server_invalidate_module", vfun1 (fun p ->
let mpath = parse_path (decode_string p) in
let com = ccom() in
(try
ignore(com.module_lut#find mpath);
let msg = "Cannot invalidate loaded module " ^ (s_type_path mpath) in
let pos = get_api_call_pos() in
compiler_error (Error.make_error (Custom msg) pos)
with Not_found ->
com.cs#taint_module mpath ServerInvalidateModule);
vnull
);
"server_invalidate_files", vfun1 (fun a ->
let com = ccom() in
let cs = com.cs in
Expand Down
25 changes: 17 additions & 8 deletions src/typing/macroContext.ml
Original file line number Diff line number Diff line change
Expand Up @@ -464,13 +464,21 @@ let make_macro_api ctx mctx p =
| _ -> false
in
let add is_macro ctx =
let mdep = Option.map_default (fun s -> TypeloadModule.load_module ~origin:MDepFromMacro ctx (parse_path s) pos) ctx.m.curmod mdep in
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
mnew.m_extra.m_kind <- if is_macro then MMacro else MFake;
add_dependency mnew mdep MDepFromMacro;
add_dependency mdep mnew MDepFromMacroDefine;
ctx.com.module_nonexistent_lut#clear;
in
try
let m = ctx.com.module_lut#find mpath in
let pos = { pfile = (Path.UniqueKey.lazy_path m.m_extra.m_file); pmin = 0; pmax = 0 } in
raise_typing_error_ext (make_error ~sub:[
make_error ~depth:1 (Custom "Previously defined here") pos
] (Custom (Printf.sprintf "Cannot redefine module %s" (s_type_path mpath))) p);
with Not_found ->
ctx.com.cs#taint_module mpath DefineType;
let mdep = Option.map_default (fun s -> TypeloadModule.load_module ~origin:MDepFromMacro ctx (parse_path s) pos) ctx.m.curmod mdep in
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
mnew.m_extra.m_kind <- if is_macro then MMacro else MFake;
add_dependency mnew mdep MDepFromMacro;
add_dependency mdep mnew MDepFromMacroDefine;
ctx.com.module_nonexistent_lut#clear;
in
add false ctx;
(* if we are adding a class which has a macro field, we also have to add it to the macro context (issue #1497) *)
if not ctx.com.is_macro_context then match tdef with
Expand Down Expand Up @@ -500,8 +508,9 @@ let make_macro_api ctx mctx p =
make_error ~depth:1 (Custom "Previously defined here") pos
] (Custom (Printf.sprintf "Cannot redefine module %s" (s_type_path mpath))) p);
end else
ignore(TypeloadModule.type_types_into_module ctx.com ctx.g m types pos)
ignore(TypeloadModule.type_types_into_module ctx.com ctx.g ctx.m.curmod types pos)
with Not_found ->
ctx.com.cs#taint_module mpath DefineModule;
let mnew = TypeloadModule.type_module ctx.com ctx.g mpath (ctx.com.file_keys#generate_virtual mpath ctx.com.compilation_step) types pos in
mnew.m_extra.m_kind <- MFake;
add_dependency mnew ctx.m.curmod MDepFromMacro;
Expand Down
10 changes: 10 additions & 0 deletions std/haxe/macro/CompilationServer.hx
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ class CompilationServer {
});
}

/**
Invalidates a module, removing it from the cache.

If the module has already been loaded in current context, a compiler
error will be raised which can be caught using `try ... catch`.
**/
static public function invalidateModule(path:String) {
@:privateAccess Compiler.load("server_invalidate_module", 1)(path);
}

/**
Invalidates all files given in `filePaths`, removing them from the cache.
**/
Expand Down
103 changes: 103 additions & 0 deletions tests/server/src/cases/issues/Issue12001.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package cases.issues;

import utest.Async;

class Issue12001 extends TestCase {
function testDefineType(_) {
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
var args = ["-main", "Empty", "--macro", "Macro.defineType()"];
runHaxe(args);
assertSuccess();

// Nothing is loading Foo, so no redefinition issue
runHaxe(args);
assertSuccess();
}

@:async
@:timeout(3000)
function testRedefineType(async:Async) {
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
vfs.putContent("Main.hx", getTemplate("issues/Issue12001/Main.hx"));
var args = ["-main", "Main", "--interp", "--macro", "Macro.defineType()"];
var i = 0;
function test() {
// Was failing with nightlies (HxbFailure)
runHaxe(args, () -> {
assertSuccess();
assertHasPrint("Foo.test() = " + i);
if (++i >= 5) async.done();
else test();
});
}
test();
}

function testDefineModule(_) {
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
var args = ["-main", "Empty", "--macro", "Macro.defineModule()"];
runHaxe(args);
assertSuccess();

// Nothing is loading Bar, so no redefinition issue
runHaxe(args);
assertSuccess();
}

@:async
@:timeout(3000)
function testRedefineModule(async:Async) {
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
vfs.putContent("Main.hx", getTemplate("issues/Issue12001/Main1.hx"));
var args = ["-main", "Main", "--interp", "--macro", "Macro.defineModule()"];
var i = 0;
function test() {
// Was failing with nightlies (HxbFailure)
runHaxe(args, () -> {
assertSuccess();
assertHasPrint("Bar.test() = " + i);
if (++i >= 5) async.done();
else test();
});
}
test();
}

@:async
@:timeout(3000)
function testRedefineAfterTyping(async:Async) {
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro.hx"));
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
var args = ["-main", "Empty", "--interp", "--macro", "Macro.hookRedefine()"];
var i = 0;
function test() {
runHaxe(args, () -> {
assertSuccess();
// Newest version is being included
assertHasPrint("Baz.test() = " + i);
if (++i >= 5) async.done();
else test();
});
}
test();
}

function testInvalidateError(_) {
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro1.hx"));
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
var args = ["-main", "Empty", "--interp", "--macro", "Macro.hookInvalidateError()"];
runHaxe(args);
assertErrorMessage("Cannot invalidate loaded module Empty");
}

function testInvalidateCaughtError(_) {
vfs.putContent("Macro.hx", getTemplate("issues/Issue12001/Macro1.hx"));
vfs.putContent("Empty.hx", getTemplate("Empty.hx"));
var args = ["-main", "Empty", "--interp", "--macro", "Macro.hookInvalidateCatch()"];
runHaxe(args);
assertSuccess();
assertHasPrint("Cannot invalidate loaded module Empty");
}
}
18 changes: 5 additions & 13 deletions tests/server/test/templates/csSafeTypeBuilding/Macro.macro.hx
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,19 @@ class Macro {

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

#if config.getType
static function isAlive(name:String):Bool {
static function isAlive(name:String, ct:ComplexType, pos:Position):Bool {
// Null check is just there to make it a one liner
// Basically returning true if no exception is caught
#if config.getType
return try Context.getType(name) != null
catch(s:String) {
if (s != 'Type not found \'$name\'') throw s;
false;
};
}
#else
static function isAlive(ct:ComplexType, pos:Position):Bool {
// Null check is just there to make it a one liner
// Basically returning true if no exception is caught
#else
return try Context.resolveType(ct, pos) != null catch(e) false;
#end
}
#end

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

if (generated.exists(key)) {
#if config.getType
if (isAlive(key)) {
#else
if (isAlive(ct, pos)) {
#end
if (isAlive(key, ct, pos)) {
print('Reusing previously generated type for $key.');
return ct;
}
Expand Down
50 changes: 50 additions & 0 deletions tests/server/test/templates/issues/Issue12001/Macro.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import haxe.macro.Context;

@:persistent var i = 0;
function defineType() {
Context.onAfterInitMacros(() -> {
Context.defineType({
pos: Context.currentPos(),
pack: [],
name: "Foo",
kind: TDClass(null, null, false, false, false),
fields: (macro class Foo {
public static function test() Sys.println("Foo.test() = " + $v{i++});
}).fields
});
});
}

@:persistent var j = 0;
function defineModule() {
Context.onAfterInitMacros(() -> {
Context.defineModule("Bar", [{
pos: Context.currentPos(),
pack: [],
name: "Bar",
kind: TDClass(null, null, false, false, false),
fields: (macro class Bar {
public static function test() Sys.println("Bar.test() = " + $v{j++});
}).fields
}]);
});
}

@:persistent var k = 0;
function hookRedefine() {
var generated = false;
Context.onAfterTyping((_) -> {
if (generated) return;
generated = true;

Context.defineModule("Baz", [{
pos: Context.currentPos(),
pack: [],
name: "Baz",
kind: TDClass(null, null, false, false, false),
fields: (macro class Baz {
public static function __init__() Sys.println("Baz.test() = " + $v{k++});
}).fields
}]);
});
}
18 changes: 18 additions & 0 deletions tests/server/test/templates/issues/Issue12001/Macro1.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import haxe.macro.CompilationServer;
import haxe.macro.Context;

function hookInvalidateError() {
Context.onAfterTyping((_) -> {
CompilationServer.invalidateModule("Empty");
});
}

function hookInvalidateCatch() {
Context.onAfterTyping((_) -> {
try {
CompilationServer.invalidateModule("Empty");
} catch (e:Dynamic) {
Sys.println(Std.string(e));
}
});
}
3 changes: 3 additions & 0 deletions tests/server/test/templates/issues/Issue12001/Main.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function main() {
Foo.test();
}
3 changes: 3 additions & 0 deletions tests/server/test/templates/issues/Issue12001/Main1.hx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
function main() {
Bar.test();
}
Loading