This is an archive of the discontinued LLVM Phabricator instance.

Fix one of the regressions found in revert of concept sugaring
ClosedPublic

Authored by erichkeane on Jan 24 2023, 12:52 PM.

Details

Summary

It seems that the sugaring patches had some pretty significant
interdependencies, and at least one issue exists that requires part of
the concept-sugaring patch. I don't believe this to have anything to do
with the increased compile-times that were seen, so hopefully this will
fix our problems without further regressions.

See https://reviews.llvm.org/rG12cb1cb3720de8d164196010123ce1a8901d8122
for discussion.

Diff Detail

Event Timeline

erichkeane created this revision.Jan 24 2023, 12:52 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2023, 12:52 PM

FYI I tested this patch with libc++ and as expected it does not fix the libc++ modular build ICE.

alexfh accepted this revision.Jan 25 2023, 2:32 AM

Thanks for preparing this fix! I've verified that it fixes the original reproducer as well.

This revision is now accepted and ready to land.Jan 25 2023, 2:32 AM
erichkeane added a comment.EditedJan 25 2023, 6:01 AM

FYI I tested this patch with libc++ and as expected it does not fix the libc++ modular build ICE.

A man can dream, eh? Thank you for testing it! I'm having troubles getting it reproduced unfortunately.

This revision was landed with ongoing or failed builds.Jan 25 2023, 6:01 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJan 25 2023, 6:01 AM

FYI I tested this patch with libc++ and as expected it does not fix the libc++ modular build ICE.

A man can dream, eh? Thank you for testing it! I'm having troubles getting it reproduced unfortunately.

Of course you can dream, and it would have been great if it had resolved the issue.

Do you have trouble with libc++ or your hardware?

FYI I tested this patch with libc++ and as expected it does not fix the libc++ modular build ICE.

A man can dream, eh? Thank you for testing it! I'm having troubles getting it reproduced unfortunately.

Of course you can dream, and it would have been great if it had resolved the issue.

Do you have trouble with libc++ or your hardware?

I was having trouble with libc++ and the config. The cmake you'd given me was either used poorly, or just didn't work. However, I was able to reproduce it with just the zip file you gave me.

FYI I tested this patch with libc++ and as expected it does not fix the libc++ modular build ICE.

A man can dream, eh? Thank you for testing it! I'm having troubles getting it reproduced unfortunately.

Of course you can dream, and it would have been great if it had resolved the issue.

Do you have trouble with libc++ or your hardware?

I was having trouble with libc++ and the config. The cmake you'd given me was either used poorly, or just didn't work. However, I was able to reproduce it with just the zip file you gave me.

Woops, it wasn't you :) It was @mstorsjo who gave me the zip file I can repro from :)

FYI I tested this patch with libc++ and as expected it does not fix the libc++ modular build ICE.

A man can dream, eh? Thank you for testing it! I'm having troubles getting it reproduced unfortunately.

Of course you can dream, and it would have been great if it had resolved the issue.

Do you have trouble with libc++ or your hardware?

I was having trouble with libc++ and the config. The cmake you'd given me was either used poorly, or just didn't work.

:-(

However, I was able to reproduce it with just the zip file you gave me.

Woops, it wasn't you :) It was @mstorsjo who gave me the zip file I can repro from :)

That's great!