Browse Source

remove Lexer fmt_string mapping and process strings based on their single-quotedness (#9410)

* remove Lexer fmt_string mapping and process strings based on their single-quotedness

* simplify things

* minor

* remove is_fmt_string

* remove unnecessary deprecation

* recursively disable string formatting for macro-argument constants (unbreak #7688)

* add test for #1284

* add test (closes #8966)
Dan Korostelev 5 years ago
parent
commit
5cd188dd4c

+ 1 - 1
src/context/display/display.ml

@@ -183,7 +183,7 @@ module ExprPreprocessing = struct
 				raise Exit
 				raise Exit
 			| EMeta((Meta.Markup,_,_),(EConst(String _),p)) when is_annotated p ->
 			| EMeta((Meta.Markup,_,_),(EConst(String _),p)) when is_annotated p ->
 				annotate_marked e
 				annotate_marked e
-			| EConst (String _) when (not (Lexer.is_fmt_string (pos e)) || !Parser.was_auto_triggered) && is_annotated (pos e) && is_completion ->
+			| EConst (String (_,q)) when ((q <> SSingleQuotes) || !Parser.was_auto_triggered) && is_annotated (pos e) && is_completion ->
 				(* TODO: check if this makes any sense *)
 				(* TODO: check if this makes any sense *)
 				raise Exit
 				raise Exit
 			| EConst(Regexp _) when is_annotated (pos e) && is_completion ->
 			| EConst(Regexp _) when is_annotated (pos e) && is_completion ->

+ 3 - 0
src/core/texpr.ml

@@ -593,6 +593,9 @@ let rec type_constant_value basic (e,p) =
 	| _ ->
 	| _ ->
 		error "Constant value expected" p
 		error "Constant value expected" p
 
 
+let is_constant_value basic e = 
+	try (ignore (type_constant_value basic e); true) with Error (Custom _,_) -> false
+
 let for_remap basic v e1 e2 p =
 let for_remap basic v e1 e2 p =
 	let v' = alloc_var v.v_kind v.v_name e1.etype e1.epos in
 	let v' = alloc_var v.v_kind v.v_name e1.etype e1.epos in
 	let ev' = mk (TLocal v') e1.etype e1.epos in
 	let ev' = mk (TLocal v') e1.etype e1.epos in

+ 0 - 3
src/macro/macroApi.ml

@@ -1658,9 +1658,6 @@ let macro_api ccom get_api =
 			let f = if decode_opt_bool b then Type.s_expr_pretty false "" false else Type.s_expr_ast true "" in
 			let f = if decode_opt_bool b then Type.s_expr_pretty false "" false else Type.s_expr_ast true "" in
 			encode_string (f (Type.s_type (print_context())) (decode_texpr v))
 			encode_string (f (Type.s_type (print_context())) (decode_texpr v))
 		);
 		);
-		"is_fmt_string", vfun1 (fun p ->
-			vbool (Lexer.is_fmt_string (decode_pos p))
-		);
 		"format_string", vfun2 (fun s p ->
 		"format_string", vfun2 (fun s p ->
 			encode_expr ((get_api()).format_string (decode_string s) (decode_pos p))
 			encode_expr ((get_api()).format_string (decode_string s) (decode_pos p))
 		);
 		);

+ 2 - 38
src/syntax/lexer.ml

@@ -58,7 +58,6 @@ type lexer_file = {
 	mutable lmaxline : int;
 	mutable lmaxline : int;
 	mutable llines : (int * int) list;
 	mutable llines : (int * int) list;
 	mutable lalines : (int * int) array;
 	mutable lalines : (int * int) array;
-	mutable lstrings : int list;
 	mutable llast : int;
 	mutable llast : int;
 	mutable llastindex : int;
 	mutable llastindex : int;
 }
 }
@@ -70,7 +69,6 @@ let make_file file =
 		lmaxline = 1;
 		lmaxline = 1;
 		llines = [0,1];
 		llines = [0,1];
 		lalines = [|0,1|];
 		lalines = [|0,1|];
-		lstrings = [];
 		llast = max_int;
 		llast = max_int;
 		llastindex = 0;
 		llastindex = 0;
 	}
 	}
