[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

[Martin Storsjö via Phabricator via cfe-commits]

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