Revert "Merge pull request #13914 from Vexu/variadic"

This reverts commit aca9c74, reversing
changes made to d93edad.

These changes were not tested with LLVM with assertions enabled. They
caused a regression in master branch, triggering LLVM assertions when
running behavior tests.

Reopens #515

Author: andrewrk

doc/langref.html.in

@@ -8137,35 +8137,6 @@ test "main" {
       {#see_also|Import from C Header File|@cImport|@cDefine|@cInclude#}
       {#header_close#}
 
-      {#header_open|@cVaArg#}
-      <pre>{#syntax#}@cVaArg(operand: *std.builtin.VaList, comptime T: type) T{#endsyntax#}</pre>
-      <p>
-      Implements the C macro {#syntax#}va_arg{#endsyntax#}.
-      </p>
-      {#see_also|@cVaCopy|@cVaEnd|@cVaStart#}
-      {#header_close#}
-      {#header_open|@cVaCopy#}
-      <pre>{#syntax#}@cVaCopy(src: *std.builtin.VaList) std.builtin.VaList{#endsyntax#}</pre>
-      <p>
-      Implements the C macro {#syntax#}va_copy{#endsyntax#}.
-      </p>
-      {#see_also|@cVaArg|@cVaEnd|@cVaStart#}
-      {#header_close#}
-      {#header_open|@cVaEnd#}
-      <pre>{#syntax#}@cVaEnd(src: *std.builtin.VaList) void{#endsyntax#}</pre>
-      <p>
-      Implements the C macro {#syntax#}va_end{#endsyntax#}.
-      </p>
-      {#see_also|@cVaArg|@cVaCopy|@cVaStart#}
-      {#header_close#}
-      {#header_open|@cVaStart#}
-      <pre>{#syntax#}@cVaStart() std.builtin.VaList{#endsyntax#}</pre>
-      <p>
-      Implements the C macro {#syntax#}va_start{#endsyntax#}. Only valid inside a variadic function.
-      </p>
-      {#see_also|@cVaArg|@cVaCopy|@cVaEnd#}
-      {#header_close#}
-
       {#header_open|@divExact#}
       <pre>{#syntax#}@divExact(numerator: T, denominator: T) T{#endsyntax#}</pre>
       <p>
@@ -10874,32 +10845,14 @@ test "variadic function" {
 }
       {#code_end#}
       <p>
-        Variadic functions can be implemented using {#link|@cVaStart#}, {#link|@cVaEnd#}, {#link|@cVaArg#} and {#link|@cVaCopy#}
+        Non extern variadic functions are currently not implemented, but there
+        is an accepted proposal. See <a href="https://github.com/ziglang/zig/issues/515">#515</a>.
       </p>
-      {#code_begin|test|defining_variadic_function#}
-const std = @import("std");
-const testing = std.testing;
-const builtin = @import("builtin");
+      {#code_begin|obj_err|non-extern function is variadic#}
+export fn printf(format: [*:0]const u8, ...) c_int {
+    _ = format;
 
-fn add(count: c_int, ...) callconv(.C) c_int {
-    var ap = @cVaStart();
-    defer @cVaEnd(&ap);
-    var i: usize = 0;
-    var sum: c_int = 0;
-    while (i < count) : (i += 1) {
-        sum += @cVaArg(&ap, c_int);
-    }
-    return sum;
-}
-
-test "defining a variadic function" {
-    // Variadic functions are currently disabled on some targets due to miscompilations.
-    if (builtin.cpu.arch == .aarch64 and builtin.os.tag != .windows and builtin.os.tag != .macos) return error.SkipZigTest;
-    if (builtin.cpu.arch == .x86_64 and builtin.os.tag == .windows) return error.SkipZigTest;
-
-    try std.testing.expectEqual(@as(c_int, 0), add(0));
-    try std.testing.expectEqual(@as(c_int, 1), add(1, @as(c_int, 1)));
-    try std.testing.expectEqual(@as(c_int, 3), add(2, @as(c_int, 1), @as(c_int, 2)));
+    return 0;
 }
       {#code_end#}
       {#header_close#}

lib/std/builtin.zig

@@ -620,87 +620,6 @@ pub const CallModifier = enum {
     compile_time,
 };
 
-/// This data structure is used by the Zig language code generation and
-/// therefore must be kept in sync with the compiler implementation.
-pub const VaListAarch64 = extern struct {
-    __stack: *anyopaque,
-    __gr_top: *anyopaque,
-    __vr_top: *anyopaque,
-    __gr_offs: c_int,
-    __vr_offs: c_int,
-};
-
-/// This data structure is used by the Zig language code generation and
-/// therefore must be kept in sync with the compiler implementation.
-pub const VaListHexagon = extern struct {
-    __gpr: c_long,
-    __fpr: c_long,
-    __overflow_arg_area: *anyopaque,
-    __reg_save_area: *anyopaque,
-};
-
-/// This data structure is used by the Zig language code generation and
-/// therefore must be kept in sync with the compiler implementation.
-pub const VaListPowerPc = extern struct {
-    gpr: u8,
-    fpr: u8,
-    reserved: c_ushort,
-    overflow_arg_area: *anyopaque,
-    reg_save_area: *anyopaque,
-};
-
-/// This data structure is used by the Zig language code generation and
-/// therefore must be kept in sync with the compiler implementation.
-pub const VaListS390x = extern struct {
-    __current_saved_reg_area_pointer: *anyopaque,
-    __saved_reg_area_end_pointer: *anyopaque,
-    __overflow_area_pointer: *anyopaque,
-};
-
-/// This data structure is used by the Zig language code generation and
-/// therefore must be kept in sync with the compiler implementation.
-pub const VaListX86_64 = extern struct {
-    gp_offset: c_uint,
-    fp_offset: c_uint,
-    overflow_arg_area: *anyopaque,
-    reg_save_area: *anyopaque,
-};
-
-/// This data structure is used by the Zig language code generation and
-/// therefore must be kept in sync with the compiler implementation.
-pub const VaList = switch (builtin.cpu.arch) {
-    .aarch64 => switch (builtin.os.tag) {
-        .windows => *u8,
-        .ios, .macos, .tvos, .watchos => *u8,
-        else => @compileError("disabled due to miscompilations"), // VaListAarch64,
-    },
-    .arm => switch (builtin.os.tag) {
-        .ios, .macos, .tvos, .watchos => *u8,
-        else => *anyopaque,
-    },
-    .amdgcn => *u8,
-    .avr => *anyopaque,
-    .bpfel, .bpfeb => *anyopaque,
-    .hexagon => if (builtin.target.isMusl()) VaListHexagon else *u8,
-    .mips, .mipsel, .mips64, .mips64el => *anyopaque,
-    .riscv32, .riscv64 => *anyopaque,
-    .powerpc, .powerpcle => switch (builtin.os.tag) {
-        .ios, .macos, .tvos, .watchos, .aix => *u8,
-        else => VaListPowerPc,
-    },
-    .powerpc64, .powerpc64le => *u8,
-    .sparc, .sparcel, .sparc64 => *anyopaque,
-    .spirv32, .spirv64 => *anyopaque,
-    .s390x => VaListS390x,
-    .wasm32, .wasm64 => *anyopaque,
-    .x86 => *u8,
-    .x86_64 => switch (builtin.os.tag) {
-        .windows => @compileError("disabled due to miscompilations"), // *u8,
-        else => VaListX86_64,
-    },
-    else => @compileError("VaList not supported for this target yet"),
-};
-
 /// This data structure is used by the Zig language code generation and
 /// therefore must be kept in sync with the compiler implementation.
 pub const PrefetchOptions = struct {

src/Air.zig

@@ -741,19 +741,6 @@ pub const Inst = struct {
         /// Uses the `vector_store_elem` field.
         vector_store_elem,
 
-        /// Implements @cVaArg builtin.
-        /// Uses the `ty_op` field.
-        c_va_arg,
-        /// Implements @cVaCopy builtin.
-        /// Uses the `ty_op` field.
-        c_va_copy,
-        /// Implements @cVaEnd builtin.
-        /// Uses the `un_op` field.
-        c_va_end,
-        /// Implements @cVaStart builtin.
-        /// Uses the `ty` field.
-        c_va_start,
-
         pub fn fromCmpOp(op: std.math.CompareOperator, optimized: bool) Tag {
             switch (op) {
                 .lt => return if (optimized) .cmp_lt_optimized else .cmp_lt,
@@ -1105,7 +1092,6 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type {
         .ret_ptr,
         .arg,
         .err_return_trace,
-        .c_va_start,
         => return datas[inst].ty,
 
         .assembly,
@@ -1170,8 +1156,6 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type {
         .byte_swap,
         .bit_reverse,
         .addrspace_cast,
-        .c_va_arg,
-        .c_va_copy,
         => return air.getRefType(datas[inst].ty_op.ty),
 
         .loop,
@@ -1203,7 +1187,6 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type {
         .prefetch,
         .set_err_return_trace,
         .vector_store_elem,
-        .c_va_end,
         => return Type.void,
 
         .ptrtoint,

src/AstGen.zig

@@ -42,7 +42,6 @@ string_table: std.HashMapUnmanaged(u32, void, StringIndexContext, std.hash_map.d
 compile_errors: ArrayListUnmanaged(Zir.Inst.CompileErrors.Item) = .{},
 /// The topmost block of the current function.
 fn_block: ?*GenZir = null,
-fn_var_args: bool = false,
 /// Maps string table indexes to the first `@import` ZIR instruction
 /// that uses this string as the operand.
 imports: std.AutoArrayHashMapUnmanaged(u32, Ast.TokenIndex) = .{},
@@ -3891,17 +3890,17 @@ fn fnDecl(
             .noalias_bits = noalias_bits,
         });
     } else func: {
+        if (is_var_args) {
+            return astgen.failTok(fn_proto.ast.fn_token, "non-extern function is variadic", .{});
+        }
+
         // as a scope, fn_gz encloses ret_gz, but for instruction list, fn_gz stacks on ret_gz
         fn_gz.instructions_top = ret_gz.instructions.items.len;
 
         const prev_fn_block = astgen.fn_block;
         astgen.fn_block = &fn_gz;
         defer astgen.fn_block = prev_fn_block;
 
-        const prev_var_args = astgen.fn_var_args;
-        astgen.fn_var_args = is_var_args;
-        defer astgen.fn_var_args = prev_var_args;
-
         astgen.advanceSourceCursorToNode(body_node);
         const lbrace_line = astgen.source_line - decl_gz.decl_line;
         const lbrace_column = astgen.source_column;
@@ -6070,15 +6069,15 @@ fn whileExpr(
             const tag: Zir.Inst.Tag = if (payload_is_ref) .is_non_err_ptr else .is_non_err;
             break :c .{
                 .inst = err_union,
-                .bool_bit = try cond_scope.addUnNode(tag, err_union, while_full.ast.cond_expr),
+                .bool_bit = try cond_scope.addUnNode(tag, err_union, while_full.ast.then_expr),
             };
         } else if (while_full.payload_token) |_| {
             const cond_ri: ResultInfo = .{ .rl = if (payload_is_ref) .ref else .none };
             const optional = try expr(&cond_scope, &cond_scope.base, cond_ri, while_full.ast.cond_expr);
             const tag: Zir.Inst.Tag = if (payload_is_ref) .is_non_null_ptr else .is_non_null;
             break :c .{
                 .inst = optional,
-                .bool_bit = try cond_scope.addUnNode(tag, optional, while_full.ast.cond_expr),
+                .bool_bit = try cond_scope.addUnNode(tag, optional, while_full.ast.then_expr),
             };
         } else {
             const cond = try expr(&cond_scope, &cond_scope.base, bool_ri, while_full.ast.cond_expr);
@@ -8369,46 +8368,6 @@ fn builtinCall(
             });
             return rvalue(gz, ri, result, node);
         },
-        .c_va_arg => {
-            if (astgen.fn_block == null) {
-                return astgen.failNode(node, "'@cVaArg' outside function scope", .{});
-            }
-            const result = try gz.addExtendedPayload(.c_va_arg, Zir.Inst.BinNode{
-                .node = gz.nodeIndexToRelative(node),
-                .lhs = try expr(gz, scope, .{ .rl = .none }, params[0]),
-                .rhs = try typeExpr(gz, scope, params[1]),
-            });
-            return rvalue(gz, ri, result, node);
-        },
-        .c_va_copy => {
-            if (astgen.fn_block == null) {
-                return astgen.failNode(node, "'@cVaCopy' outside function scope", .{});
-            }
-            const result = try gz.addExtendedPayload(.c_va_copy, Zir.Inst.UnNode{
-                .node = gz.nodeIndexToRelative(node),
-                .operand = try expr(gz, scope, .{ .rl = .none }, params[0]),
-            });
-            return rvalue(gz, ri, result, node);
-        },
-        .c_va_end => {
-            if (astgen.fn_block == null) {
-                return astgen.failNode(node, "'@cVaEnd' outside function scope", .{});
-            }
-            const result = try gz.addExtendedPayload(.c_va_end, Zir.Inst.UnNode{
-                .node = gz.nodeIndexToRelative(node),
-                .operand = try expr(gz, scope, .{ .rl = .none }, params[0]),
-            });
-            return rvalue(gz, ri, result, node);
-        },
-        .c_va_start => {
-            if (astgen.fn_block == null) {
-                return astgen.failNode(node, "'@cVaStart' outside function scope", .{});
-            }
-            if (!astgen.fn_var_args) {
-                return astgen.failNode(node, "'@cVaStart' in a non-variadic function", .{});
-            }
-            return rvalue(gz, ri, try gz.addNodeExtended(.c_va_start, node), node);
-        },
     }
 }
 

src/BuiltinFn.zig

@@ -30,10 +30,6 @@ pub const Tag = enum {
     compile_log,
     ctz,
     c_undef,
-    c_va_arg,
-    c_va_copy,
-    c_va_end,
-    c_va_start,
     div_exact,
     div_floor,
     div_trunc,
@@ -358,30 +354,6 @@ pub const list = list: {
                 .param_count = 1,
             },
         },
-        .{
-            "@cVaArg", .{
-                .tag = .c_va_arg,
-                .param_count = 2,
-            },
-        },
-        .{
-            "@cVaCopy", .{
-                .tag = .c_va_copy,
-                .param_count = 1,
-            },
-        },
-        .{
-            "@cVaEnd", .{
-                .tag = .c_va_end,
-                .param_count = 1,
-            },
-        },
-        .{
-            "@cVaStart", .{
-                .tag = .c_va_start,
-                .param_count = 0,
-            },
-        },
         .{
             "@divExact",
             .{

src/Liveness.zig

@@ -238,7 +238,6 @@ pub fn categorizeOperand(
         .wasm_memory_size,
         .err_return_trace,
         .save_err_return_trace_index,
-        .c_va_start,
         => return .none,
 
         .fence => return .write,
@@ -280,8 +279,6 @@ pub fn categorizeOperand(
         .splat,
         .error_set_has_value,
         .addrspace_cast,
-        .c_va_arg,
-        .c_va_copy,
         => {
             const o = air_datas[inst].ty_op;
             if (o.operand == operand_ref) return matchOperandSmallIndex(l, inst, 0, .none);
@@ -325,7 +322,6 @@ pub fn categorizeOperand(
         .trunc_float,
         .neg,
         .cmp_lt_errors_len,
-        .c_va_end,
         => {
             const o = air_datas[inst].un_op;
             if (o == operand_ref) return matchOperandSmallIndex(l, inst, 0, .none);
@@ -861,7 +857,6 @@ fn analyzeInst(
         .wasm_memory_size,
         .err_return_trace,
         .save_err_return_trace_index,
-        .c_va_start,
         => return trackOperands(a, new_set, inst, main_tomb, .{ .none, .none, .none }),
 
         .not,
@@ -903,8 +898,6 @@ fn analyzeInst(
         .splat,
         .error_set_has_value,
         .addrspace_cast,
-        .c_va_arg,
-        .c_va_copy,
         => {
             const o = inst_datas[inst].ty_op;
             return trackOperands(a, new_set, inst, main_tomb, .{ o.operand, .none, .none });
@@ -943,7 +936,6 @@ fn analyzeInst(
         .neg_optimized,
         .cmp_lt_errors_len,
         .set_err_return_trace,
-        .c_va_end,
         => {
             const operand = inst_datas[inst].un_op;
             return trackOperands(a, new_set, inst, main_tomb, .{ operand, .none, .none });