cgen, checker: fix conditional evaluation and code generation for $if/$else $if/$else for known true conditions (#16823)

vlib/v/checker/checker.v

@@ -98,6 +98,7 @@ mut:
 	comptime_for_field_var           string
 	comptime_fields_default_type     ast.Type
 	comptime_fields_type             map[string]ast.Type
+	comptime_for_field_value         ast.StructField // value of the field variable
 	fn_scope                         &ast.Scope = unsafe { nil }
 	main_fn_decl_node                ast.FnDecl
 	match_exhaustive_cutoff_limit    int = 10

vlib/v/checker/comptime.v

@@ -180,6 +180,7 @@ fn (mut c Checker) comptime_for(node ast.ComptimeFor) {
 			sym_info := sym.info as ast.Struct
 			c.inside_comptime_for_field = true
 			for field in sym_info.fields {
+				c.comptime_for_field_value = field
 				c.comptime_for_field_var = node.val_var
 				c.comptime_fields_type[node.val_var] = node.typ
 				c.comptime_fields_default_type = field.typ
@@ -725,7 +726,8 @@ fn (mut c Checker) comptime_if_branch(cond ast.Expr, pos token.Pos) ComptimeBran
 		ast.SelectorExpr {
 			if c.check_comptime_is_field_selector(cond) {
 				if c.check_comptime_is_field_selector_bool(cond) {
-					return .eval
+					ret_bool := c.get_comptime_selector_bool_field(cond.field_name)
+					return if ret_bool { .eval } else { .skip }
 				}
 				c.error('unknown field `${cond.field_name}` from ${c.comptime_for_field_var}',
 					cond.pos)
@@ -755,3 +757,23 @@ fn (mut c Checker) check_comptime_is_field_selector_bool(node ast.SelectorExpr)
 	}
 	return false
 }
+
+fn (mut c Checker) get_comptime_selector_bool_field(field_name string) bool {
+	field := c.comptime_for_field_value
+	field_typ := c.comptime_fields_default_type
+	field_sym := c.table.sym(c.unwrap_generic(c.comptime_fields_default_type))
+
+	match field_name {
+		'is_pub' { return field.is_pub }
+		'is_mut' { return field.is_mut }
+		'is_shared' { return field_typ.has_flag(.shared_f) }
+		'is_atomic' { return field_typ.has_flag(.atomic_f) }
+		'is_optional' { return field.typ.has_flag(.optional) }
+		'is_array' { return field_sym.kind in [.array, .array_fixed] }
+		'is_map' { return field_sym.kind == .map }
+		'is_chan' { return field_sym.kind == .chan }
+		'is_struct' { return field_sym.kind == .struct_ }
+		'is_alias' { return field_sym.kind == .alias }
+		else { return false }
+	}
+}

vlib/v/checker/if.v

@@ -72,7 +72,13 @@ fn (mut c Checker) if_expr(mut node ast.IfExpr) ast.Type {
 		if node.is_comptime { // Skip checking if needed
 			// smartcast field type on comptime if
 			mut comptime_field_name := ''
-			if mut branch.cond is ast.InfixExpr {
+			if branch.cond is ast.SelectorExpr && skip_state != .unknown {
+				is_comptime_type_is_expr = true
+			} else if mut branch.cond is ast.PrefixExpr {
+				if branch.cond.right is ast.SelectorExpr && skip_state != .unknown {
+					is_comptime_type_is_expr = true
+				}
+			} else if mut branch.cond is ast.InfixExpr {
 				if branch.cond.op == .key_is {
 					left := branch.cond.left
 					right := branch.cond.right

vlib/v/gen/c/comptime.v

@@ -285,19 +285,28 @@ fn (mut g Gen) comptime_if(node ast.IfExpr) {
 	mut comptime_if_stmts_skip := false // don't write any statements if the condition is false
 	// (so that for example windows calls don't get generated inside `$if macos` which
 	// will lead to compilation errors)
+	mut comptime_may_skip_else := false
 
 	for i, branch in node.branches {
 		start_pos := g.out.len
+		if comptime_may_skip_else {
+			continue // if we already have a known true, ignore other branches
+		}
 		if i == node.branches.len - 1 && node.has_else {
 			g.writeln('#else')
-			comptime_if_stmts_skip = false
+			comptime_if_stmts_skip = comptime_may_skip_else
 		} else {
 			if i == 0 {
 				g.write('#if ')
 			} else {
 				g.write('#elif ')
 			}
-			comptime_if_stmts_skip = !g.comptime_if_cond(branch.cond, branch.pkg_exist)
+			comptime_if_stmts_skip, comptime_may_skip_else = g.comptime_if_cond(branch.cond,
+				branch.pkg_exist)
+			if !comptime_if_stmts_skip && comptime_may_skip_else {
+				comptime_may_skip_else = false // if the cond is false, not skip else branch
+			}
+			comptime_if_stmts_skip = !comptime_if_stmts_skip
 			g.writeln('')
 		}
 		expr_str := g.out.last_n(g.out.len - start_pos).trim_space()
@@ -351,39 +360,45 @@ fn (mut g Gen) comptime_if(node ast.IfExpr) {
 	}
 }
 
-// returns the value of the bool comptime expression
+// returns the value of the bool comptime expression and if next branches may be discarded
 // returning `false` means the statements inside the $if can be skipped
-fn (mut g Gen) comptime_if_cond(cond ast.Expr, pkg_exist bool) bool {
+fn (mut g Gen) comptime_if_cond(cond ast.Expr, pkg_exist bool) (bool, bool) {
 	match cond {
 		ast.BoolLiteral {
 			g.expr(cond)
-			return true
+			return cond.val, true
 		}
 		ast.ParExpr {
 			g.write('(')
-			is_cond_true := g.comptime_if_cond(cond.expr, pkg_exist)
+			is_cond_true, may_discard := g.comptime_if_cond(cond.expr, pkg_exist)
 			g.write(')')
-			return is_cond_true
+			return is_cond_true, may_discard
 		}
 		ast.PrefixExpr {
 			g.write(cond.op.str())
-			return g.comptime_if_cond(cond.right, pkg_exist)
+			is_cond_true, _ := g.comptime_if_cond(cond.right, pkg_exist)
+			if cond.op == .not {
+				if cond.right in [ast.BoolLiteral, ast.SelectorExpr] {
+					return !is_cond_true, true
+				}
+			}
+			return is_cond_true, false
 		}
 		ast.PostfixExpr {
 			ifdef := g.comptime_if_to_ifdef((cond.expr as ast.Ident).name, true) or {
 				verror(err.msg())
-				return false
+				return false, true
 			}
 			g.write('defined(${ifdef})')
-			return true
+			return true, false
 		}
 		ast.InfixExpr {
 			match cond.op {
 				.and, .logical_or {
-					l := g.comptime_if_cond(cond.left, pkg_exist)
+					l, d1 := g.comptime_if_cond(cond.left, pkg_exist)
 					g.write(' ${cond.op} ')
-					r := g.comptime_if_cond(cond.right, pkg_exist)
+					r, d2 := g.comptime_if_cond(cond.right, pkg_exist)
-					return if cond.op == .and { l && r } else { l || r }
+					return if cond.op == .and { l && r } else { l || r }, d1 && d1 == d2
 				}
 				.key_is, .not_is {
 					left := cond.left
@@ -397,14 +412,14 @@ fn (mut g Gen) comptime_if_cond(cond ast.Expr, pkg_exist bool) bool {
 							} else {
 								g.write('0')
 							}
-							return is_true
+							return is_true, true
 						} else {
 							if is_true {
 								g.write('0')
 							} else {
 								g.write('1')
 							}
-							return !is_true
+							return !is_true, true
 						}
 					}
 					mut exp_type := ast.Type(0)
@@ -429,14 +444,14 @@ fn (mut g Gen) comptime_if_cond(cond ast.Expr, pkg_exist bool) bool {
 								} else {
 									g.write('0')
 								}
-								return is_true
+								return is_true, true
 							} else if cond.op == .not_is {
 								if is_true {
 									g.write('0')
 								} else {
 									g.write('1')
 								}
-								return !is_true
+								return !is_true, true
 							}
 							// matches_interface = '/*iface:$got_type $exp_type*/ true'
 							//}
@@ -455,53 +470,53 @@ fn (mut g Gen) comptime_if_cond(cond ast.Expr, pkg_exist bool) bool {
 
 					if cond.op == .key_is {
 						g.write('${exp_type.idx()} == ${got_type.idx()} && ${exp_type.has_flag(.optional)} == ${got_type.has_flag(.optional)}')
-						return exp_type == got_type
+						return exp_type == got_type, true
 					} else {
 						g.write('${exp_type.idx()} != ${got_type.idx()}')
-						return exp_type != got_type
+						return exp_type != got_type, true
 					}
 				}
 				.eq, .ne {
 					// TODO Implement `$if method.args.len == 1`
 					if cond.left is ast.SelectorExpr || cond.right is ast.SelectorExpr {
-						l := g.comptime_if_cond(cond.left, pkg_exist)
+						l, d1 := g.comptime_if_cond(cond.left, pkg_exist)
 						g.write(' ${cond.op} ')
-						r := g.comptime_if_cond(cond.right, pkg_exist)
+						r, d2 := g.comptime_if_cond(cond.right, pkg_exist)
-						return if cond.op == .eq { l == r } else { l != r }
+						return if cond.op == .eq { l == r } else { l != r }, d1 && d1 == d2
 					} else {
 						g.write('1')
-						return true
+						return true, true
 					}
 				}
 				else {
-					return true
+					return true, false
 				}
 			}
 		}
 		ast.Ident {
 			ifdef := g.comptime_if_to_ifdef(cond.name, false) or { 'true' } // handled in checker
 			g.write('defined(${ifdef})')
-			return true
+			return true, false
 		}
 		ast.ComptimeCall {
 			g.write('${pkg_exist}')
-			return true
+			return true, false
 		}
 		ast.SelectorExpr {
 			if g.inside_comptime_for_field && cond.expr is ast.Ident
 				&& (cond.expr as ast.Ident).name == g.comptime_for_field_var && cond.field_name in ['is_mut', 'is_pub', 'is_shared', 'is_atomic', 'is_optional', 'is_array', 'is_map', 'is_chan', 'is_struct', 'is_alias'] {
 				ret_bool := g.get_comptime_selector_bool_field(cond.field_name)
 				g.write(ret_bool.str())
-				return ret_bool
+				return ret_bool, true
 			} else {
 				g.write('1')
-				return true
+				return true, true
 			}
 		}
 		else {
 			// should be unreachable, but just in case
 			g.write('1')
-			return true
+			return true, true
 		}
 	}
 }

vlib/v/tests/ifcomp_expr_evaluate_test.v

@@ -0,0 +1,65 @@
+struct EmptyStruct {
+	b []string
+pub:
+	a int
+}
+
+fn encode_struct[T](val T) {
+	mut result := map[string][]string{}
+	$for field in T.fields {
+		$if field.is_array == false {
+			result[field.name] << '> is not array'
+		} $else {
+			result[field.name] << '> is array'
+		}
+		$if !field.is_array {
+			result[field.name] << '>> is not array'
+		} $else $if field.is_pub {
+			result[field.name] << '>> is public'
+		} $else {
+			result[field.name] << '>> is array'
+		}
+		$if field.is_array {
+			result[field.name] << '>>> is array'
+		} $else {
+			result[field.name] << '>>> is not array'
+		}
+		$if field.is_shared {
+			result[field.name] << '>>>> is shared'
+		} $else $if field.is_pub {
+			result[field.name] << '>>>> is pub'
+		}
+	}
+	$if !false {
+		result['bool'] << '1'
+	}
+	$if !true {
+		result['bool'] << '2'
+	}
+	$if true {
+		result['bool'] << '3'
+	}
+	$if false {
+		result['bool'] << '4'
+	}
+
+	assert result['a'].len == 4
+	assert result['b'].len == 3
+	assert result['bool'].len == 2
+
+	assert result['a'][0] == '> is not array'
+	assert result['a'][1] == '>> is not array'
+	assert result['a'][2] == '>>> is not array'
+	assert result['a'][3] == '>>>> is pub'
+
+	assert result['b'][0] == '> is array'
+	assert result['b'][1] == '>> is array'
+	assert result['b'][2] == '>>> is array'
+
+	assert result['bool'][0] == '1'
+	assert result['bool'][1] == '3'
+}
+
+fn test_main() {
+	encode_struct(EmptyStruct{})
+}