Page MenuHomePhabricator

Implement codegen for MSVC unions with reference members
ClosedPublic

Authored by domdom on May 29 2019, 12:17 AM.

Details

Summary

Currently, clang accepts a union with a reference member when given the -fms-extensions flag. This change fixes the codegen for this case.

Diff Detail

Event Timeline

domdom created this revision.May 29 2019, 12:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 29 2019, 12:17 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
asl resigned from this revision.May 30 2019, 12:33 PM
domdom edited reviewers, added: aaron.ballman; removed: asl.May 30 2019, 5:12 PM
aaron.ballman added subscribers: majnemer, rnk.

This looks reasonable to me, but @rnk and @majnemer may have opinions as well.

clang/test/CodeGenCXX/ms-union-member-ref.cpp
4–7

Can you clang-format the patch?

domdom updated this revision to Diff 205495.Jun 18 2019, 6:53 PM

clang-formatted the test.

domdom marked an inline comment as done.Jun 18 2019, 6:53 PM

Ping!

Cheers,
Dom

aaron.ballman accepted this revision.Aug 21 2019, 7:01 AM

LGTM, thank you! I think this is actually needed to properly support MFC on Windows, IIRC.

This revision is now accepted and ready to land.Aug 21 2019, 7:01 AM

Thanks @aaron.ballman!

I will need someone to commit this for me :)

Cheers,
Dom

Thanks @aaron.ballman!

I will need someone to commit this for me :)

I'm happy to commit for you, but I get merge conflicts when trying to apply your patch. Can you rebase on trunk?

domdom updated this revision to Diff 217289.Aug 26 2019, 6:21 PM

Rebased onto master, clang format the patch.

Merge conflict resolve by having the bitcast of the field reference happening after recording access index.

aaron.ballman closed this revision.Aug 27 2019, 5:43 AM

Rebased onto master, clang format the patch.

Merge conflict resolve by having the bitcast of the field reference happening after recording access index.

Thanks! I've committed in r370052.