v3: address post-merge review feedback

Author: medvednikov

ci/qemu_linux_tests.sh

@@ -119,10 +119,24 @@ fi
 ssh_options=(
 	-i "$key_file"
 	-p "$ssh_port"
-	-o "UserKnownHostsFile=${known_hosts_file}"
+	-o "UserKnownHostsFile=\"${known_hosts_file}\""
 	-o StrictHostKeyChecking=yes
 	-o ConnectTimeout=5
 )
+if [[ ! "$ssh_port" =~ ^[0-9]+$ ]]; then
+	echo "Invalid SSH port: ${ssh_port}" >&2
+	exit 1
+fi
+if [[ "$key_file" == *"'"* || "$known_hosts_file" == *"'"* || "$known_hosts_file" == *'"'* ]]; then
+	echo 'The QEMU SSH key and known-hosts paths cannot contain quote characters.' >&2
+	exit 1
+fi
+# openrsync understands quoted arguments but treats Bash `%q` backslashes
+# literally. Preserve spaces with single-quoted arguments, and retain inner
+# quotes around the known-hosts value for OpenSSH's own option parser.
+printf -v rsync_ssh_command \
+	"ssh -i '%s' -p '%s' '-oUserKnownHostsFile=\"%s\"' -o StrictHostKeyChecking=yes" \
+	"$key_file" "$ssh_port" "$known_hosts_file"
 
 vm_is_running() {
 	[[ -s "$pid_file" ]] && kill -0 "$(<"$pid_file")" 2>/dev/null
@@ -235,7 +249,7 @@ EOF
 		--from0 \
 		--files-from=- \
 		--exclude '.detect_tcc*' \
-		-e "ssh -i ${key_file} -p ${ssh_port} -o UserKnownHostsFile=${known_hosts_file} -o StrictHostKeyChecking=yes" \
+		-e "$rsync_ssh_command" \
 		"${repo_root}/" "${guest}:${guest_repo}/"
 	# Record only untracked paths that the rsync exclusion above allows through.
 	git -C "$repo_root" ls-files -z --others --exclude-standard \

cmd/v/macos_v3_args.c.v

@@ -79,6 +79,7 @@ fn macos_v3_explicit_compilation_requested(command string, prefs &pref.Preferenc
 	}
 	return command in ['run', 'build'] || prefs.is_script || os.is_dir(prefs.path)
 		|| normalized_path.ends_with('.v') || normalized_path.ends_with('.vsh')
+		|| normalized_path.ends_with('.vv')
 }
 
 @[markused]

cmd/v/macos_v3_test.v

@@ -2343,6 +2343,10 @@ fn test_explicit_v3_rejects_structured_v1_only_preferences() {
 		new_compiler: true
 		path:         'main.v'
 	})
+	assert macos_v3_explicit_compilation_requested('build', &pref.Preferences{
+		new_compiler: true
+		path:         'fixture.vv'
+	})
 	assert !macos_v3_explicit_compilation_requested('fmt', &pref.Preferences{
 		new_compiler: true
 		is_quiet:     true
@@ -2371,6 +2375,33 @@ fn test_explicit_v3_rejects_structured_v1_only_preferences() {
 	})
 }
 