@@ -129,37 +127,6 @@ let newline lexbuf =
 	cur.lline <- cur.lline + 1;
 	cur.lline <- cur.lline + 1;
 	cur.llines <- (lexeme_end lexbuf,cur.lline) :: cur.llines
 	cur.llines <- (lexeme_end lexbuf,cur.lline) :: cur.llines
 
 
-let fmt_pos p =
-	p.pmin + (p.pmax - p.pmin) * 1000000
-
-let add_fmt_string p =
-	let file = (try
-		Hashtbl.find all_files p.pfile
-	with Not_found ->
-		let f = make_file p.pfile in
-		Hashtbl.replace all_files p.pfile f;
-		f
-	) in
-	file.lstrings <- (fmt_pos p) :: file.lstrings
-
-let fast_add_fmt_string p =
-	let cur = !cur in
-	cur.lstrings <- (fmt_pos p) :: cur.lstrings
-
-let is_fmt_string p =
-	try
-		let file = Hashtbl.find all_files p.pfile in
-		List.mem (fmt_pos p) file.lstrings
-	with Not_found ->
-		false
-
-let remove_fmt_string p =
-	try
-		let file = Hashtbl.find all_files p.pfile in
-		file.lstrings <- List.filter ((<>) (fmt_pos p)) file.lstrings
-	with Not_found ->
-		()
-
 let find_line p f =
 let find_line p f =
 	(* rebuild cache if we have a new line *)
 	(* rebuild cache if we have a new line *)
 	if f.lmaxline <> f.lline then begin
 	if f.lmaxline <> f.lline then begin
@@ -417,9 +384,7 @@ let rec token lexbuf =
 		let pmin = lexeme_start lexbuf in
 		let pmin = lexeme_start lexbuf in
 		let pmax = (try string2 lexbuf with Exit -> error Unterminated_string pmin) in
 		let pmax = (try string2 lexbuf with Exit -> error Unterminated_string pmin) in
 		let str = (try unescape (contents()) with Invalid_escape_sequence(c,i,msg) -> error (Invalid_escape (c,msg)) (pmin + i)) in
 		let str = (try unescape (contents()) with Invalid_escape_sequence(c,i,msg) -> error (Invalid_escape (c,msg)) (pmin + i)) in
-		let t = mk_tok (Const (String(str,SSingleQuotes))) pmin pmax in
-		fast_add_fmt_string (snd t);
-		t
+		mk_tok (Const (String(str,SSingleQuotes))) pmin pmax;
 	| "~/" ->
 	| "~/" ->
 		reset();
 		reset();
 		let pmin = lexeme_start lexbuf in
 		let pmin = lexeme_start lexbuf in
@@ -544,9 +509,8 @@ and code_string lexbuf open_braces =
 	| "'" ->
 	| "'" ->
 		add "'";
 		add "'";
 		let pmin = lexeme_start lexbuf in
 		let pmin = lexeme_start lexbuf in
-		let pmax = (try string2 lexbuf with Exit -> error Unterminated_string pmin) in
+		(try ignore(string2 lexbuf) with Exit -> error Unterminated_string pmin);
 		add "'";
 		add "'";
-		fast_add_fmt_string { pfile = !cur.lfile; pmin = pmin; pmax = pmax };
 		code_string lexbuf open_braces
 		code_string lexbuf open_braces
 	| "/*" ->
 	| "/*" ->
 		let pmin = lexeme_start lexbuf in
 		let pmin = lexeme_start lexbuf in

+ 14 - 20
src/typing/macroContext.ml

@@ -644,7 +644,6 @@ let type_macro ctx mode cpath f (el:Ast.expr list) p =
 		| _ ->
 		| _ ->
 			el,[]
 			el,[]
 	in
 	in
