[all-commits] [llvm/llvm-project] f94bbf: [Clang][CodeGen] Fix `CanSkipVTablePointerInitiali...

[Max Winkler via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: f94bbfed7cf08f60e20756dce8965d2c6ed70ea1
      https://github.com/llvm/llvm-project/commit/f94bbfed7cf08f60e20756dce8965d2c6ed70ea1
  Author: Max Winkler <max.enrico.winkler at gmail.com>
  Date:   2024-04-08 (Mon, 08 Apr 2024)

  Changed paths:
    M clang/lib/CodeGen/CGClass.cpp
    M clang/test/CodeGenCXX/skip-vtable-pointer-initialization.cpp

  Log Message:
  -----------
  [Clang][CodeGen] Fix `CanSkipVTablePointerInitialization` for dynamic classes with a trivial anonymous union (#84651)

Hit this when trying upgrade an old project of mine. I couldn't find a
corresponding existing issue for this when spelunking the open issues
here on github.
Thankfully I can work-around it today with the `[[clang::no_destroy]]`
attribute for my use case. However it should still be properly fixed.

### Issue and History ###

https://godbolt.org/z/EYnhce8MK for reference.
All subsequent text below refers to the example in the godbolt above.

Anonymous unions never have their destructor invoked automatically.
Therefore we can skip vtable initialization of the destructor of a
dynamic class if that destructor effectively does no work.

This worked previously as the following check would be hit and return
true for the trivial anonymous union,
https://github.com/llvm/llvm-project/blob/release/18.x/clang/lib/CodeGen/CGClass.cpp#L1348,
resulting in the code skipping vtable initialization.

This was broken here
https://github.com/llvm/llvm-project/commit/982bbf404eba2d968afda5c674d4821652159c53
in relation to comments made on this review here
https://reviews.llvm.org/D10508.

### Fixes ###

The check the code is doing is correct however the return value is
inverted. We want to return true here since a field with anonymous union
never has its destructor invoked and thus effectively has a trivial
destructor body from the perspective of requiring vtable init in the
parent dynamic class.

Also added some extra missing unit tests to test for this use case and a
couple others.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications