stage2 translate-c: function body statements

[hryx]

See #1964

Overview

This PR adds various translations that apply to statements inside function bodies.

Support translating from these Clang classes:

• BinaryOperator
  • Assign
  • Add
  • Sub
• CStyleCastExpr
• DeclStmt
  • Var
• DeclRefExpr
• ImplicitCastExpr
  • ArrayToPointerDecay
  • BitCast
  • FunctionToPointerDecay
  • IntegralCast
  • LValueToRValue
• IntegerLiteral
• ReturnStmt
• StringLiteral
• Type
  • Paren
  • Pointer

Other changes

• Make stage1 always render a space after fn to match stage2 behavior. This convergence was done to make existing translate-c test cases work for both stage1 and stage2. stage2 behavior was favored because the existing meaningful tests on rendering (i.e. zig fmt) pertain to stage2.
• Separate generated Zig source tokens by a space instead of a newline to prevent the rendered Zig code from having excessive newlines/indentation.

Regressions

• The stage2-only tests were commented out because function prototype translation changed after implementing "translate mode" for stage2. I left them commented rather than removing them because, as discussed, the concept of translate mode will be removed, at which point this test should be reinstated as-is.

TODO

• Fix recursive rendering of local var decls
• Render integer literal with correct value
• Find "wanted" Zig name from C name
• Convert several a few stage1 test cases to stage1+stage2
• Clean up clang.zig?
  • This PR follows the existing organization, no need to change that right now
• Have stage2 respect "translate mode"
• Make a follow-up issue to remove "translate mode" and converge behavior
  • translate-c: remove concept of "translate mode" #2780

[andrewrk]

BTW did you see that test/translate_c.zig supports stage2 tests now?

[hryx]

@andrewrk Cool, I hadn't seen that! I'll make use of that once the new changes can pass a test.

Current status: This is getting bigger than I wanted, but after a squish 'n' rebase it shouldn't cause any problems. Something I did is causing infinitely nesting blocks to be rendered when one or more local var decl exists in the function body. After that is solved I'll fast-track this to mergeability.

[hryx]

Something is wrong with this test. It passes on my Linux machine but not on mac:

Test 6/119 translate-c-2 ignore result, explicit function arguments...
========= Expected this output: ================
pub fn foo() void {
    var a: c_int = undefined;
    _ = 1;
    _ = c"hey";
    _ = (1 + 1);
    _ = (1 - 1);
    a = 1;
}
========= But found: ===========================
pub fn foo() void {
    var a: c_int = undefined;
    _ = 1;
    _ = c"hey<\xb0\x7f";
    _ = (1 + 1);
    _ = (1 - 1);
    a = 1;
}

I think there may be a legitimate bug in my string literal implementation but I haven't identified it yet.

[hryx]

Ok @andrewrk, this should be good to review now — I expect the tests to pass Windows CI is failing, see below.

If you wouldn't mind sanity-checking that the last commit (247e567) was actually necessary?

[Sahnvour]

Regarding the error, I was able to get more information by running debug zig, and it looks like it may be related to result location. See https://gist.github.com/Sahnvour/16d36b44e0d313a1cd520ba4b6e0eccd.

[andrewrk]

Indeed, the crash appears to be a regression from result location changes, in the return statement of this code:

fn getExprQualType(c: *Context, expr: *const ZigClangExpr) ZigClangQualType {
    blk: {
        // If this is a C `char *`, turn it into a `const char *`
        if (ZigClangExpr_getStmtClass(expr) != .ImplicitCastExprClass) break :blk;
        const cast_expr = @ptrCast(*const ZigClangImplicitCastExpr, expr);
        if (ZigClangImplicitCastExpr_getCastKind(cast_expr) != .ArrayToPointerDecay) break :blk;
        const sub_expr = ZigClangImplicitCastExpr_getSubExpr(cast_expr);
        if (ZigClangExpr_getStmtClass(sub_expr) != .StringLiteralClass) break :blk;
        const array_qt = ZigClangExpr_getType(sub_expr);
        const array_type = @ptrCast(*const ZigClangArrayType, ZigClangQualType_getTypePtr(array_qt));
        var pointee_qt = ZigClangArrayType_getElementType(array_type);
        ZigClangQualType_addConst(&pointee_qt);
        return ZigClangASTContext_getPointerType(c.clang_context, pointee_qt);
    }
    return ZigClangExpr_getType(expr);
}

I'll modify the code to work around the issue and then try to come up with a reduction for the bug to fix separately.

[andrewrk]

I was able to fix the issue. Here's an explanation.

First of all, why is it failing on Windows? I don't know, but we were tripping an LLVM assert on all platforms and Windows just happened to be the one where Undefined Behavior caused a crash. It would have eventually been caught regardless, when we tested the code on a debug build of LLVM, which I always do for the branch that tests against the next LLVM version. A debug build of LLVM catches this issue on all platforms, not just Windows.

What happened is that, before, we had this code:

    } else if (handle_is_ptr(src_return_type)) {
        LLVMValueRef store_instr = LLVMBuildStore(g->builder, result, result_loc);
        LLVMSetAlignment(store_instr, LLVMGetAlignment(result_loc)); // note this line
        return result_loc;
    } else {

Except before the result location branch, result_loc was instruction->tmp_ptr. And that was always an alloca instruction. Look at LLVM's implementation of the relevant function:

unsigned LLVMGetAlignment(LLVMValueRef V) {
  Value *P = unwrap<Value>(V);
  if (GlobalValue *GV = dyn_cast<GlobalValue>(P))
    return GV->getAlignment();
  if (AllocaInst *AI = dyn_cast<AllocaInst>(P))
    return AI->getAlignment();
  if (LoadInst *LI = dyn_cast<LoadInst>(P))
    return LI->getAlignment();
  if (StoreInst *SI = dyn_cast<StoreInst>(P))
    return SI->getAlignment();

  llvm_unreachable(
      "only GlobalValue, AllocaInst, LoadInst and StoreInst have alignment");
}

So you can see that it always worked before because it always hit the AllocaInst case. However, after result location changes, the result location value is not necessarily an LLVM Alloca instruction. However, it is always a pointer, and we have a Zig type for it. So the fix is to use that type information:

    } else if (handle_is_ptr(src_return_type)) {
        LLVMValueRef store_instr = LLVMBuildStore(g->builder, result, result_loc);
-        LLVMSetAlignment(store_instr, LLVMGetAlignment(result_loc));
+        LLVMSetAlignment(store_instr, get_ptr_align(g, instruction->result_loc->value.type));
        return result_loc;
    } else {

After this fix, I can build zig against debug LLVM and all the translate-c tests pass 🎉

[hryx]

Amazing explanation of what happened!