+fn test_embedded_v3_explicit_vv_build_is_rejected() {
+	$if macos || linux {
+		root := os.join_path(os.real_path(os.vtmp_dir()), 'v3_explicit_vv_${os.getpid()}')
+		os.rmdir_all(root) or {}
+		os.mkdir_all(root) or { panic(err) }
+		defer {
+			os.rmdir_all(root) or {}
+		}
+		source := os.join_path(root, 'fixture.vv')
+		os.write_file(source, 'fn main() {}\n')!
+		mut environment := os.environ()
+		environment['VFLAGS'] = ''
+		environment['VOSARGS'] = ''
+		mut process := os.new_process(@VEXE)
+		process.set_args(['-new-compiler', source])
+		process.set_environment(environment)
+		process.set_redirect_stdio()
+		process.run()
+		process.wait()
+		output := process.stdout_slurp() + process.stderr_slurp()
+		exit_code := process.code
+		process.close()
+		assert exit_code == 1, output
+		assert output.contains('options that require the established compiler'), output
+	}
+}
+
 fn test_explicit_v3_options_do_not_reject_external_tools() {
 	root := os.join_path(os.real_path(os.vtmp_dir()), 'v3_external_tool_${os.getpid()}')
 	os.rmdir_all(root) or {}

doc/docs.md

@@ -79,9 +79,10 @@ On macOS and Linux, `v` compiles your program with the experimental **V3**
 compiler (a newer implementation of the V compiler, whose source lives in
 `vlib/v3`) by default. On other platforms, and for C builds that select a
 target OS different from the host, the established compiler in `vlib/v` is
-used. V scripts (`.vsh`, including `v run script.vsh`) and debug builds selected
-with `-g`/`-debug` also remain on the established compiler. Same-OS
-cross-architecture builds can still use V3 when the target is supported.
+used. V scripts (`.vsh`, including `v run script.vsh`), the `crun` and
+`build-module` commands, and debug builds selected with `-g`/`-debug` also
+remain on the established compiler. Same-OS cross-architecture builds can still
+use V3 when the target is supported.
 
 You normally do not need to do anything: when V3 cannot yet build an eligible
 program, `v` automatically falls back to the established compiler, so your build
@@ -118,9 +119,11 @@ An internal V3 compiler error on a short program (and any directory build such a
 file, so it can still upload a strict-subset excerpt of that file plus a few lines
 of context around the failing line, even when the file is short. Inline-assembly
 fallbacks are notice-only and do not submit a report; reporting is also skipped
-for test compilations and to the default endpoint in GitHub CI. A custom endpoint
-set with `-bug-report-url` or `V_C_ERROR_BUG_REPORT_URL` remains active in CI. You
-can turn reporting off entirely by setting `V_C_ERROR_BUG_REPORT_DISABLED=1`.
+for test compilations and to the default endpoint in GitHub CI. A custom fallback
+endpoint set with `V_C_ERROR_BUG_REPORT_URL` remains active in CI. The
+`-bug-report-url` option selects the established compiler and configures only its
+reports. You can turn reporting off entirely by setting
+`V_C_ERROR_BUG_REPORT_DISABLED=1`.
 
 ## Packaging V for distribution
 See the [notes on how to prepare a package for V](packaging_v_for_distributions.md) .

vlib/v/help/build/build.txt

@@ -27,11 +27,13 @@ The default compiler:
   such as `v .` submits metadata only, while a generated-C error maps to a V file
   and can still upload a strict-subset excerpt of it, even for a short file.
   Inline-assembly fallbacks are notice-only; test compilations and GitHub CI also
-  submit no report to the default endpoint. An explicitly configured custom bug
-  report URL remains active in CI. Pass `-old-compiler` (see `v help build-c`) to always use the
-  established compiler and skip the V3 attempt and its fallback reporting. The
-  established compiler can still report its own eligible generated-C failures;
-  set V_C_ERROR_BUG_REPORT_DISABLED=1 to disable all such submissions.
+  submit no report to the default endpoint. A custom fallback URL set with
+  V_C_ERROR_BUG_REPORT_URL remains active in CI. The -bug-report-url option
+  selects the established compiler and configures only its reports. Pass
+  `-old-compiler` (see `v help build-c`) to always use the established compiler
+  and skip the V3 attempt and its fallback reporting. The established compiler
+  can still report its own eligible generated-C failures; set
+  V_C_ERROR_BUG_REPORT_DISABLED=1 to disable all such submissions.
 
 Use `v build script.vsh` to compile a `.vsh` script without running it.
 `v -skip-running script.vsh` works too.

vlib/v3/gen/c/cleanc.v

@@ -4530,11 +4530,12 @@ fn (mut g FlatGen) collect_preserved_header_file(path string, include_dirs []str
 	}
 	g.preserved_header_files_seen[real_path] = true
 	text := os.read_file(real_path) or { return }
-	g.collect_inlined_c_structs(text)
-	g.collect_inlined_c_fns(text)
-	g.collect_inlined_c_declared_fns(text)
+	possible_text := c_header_possible_active_text(text, g.c_flags, g.c99_mode, g.target)
+	g.collect_inlined_c_structs(possible_text)
+	g.collect_inlined_c_fns(possible_text)
+	g.collect_inlined_c_declared_fns(possible_text)
 	mut in_block_comment := false
-	for line in text.split_into_lines() {
+	for line in possible_text.split_into_lines() {
 		clean, next_in_block_comment := c_preprocessor_directive_scan_line(line, in_block_comment)
 		in_block_comment = next_in_block_comment
 		if c_directive_name(clean) !in ['include', 'import'] {
@@ -4558,6 +4559,186 @@ fn (mut g FlatGen) collect_preserved_header_file(path string, include_dirs []str
 	}
 }
 
+// c_header_possible_active_text removes declarations from preprocessor branches
+// known to be inactive for the current target and C flags. Unknown branches stay
+// in the scan so a declaration that the real compiler may see remains authoritative.
+fn c_header_possible_active_text(text string, flags []string, c99_mode bool, target pref.Target) string {
+	mut defined := map[string]bool{}
+	mut undefined := map[string]bool{}
+	mut uncertain := map[string]bool{}
+	// A preserved header inherits macros from its including source and parent
+	// headers. That context is not carried through this lightweight recursive
+	// scan, so an otherwise unknown macro must keep both branches possible.
+	mut external_macros_possible := true
+	mut i := 0
+	for i < flags.len {
+		clean := trimmed_space(flags[i])
+		mut definition := ''
+		mut is_undef := false
+		if clean == '-D' && i + 1 < flags.len {
+			definition = trimmed_space(flags[i + 1])
+			i++
+		} else if clean.starts_with('-D') {
+			definition = clean[2..]
+		} else if clean == '-U' && i + 1 < flags.len {
+			definition = trimmed_space(flags[i + 1])
+			is_undef = true
+			i++
+		} else if clean.starts_with('-U') {
+			definition = clean[2..]
+			is_undef = true
+		}
+		name := definition.all_before('=').trim_space()
+		if name.len > 0 {
+			if is_undef {
+				defined.delete(name)
+				undefined[name] = true
+			} else {
+				undefined.delete(name)
+				defined[name] = true
+			}
+		}
+		i++
+	}
+	strict_iso_mode := c_effective_strict_iso_mode(flags, c99_mode)
+	mut condition_known := []bool{}
+	mut condition_active := []bool{}
+	mut condition_taken_known := []bool{}
+	mut condition_taken := []bool{}
+	mut output := strings.new_builder(text.len)
+	mut in_block_comment := false
+	for line in c_join_continued_lines(text) {
+		clean, next_in_block_comment := c_preprocessor_directive_scan_line(line, in_block_comment)
+		in_block_comment = next_in_block_comment
+		name := c_directive_name(clean)
+		if name in ['ifdef', 'ifndef'] {
+			macro_name := c_directive_arg(clean).fields()[0] or { '' }
+			known, mut active := c_preprocessor_ifdef_macro_state(macro_name, defined, undefined,
+				uncertain, external_macros_possible, strict_iso_mode, target)
+			if name == 'ifndef' {
+				active = !active
+			}
+			condition_known << known
+			condition_active << (if known { active } else { true })
+			condition_taken_known << known
+			condition_taken << (if known { active } else { true })
+			output.writeln('')
+			continue
+		}
+		if name == 'if' {
+			known, active := c_preprocessor_condition_state(c_directive_arg(clean), defined,
+				undefined, uncertain, external_macros_possible, strict_iso_mode, target)
+			condition_known << known
+			condition_active << (if known { active } else { true })
+			condition_taken_known << known
+			condition_taken << (if known { active } else { true })
+			output.writeln('')
+			continue
+		}
+		if name == 'elif' && condition_known.len > 0 {
+			last := condition_known.len - 1
+			prior_known := condition_taken_known[last]
+			prior_taken := condition_taken[last]
+			known, active := c_preprocessor_condition_state(c_directive_arg(clean), defined,
+				undefined, uncertain, external_macros_possible, strict_iso_mode, target)
+			if (prior_known && prior_taken) || (known && !active) {
+				condition_known[last] = true
+				condition_active[last] = false
+			} else if prior_known && known {
+				condition_known[last] = true
+				condition_active[last] = true
+			} else {
+				condition_known[last] = false
+				condition_active[last] = true
+			}
+			if (prior_known && prior_taken) || (known && active) {
+				condition_taken_known[last] = true
+				condition_taken[last] = true
+			} else if prior_known && known {
+				condition_taken_known[last] = true
+				condition_taken[last] = false
+			} else {
+				condition_taken_known[last] = false
+				condition_taken[last] = true
+			}
+			output.writeln('')
+			continue
+		}
+		if name == 'else' && condition_known.len > 0 {
+			last := condition_known.len - 1
+			condition_known[last] = condition_taken_known[last]
+			condition_active[last] = if condition_taken_known[last] {
+				!condition_taken[last]
+			} else {
+				true
+			}
+			condition_taken_known[last] = true
+			condition_taken[last] = true
+			output.writeln('')
+			continue
+		}
+		if name == 'endif' && condition_known.len > 0 {
+			condition_known.delete_last()
+			condition_active.delete_last()
+			condition_taken_known.delete_last()
+			condition_taken.delete_last()
+			output.writeln('')
+			continue
+		}
+		mut possibly_active := true
+		mut definitely_active := true
+		for depth in 0 .. condition_known.len {
+			if condition_known[depth] && !condition_active[depth] {
+				possibly_active = false
+				definitely_active = false
+				break
+			}
+			if !condition_known[depth] {
+				definitely_active = false
+			}
+		}
+		if !possibly_active {
+			output.writeln('')
+			continue
+		}
+		if name in ['include', 'import'] {
+			c_preprocessor_invalidate_macro_state(mut defined, mut undefined, mut uncertain)
+			external_macros_possible = true
+		} else if name in ['define', 'undef'] {
+			parts := c_directive_arg(clean).fields()
+			if parts.len > 0 {
+				macro_name := parts[0].all_before('(')
+				if definitely_active {
+					uncertain.delete(macro_name)
+					if name == 'define' {
+						undefined.delete(macro_name)
+						defined[macro_name] = true
+					} else {
+						defined.delete(macro_name)
+						undefined[macro_name] = true
+					}
+				} else {
+					defined.delete(macro_name)
+					undefined.delete(macro_name)
+					uncertain[macro_name] = true
+				}
+			}
+		}
+		output.writeln(line)
+	}
+	return output.str()
+}
+
+fn c_preprocessor_ifdef_macro_state(name string, defined map[string]bool, undefined map[string]bool, uncertain map[string]bool, external_macros_possible bool, strict_iso_mode bool, target pref.Target) (bool, bool) {
+	if external_macros_possible && name !in defined && name !in undefined && name !in uncertain
+		&& name !in ['__linux__', '__linux', 'linux', 'unix', '__APPLE__', '__MACH__', '_WIN32', '_WIN64', '__FreeBSD__', '__OpenBSD__', '__NetBSD__'] {
+		// An earlier include can define an otherwise unknown macro. Keep both
+		// branches in that case so the declaration scan never drops active code.
+		return false, true
+	}
+	return c_preprocessor_macro_state(name, defined, undefined, uncertain, strict_iso_mode, target)
+}
+
 fn c_inline_header_text(include_arg string, vroot string, source_file string, include_dirs []string, translation_unit_uses_inttypes bool) ?CInlineHeader {
 	if replacement := c_system_include_replacement(include_arg, translation_unit_uses_inttypes) {
 		return CInlineHeader{
@@ -8378,8 +8559,8 @@ fn c_native_source_context_state(directives []string, flags []string, c99_mode b
 			name := c_directive_name(clean)
 			if name in ['ifdef', 'ifndef'] {
 				macro_name := c_directive_arg(clean).fields()[0] or { '' }
-				known, mut active := c_preprocessor_macro_state(macro_name, defined, undefined,
-					uncertain, strict_iso_mode, target)
+				known, mut active := c_preprocessor_ifdef_macro_state(macro_name, defined,
+					undefined, uncertain, external_macros_possible, strict_iso_mode, target)
 				if name == 'ifndef' {
 					active = !active
 				}
@@ -8624,8 +8805,8 @@ fn c_preprocessor_condition_state(raw string, defined map[string]bool, undefined
 	if macro_name.len == 0 || c_header_struct_tag(macro_name) != macro_name {
 		return false, true
 	}
-	known, mut active := c_preprocessor_macro_state(macro_name, defined, undefined, uncertain,
-		strict_iso_mode, target)
+	known, mut active := c_preprocessor_ifdef_macro_state(macro_name, defined, undefined,
+		uncertain, external_macros_possible, strict_iso_mode, target)
 	if negated {
 		active = !active
 	}

vlib/v3/gen/c/source_directive_test.v

@@ -57,6 +57,34 @@ fn test_preserved_header_trees_scan_shared_files_once() {
 	assert g.preserved_header_files_seen.len == 3
 }
 
+fn test_preserved_header_ignores_known_inactive_declarations() {
+	root := os.join_path(os.vtmp_dir(), 'v3_preserved_inactive_${os.getpid()}')
+	os.rmdir_all(root) or {}
+	os.mkdir_all(root) or { panic(err) }
+	defer {
+		os.rmdir_all(root) or {}
+	}
+	inactive_header := os.join_path(root, 'inactive.h')
+	macro_header := os.join_path(root, 'macro.h')
+	header := os.join_path(root, 'top.h')
+	os.write_file(inactive_header, 'int nested_inactive_fn(void);\n')!
+	os.write_file(macro_header,
+		'#if defined(PARENT_HEADER_FEATURE)\nint parent_enabled_fn(void);\n#endif\n#define PRESERVED_HEADER_FEATURE 1\n')!
+	os.write_file(header,
+		'#if 0\nint inactive_fn(void);\n#include "inactive.h"\n#else\nint active_fn(void);\n#endif\n#define PARENT_HEADER_FEATURE 1\n#include "macro.h"\n#if defined(PRESERVED_HEADER_FEATURE)\nint include_enabled_fn(void);\n#endif\n')!
+
+	mut g := FlatGen.new()
+	g.collect_preserved_header_file(header, [root])
+
+	assert 'inactive_fn' !in g.inlined_c_declared_fns
+	assert 'nested_inactive_fn' !in g.inlined_c_declared_fns
+	assert 'active_fn' in g.inlined_c_declared_fns
+	assert 'include_enabled_fn' in g.inlined_c_declared_fns
+	assert 'parent_enabled_fn' in g.inlined_c_declared_fns
+	assert os.real_path(inactive_header) !in g.preserved_header_files_seen
+	assert os.real_path(macro_header) in g.preserved_header_files_seen
+}
+
 fn collect_external_input_tree_status(root string, entry string, ambient_ambiguous bool) (bool, []string) {
 	mut active_paths := map[string]bool{}
 	mut collected_paths := map[string]bool{}