This is an archive of the discontinued LLVM Phabricator instance.

Don't crash when code completing `using enum ^Foo`.
ClosedPublic

Authored by sammccall on Sep 19 2022, 4:40 PM.

Diff Detail

Event Timeline

sammccall created this revision.Sep 19 2022, 4:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2022, 4:40 PM
Herald added a subscriber: kadircet. · View Herald Transcript
sammccall requested review of this revision.Sep 19 2022, 4:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 19 2022, 4:40 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
sammccall added inline comments.Sep 19 2022, 4:45 PM
clang/lib/Parse/ParseDecl.cpp
4596

this is a bit unusual, usually parse() functions just stop after hitting CC, as if they hit something unexpected.

However ParseUsingDeclaration immediately calls ActOnUsingDeclaration, which asserts that the enum specifier we parsed was one of {enum, typename, error}. It seems simpler to signal failure using the existing mechanism than add a new one - LMK what you think.

Might be mooted by fixing https://github.com/llvm/llvm-project/issues/57659, which I am working on

Might be mooted by fixing https://github.com/llvm/llvm-project/issues/57659, which I am working on

Great! Agree that larger changes around using-enum parsing would probably end up solving this in some better way.

Do you mind if I land it anyway? The testcase is useful even if the impl changes.
(Plus this is crashing for us in production, so just in case that larger change takes longer...)

ilya-biryukov accepted this revision.Sep 20 2022, 6:58 AM

LGTM to fix the crash.
If @urnathan's changes happen to make this change obsolete, we could also revert consider reverting it afterwards.

clang/lib/Parse/ParseDecl.cpp
4596

Could you add a comment mentioning that ActOnUsingDeclaration will break if we don't do this?

This revision is now accepted and ready to land.Sep 20 2022, 6:58 AM
nridge added a subscriber: nridge.Sep 21 2022, 12:53 AM
This revision was landed with ongoing or failed builds.Sep 25 2022, 7:52 PM
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.