This is an archive of the discontinued LLVM Phabricator instance.

[RFC] [AST] [Modules] Handle full cases of DefaultArgStorage::setInherited
ClosedPublic

Authored by ChuanqiXu on Jul 1 2022, 2:27 AM.

Details

Summary

There were two assertions in DefaultArgStorage::setInherited previously. It requires the DefaultArgument is either empty or an argument value. It would crash if it has a pointer refers to the previous declaration or contains a chain to the previous declaration.

But there are edge cases could hit them actually. One is InheritDefaultArguments.cppm that I found recently (there is another issue in the driver: https://github.com/llvm/llvm-project/issues/56327). Another one is pr31469.cpp, which was created fives years ago.

This patch tries to fix the two failures by handling full cases in DefaultArgStorage::setInherited.

This is guaranteed to not introduce any breaking change since it lives in the path we wouldn't touch before. And the added assertions for sameness should keep the correctness. But this is really fundamental, it would be better if someone else would like to take a look.

Diff Detail

Event Timeline

ChuanqiXu created this revision.Jul 1 2022, 2:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 1 2022, 2:27 AM
ChuanqiXu requested review of this revision.Jul 1 2022, 2:27 AM
ChuanqiXu updated this revision to Diff 442795.Jul 6 2022, 11:54 PM

Update tests.

ChuanqiXu updated this revision to Diff 443147.Jul 7 2022, 11:52 PM
ChuanqiXu retitled this revision from [AST] [Modules] Handle full cases of DefaultArgStorage::setInherited to [RFC] [AST] [Modules] Handle full cases of DefaultArgStorage::setInherited.
ChuanqiXu edited the summary of this revision. (Show Details)

Rebase with main.

ChuanqiXu edited the summary of this revision. (Show Details)Jul 7 2022, 11:52 PM
This revision is now accepted and ready to land.Jul 12 2022, 8:21 AM
This revision was landed with ongoing or failed builds.Jul 12 2022, 9:14 AM
This revision was automatically updated to reflect the committed changes.