-	let todo = ref [] in
 	let args =
 	let args =
 		(*
 		(*
 			force default parameter types to haxe.macro.Expr, and if success allow to pass any value type since it will be encoded
 			force default parameter types to haxe.macro.Expr, and if success allow to pass any value type since it will be encoded
@@ -664,29 +663,24 @@ let type_macro ctx mode cpath f (el:Ast.expr list) p =
 		let index = ref (-1) in
 		let index = ref (-1) in
 		let constants = List.map (fun e ->
 		let constants = List.map (fun e ->
 			let p = snd e in
 			let p = snd e in
-			let e = (try
-				let e' = Texpr.type_constant_value ctx.com.basic e in
-				let rec loop e = match e with
-					| { eexpr = TConst (TString _); epos = p } when Lexer.is_fmt_string p ->
-						Lexer.remove_fmt_string p;
-						todo := (fun() -> Lexer.add_fmt_string p) :: !todo;
-					| _ -> Type.iter loop e
-				in
-				loop e';
-				e
-			with Error (Custom _,_) ->
-				(* if it's not a constant, let's make something that is typed as haxe.macro.Expr - for nice error reporting *)
-				(EBlock [
-					(EVars [("__tmp",null_pos),false,Some (CTPath ctexpr,p),Some (EConst (Ident "null"),p)],p);
-					(EConst (Ident "__tmp"),p);
-				],p)
-			) in
+			let e =
+				if Texpr.is_constant_value ctx.com.basic e then
+					(* temporarily disable format strings processing for macro call argument typing since we want to pass raw constants *)
+					let rec loop e =
+						match e with
+						| (EConst (String (s,SSingleQuotes)),p) -> (EConst (String (s,SDoubleQuotes)), p)
+						| _ -> Ast.map_expr loop e
+					in
+					loop e
+				else
+					(* if it's not a constant, let's make something that is typed as haxe.macro.Expr - for nice error reporting *)
+					(ECheckType ((EConst (Ident "null"),p), (CTPath ctexpr,p)), p)
+			in
 			(* let's track the index by doing [e][index] (we will keep the expression type this way) *)
 			(* let's track the index by doing [e][index] (we will keep the expression type this way) *)
 			incr index;
 			incr index;
 			(EArray ((EArrayDecl [e],p),(EConst (Int (string_of_int (!index))),p)),p)
 			(EArray ((EArrayDecl [e],p),(EConst (Int (string_of_int (!index))),p)),p)
 		) el in
 		) el in
-		let elt, _ = try Calls.unify_call_args mctx constants (List.map fst eargs) t_dynamic p false false with e -> List.iter (fun f -> f()) (!todo); raise e; in
-		List.iter (fun f -> f()) (!todo);
+		let elt = fst (Calls.unify_call_args mctx constants (List.map fst eargs) t_dynamic p false false) in
 		List.map2 (fun (_,mct) e ->
 		List.map2 (fun (_,mct) e ->
 			let e, et = (match e.eexpr with
 			let e, et = (match e.eexpr with
 				(* get back our index and real expression *)
 				(* get back our index and real expression *)

+ 1 - 1
src/typing/typer.ml

@@ -2486,7 +2486,7 @@ and type_expr ?(mode=MGet) ctx (e,p) (with_type:WithType.t) =
 		let opt = mk (TConst (TString opt)) ctx.t.tstring p in
 		let opt = mk (TConst (TString opt)) ctx.t.tstring p in
 		let t = Typeload.load_core_type ctx "EReg" in
 		let t = Typeload.load_core_type ctx "EReg" in
 		mk (TNew ((match t with TInst (c,[]) -> c | _ -> die "" __LOC__),[],[str;opt])) t p
 		mk (TNew ((match t with TInst (c,[]) -> c | _ -> die "" __LOC__),[],[str;opt])) t p
-	| EConst (String(s,_)) when s <> "" && Lexer.is_fmt_string p ->
+	| EConst (String(s,SSingleQuotes)) when s <> "" ->
 		type_expr ctx (format_string ctx s p) with_type
 		type_expr ctx (format_string ctx s p) with_type
 	| EConst c ->
 	| EConst c ->
 		Texpr.type_constant ctx.com.basic c p
 		Texpr.type_constant ctx.com.basic c p

+ 1 - 17
std/haxe/macro/MacroStringTools.hx

@@ -48,26 +48,10 @@ class MacroStringTools {
 	/**
 	/**
 		Tells if `e` is a format string, i.e. uses single quotes `'` as
 		Tells if `e` is a format string, i.e. uses single quotes `'` as
 		delimiters.
 		delimiters.
-
-		This only works if `e` has a position which the compiler can find. While
-		this is true for any expressions appearing in real Haxe code (i.e. some
-		.hx file), it might not work for expressions generated by macros.
-
-		This operation depends on the position of `e`.
 	**/
 	**/
 	static public function isFormatExpr(e:ExprOf<String>):Bool {
 	static public function isFormatExpr(e:ExprOf<String>):Bool {
-		#if (neko || eval)
-		return Context.load("is_fmt_string", 1)(e.pos);
-		#else
-		return isFmtString(e.pos);
-		#end
+		return e.expr.match(EConst(CString(_, SingleQuotes)));
 	}
 	}
-
-	#if !neko
-	static function isFmtString(p:Position):Bool {
-		return false;
-	}
-	#end
 	#end
 	#end
 
 
 	/**
 	/**

+ 2 - 4
std/haxe/macro/Printer.hx

@@ -97,7 +97,8 @@ class Printer {
 
 
 	public function printConstant(c:Constant)
 	public function printConstant(c:Constant)
 		return switch (c) {
 		return switch (c) {
-			case CString(s): printString(s);
+			case CString(s, SingleQuotes): printFormatString(s);
+			case CString(s, _): printString(s);
 			case CIdent(s), CInt(s), CFloat(s):
 			case CIdent(s), CInt(s), CFloat(s):
 				s;
 				s;
 			case CRegexp(s, opt): '~/$s/$opt';
 			case CRegexp(s, opt): '~/$s/$opt';
@@ -219,9 +220,6 @@ class Printer {
 
 
 	public function printExpr(e:Expr)
 	public function printExpr(e:Expr)
 		return e == null ? "#NULL" : switch (e.expr) {
 		return e == null ? "#NULL" : switch (e.expr) {
-			#if macro
-			case EConst(CString(s)): haxe.macro.MacroStringTools.isFormatExpr(e) ? printFormatString(s) : printString(s);
-			#end
 			case EConst(c): printConstant(c);
 			case EConst(c): printConstant(c);
 			case EArray(e1, e2): '${printExpr(e1)}[${printExpr(e2)}]';
 			case EArray(e1, e2): '${printExpr(e1)}[${printExpr(e2)}]';
 			case EBinop(op, e1, e2): '${printExpr(e1)} ${printBinop(op)} ${printExpr(e2)}';
 			case EBinop(op, e1, e2): '${printExpr(e1)} ${printBinop(op)} ${printExpr(e2)}';

+ 24 - 0
tests/unit/src/unit/issues/Issue1284.hx

@@ -0,0 +1,24 @@
+package unit.issues;
+
+class Issue1284 extends Test {
+	function test() {
+		m1('test$id', ['test$id'], {'test$id': 'test$id'});
+		m2('test$id', ['test$id'], {'test$id': 'test$id'});
+	}
+
+	static function check(b, p) if (!b) throw new haxe.macro.Expr.Error("assertion failed", p);
+
+	macro static function m1(a, b, c) {
+		check(a.expr.match(EConst(CString("test$id", SingleQuotes))), a.pos);
+		check(b.expr.match(EArrayDecl([{expr: EConst(CString("test$id", SingleQuotes))}])), b.pos);
+		check(c.expr.match(EObjectDecl([{field: "test$id", expr: {expr: EConst(CString("test$id", SingleQuotes))}}])), c.pos);
+		return macro utest.Assert.pass();
+	}
+
+	macro static function m2(a:String, b:Array<String>, c:{}) {
+		check(a == "test$id", (macro _).pos);
+		check(b[0] == "test$id", (macro _).pos);
+		check(Reflect.field(c, "test$id") == "test$id", (macro _).pos);
+		return macro utest.Assert.pass();
+	}
+}

+ 22 - 0
tests/unit/src/unit/issues/Issue8966.hx

@@ -0,0 +1,22 @@
+package unit.issues;
+
+#if macro
+import haxe.macro.Expr;
+import haxe.macro.ExprTools;
+import haxe.macro.MacroStringTools;
+#end
+
+class Issue8966 extends Test {
+	function test() {
+		var i = 10;
+		eq("foo10", m('foo$i'));
+	}
+
+	static macro function m(e) {
+		e.pos = (macro here).pos;
+		return macro {
+			t($v{MacroStringTools.isFormatExpr(e)});
+			$e;
+		};
+	}
+}