This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.
ClosedPublic

Authored by delcypher on Feb 23 2021, 12:22 PM.

Details

Summary

This change simplifies clang/lib/Frontend/CompilerInvocation.cpp
because we no longer need to manually parse the flag and set codegen
options in the frontend. However, we still need to manually parse the
flag in the driver because:

  • The marshalling infrastructure doesn't operate there.
  • We need to do some platform specific checks in the driver that will likely never be supported by any kind of marshalling infrastructure.

rdar://71609176

Diff Detail

Event Timeline

delcypher created this revision.Feb 23 2021, 12:22 PM
delcypher requested review of this revision.Feb 23 2021, 12:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2021, 12:22 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
yln accepted this revision.Feb 23 2021, 1:11 PM

LGTM, cleanup only, right?

This revision is now accepted and ready to land.Feb 23 2021, 1:11 PM
jansvoboda11 accepted this revision.Feb 24 2021, 10:07 AM

LGTM.

If you don't get around committing this today, please rebase this on top of D97375 that I'm going to commit tomorrow.

LGTM.

If you don't get around committing this today, please rebase this on top of D97375 that I'm going to commit tomorrow.

@jansvoboda11 This doesn't build anymore because https://reviews.llvm.org/D97375 removed AutoNormalizeEnum. Based on skim reading the patch it looks like I should remove the AutoNormalizeEnum mixin and replace MarshallingInfoString with MarshallingInfoEnum. Does that sound right?

In D97327#2582981, @yln wrote:

LGTM, cleanup only, right?

Yes. This is just a small clean up.

@jansvoboda11 This doesn't build anymore because https://reviews.llvm.org/D97375 removed AutoNormalizeEnum. Based on skim reading the patch it looks like I should remove the AutoNormalizeEnum mixin and replace MarshallingInfoString with MarshallingInfoEnum. Does that sound right?

That's exactly right.

delcypher updated this revision to Diff 326474.Feb 25 2021, 1:23 PM
  • Fix build.
This revision was landed with ongoing or failed builds.Feb 25 2021, 1:25 PM
This revision was automatically updated to reflect the committed changes.