mstorsjo added a comment. In D157332#4568341 <https://reviews.llvm.org/D157332#4568341>, @efriedma wrote:
> I see what you're getting at here... but I don't think this works quite > right. If the empty class has a non-trivial constructor, we have to pass the > correct "this" address to that constructor. Usually a constructor for an > empty class won't do anything with that address, but it could theoretically > do something with it. Hmm, I think I see. In this specific case, there's no constructor for the empty class invoked at all, we call the function `f()` which returns an `struct S` (which is empty). But if we'd remove the initialization of `y(f())` and add a constructor to `S()`, I see that we generate code that is indeed wrong in that aspect. > In order to preserve the address in the cases we need it, we need a different > invariant: the handling for each aggregate expression in EmitAggExpr needs to > ensure it doesn't store anything to an empty class. Which is unfortunately > more fragile than I'd like, but I can't think of a better approach. This > check needs to happen further down the call stack. Maybe put it in > CodeGenFunction::EmitCall. Ok, I'll see if I can look further into that... (I don't have a huge amount of time for it at the moment, so if someone else with an interest in the area, e.g. @crtrott or @amyk want to dig into it, feel free!) ================ Comment at: clang/test/CodeGenCXX/ctor-empty-nounique.cpp:1 +// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s | FileCheck %s + ---------------- cor3ntin wrote: > This should probably get another run line for powerpc64le Sure, I could do that. (Initially I had both, but thought people would consider it unnecessary duplication.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157332/new/ https://reviews.llvm.org/D157332 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits