addGlobalMetadata applied twice on abstracts

[AlexHaxe]

I've noticed that metadata added via addGlobalMetadata appears twice on abstracts with recent nightlies

steps to reproduce:
src/demo/Main.hx

package demo;

class Main {
	static function main() {
		var name:ImageName = "abc";
		trace(name);
	}
}

abstract ImageName(String) from String {
	public function new(name:String) {
		this = name;
	}
}

src/demo/Macro.hx

package demo;

#if macro
import haxe.macro.Compiler;
import haxe.macro.Context;
import haxe.macro.Expr;

class Macro {
	public static function initMacro() {
		Compiler.addGlobalMetadata("", "@:build(demo.Macro.instrumentFields())", true, true, false);
	}

	static function instrumentFields():Null<Array<Field>> {
		var fields:Array<Field> = Context.getBuildFields();
		for (field in fields) {
			switch (field.kind) {
				case FFun(f):
					if (f.expr == null) {
						continue;
					}
					switch (f.expr.expr) {
						case EBlock(exprs):
							exprs.unshift(macro trace($v{field.name}));
						case _:
					}
				case _:
			}
		}
		return fields;
	}
}
#end

compiling with:

-cp src
--macro demo.Macro.initMacro()
-D dump=pretty
--run demo.Main

results in dump/eval/demo/_Main/ImageName_Impl_.dump having two @:build… metadata and it has two traces added to it's constructor / all functions.

[kLabz]

Right, with recursive it's added to both abstract and impl, and since now we're forwarding those to impl from abstract, it ends up there twice..

[Simn]

I think we should not check global metadata for abstract implementation classes.

[kLabz]

That's an option, though I'm not exactly sure if it can lead to problems 🤔

Another option:

diff --git a/src/typing/typeloadModule.ml b/src/typing/typeloadModule.ml
index 701dd8913..2c9a88706 100644
--- a/src/typing/typeloadModule.ml
+++ b/src/typing/typeloadModule.ml
@@ -571,7 +571,7 @@ module TypeLevel = struct
                Option.may (fun c ->
                        List.iter (fun m -> match m with
                                | ((Meta.Using | Meta.Build | Meta.CoreApi | Meta.Allow | Meta.Access | Meta.Enum | Meta.Dce | Meta.Native | Meta.HlNative | Meta.JsRequire | Meta.PythonImport | Meta.Expose | Meta.Deprecated | Meta.PhpGlobal | Meta.PublicFields),_,_) ->
-                                       c.cl_meta <- m :: c.cl_meta;
+                                       if not (List.mem m c.cl_meta) then c.cl_meta <- m :: c.cl_meta;
                                | _ ->
                                        ()
                        ) a.a_meta;

This works here (for deduplication) because it's the exact same meta.

[AlexHaxe]

without recursive "" only matches top-level types. so if I want to apply metadata to everything I have to do recursive=true and ""

how would I check to avoid applying my modifications twice in case of abstracts?

[Simn]

I still think that if @:meta isn't moved to the abstract implementation class then the same should be true when adding it via addGlobalMetadata.

Also please don't List.mem complex structures, m refers to the full metadata entry here.

[kLabz]

Checking what CI thinks about it