Skip to content
This repository has been archived by the owner on Jun 20, 2019. It is now read-only.

Bug 235: Fix ICE with array slice assign. #252

Merged
merged 2 commits into from
Oct 29, 2016

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Oct 26, 2016

I've been thinking of every angle to tackle this. But actually, the best way not to fall into a myriad of edge cases is just to not return every static array or aggregate in a TARGET_EXPR.

There may be an opportunity to move this to d_save_expr, but for now I'm just going with the conservative choice.

@ibuclaw
Copy link
Member Author

ibuclaw commented Oct 27, 2016

Still need to mark calls returning via NRVO with RETURN_SLOT_OPT.

Should only be applicable to INIT_EXPR assignments.

Should only be applicable to INIT_EXPR assignments.
@ibuclaw
Copy link
Member Author

ibuclaw commented Oct 27, 2016

@jpf91 - should be ready for review.

Copy link
Contributor

@jpf91 jpf91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I assume you tested i686 manually? (the multilib configuration does not really run all 32 bit tests, IIRC the 32 bit unittests don't run in multilib configurations)

I'll test ARM as well, though that will probably take some time ;-)

@ibuclaw
Copy link
Member Author

ibuclaw commented Oct 28, 2016

Yes, I checked and it doesn't create a self referencing target expr.

@jpf91
Copy link
Contributor

jpf91 commented Oct 29, 2016

OK, ARM tests passed. BTW: Is va_args C++ mangling architecture specific? I get this error with GCC 6.1.0 on ARM:

/tmp/ccKMJDyv.o: In function `myprintf(char const*, ...)':
cppa.d:(.text+0xcdc): undefined reference to `myvprintf(char const*, __va_list)'
/tmp/ccWjjwHd.o: In function `myvprintf(char const*, std::__va_list)':
cppb.cpp:(.text+0x52c): undefined reference to `myvprintfx(char const*, std::__va_list)'
collect2: error: ld returned 1 exit status
compiler exited with status 1
PASS: runnable/cppa.d (test for excess errors)

@jpf91 jpf91 merged commit 30764bd into D-Programming-GDC:master Oct 29, 2016
@ibuclaw ibuclaw deleted the issue235 branch October 29, 2016 07:05
@ibuclaw
Copy link
Member Author

ibuclaw commented Oct 29, 2016

Yes, it is indeed given an std namespace on arm. This is something to handle in a hook like Target::mangle in the frontend.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants