Page MenuHomePhabricator

[clang] Use SourceLocations in unions [NFCI]
ClosedPublic

Authored by miyuki on Thu, Jan 7, 8:48 AM.

Details

Summary

Currently, there are many instances where SourceLocation objects are
converted to raw representation to be stored in structs that are
used as fields of tagged unions.

This is done to make the corresponding structs trivial.
Triviality allows avoiding undefined behavior when implicitly changing
the active member of the union.

However, in most cases, we can explicitly construct an active member
using placement new. This patch adds the required active member
selections and replaces SourceLocation-s represented as
unsigned int with proper SourceLocation-s.

One notable exception is DeclarationNameLoc: the objects of this class
are often not properly initialized (so the code currently relies on
its default constructor which uses memset). This class will be fixed
in a separate patch.

Diff Detail

Event Timeline

miyuki created this revision.Thu, Jan 7, 8:48 AM
miyuki requested review of this revision.Thu, Jan 7, 8:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptThu, Jan 7, 8:48 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
miyuki retitled this revision from [clang] Use SourceLocations in unions to [clang] Use SourceLocations in unions [NFCI].Thu, Jan 7, 8:49 AM
miyuki updated this revision to Diff 315759.Mon, Jan 11, 4:39 AM

Fixed a typo in the word "implicit"

Do you need to run the destructor before placement new in these situations?

Do you need to run the destructor before placement new in these situations?

No, because there is no active union member in any of those situations. And if even if there was an active member, all destructors are trivial anyway.

miyuki updated this revision to Diff 316414.Wed, Jan 13, 8:38 AM

Refactored SLocEntry in a similar way.

dblaikie accepted this revision.Wed, Jan 13, 12:57 PM

Sounds good

This revision is now accepted and ready to land.Wed, Jan 13, 12:57 PM
This revision was landed with ongoing or failed builds.Thu, Jan 14, 2:57 AM
This revision was automatically updated to reflect the committed changes.

This revision was landed with ongoing or failed builds.

The only failures were clang-format warnings, they looked bogus to me.