This is an archive of the discontinued LLVM Phabricator instance.

[Sema] Fix bug in handling of designated initializer
ClosedPublic

Authored by ahatanak on Jan 13 2017, 2:43 PM.

Details

Summary

CheckDesignatedInitializer wasn't taking into account the base classes when computing the index for the field in the derived class, which caused the test case to crash during IRGen because of a malformed AST.

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak updated this revision to Diff 84390.Jan 13 2017, 2:43 PM
ahatanak retitled this revision from to [Sema] Fix bug in handling of designated initializer.
ahatanak updated this object.
ahatanak added reviewers: rsmith, rnk, arphaman.
ahatanak added a subscriber: cfe-commits.
rnk edited edge metadata.Jan 13 2017, 4:31 PM

What happens with virtual bases?

struct B { int x; };
struct D : virtual B { int y; };
void test() { D d = {1, .y = 2}; }
lib/Sema/SemaInit.cpp
2250 ↗(On Diff #84390)

Do this before counting fields, IMO it's more intuitive.

test/SemaCXX/designated-initializers-base-class.cpp
1 ↗(On Diff #84390)

This might be less fragile as an IRGen test. Alternatively, this is a good expected-no-diagnostic test:

void test() { D d = {1, .y = 2}; }

Right now we emit warning: initializer overrides prior initialization of this subobject, which is obviously wrong.

ahatanak updated this revision to Diff 84418.Jan 13 2017, 6:46 PM
ahatanak edited edge metadata.
ahatanak marked 2 inline comments as done.

Address review comments.

In D28705#646088, @rnk wrote:

What happens with virtual bases?

struct B { int x; };
struct D : virtual B { int y; };
void test() { D d = {1, .y = 2}; }

A class with virtual base is not considered an aggregate, so it doesn't go through aggregate initialization (and therefore it doesn't enter CheckDesignatedInitializer). Instead, in TryListInitialization, it tries to find a matching constructor of D and fails.

test/SemaCXX/designated-initializers-base-class.cpp
1 ↗(On Diff #84390)

good idea.

arphaman accepted this revision.Jan 16 2017, 10:58 AM
arphaman edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 16 2017, 10:58 AM
rnk accepted this revision.Jan 17 2017, 9:45 AM

lgtm

This revision was automatically updated to reflect the committed changes.