Page MenuHomePhabricator

[clang][CodeComplete] Ensure there are no crashes when completing with ParenListExprs as LHS

Authored by kadircet on Feb 18 2021, 4:49 AM.

Diff Detail

Event Timeline

kadircet requested review of this revision.Feb 18 2021, 4:49 AM
kadircet created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 18 2021, 4:49 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
sammccall added inline comments.Feb 18 2021, 5:19 AM

I think we should merge the two comments, something like...

/// If LHS is ParenListExpr, assume a chain of comma operators and pick the last expr.
/// We expect other ParenListExprs to be resolved to e.g. constructor calls before here.
/// (So the ParenListExpr should be nonempty, but check just in case)

now this is getting a little more complicated, we could pull out a function Expr* unwrapParenList(Expr*) and avoid duplicating code & comments, but up to you.


I'm not actually sure we should fix this. Nonempty is a subtle invariant that could also be violated by e.g. broken code. And the check is cheap.


I think you want OtherOpBase = null here, not return

kadircet updated this revision to Diff 324945.Feb 19 2021, 4:31 AM
kadircet marked 4 inline comments as done.
  • Pull unwrapping of parenlistexprs into a function
  • Merge comments
sammccall accepted this revision.Feb 22 2021, 12:32 AM
This revision is now accepted and ready to land.Feb 22 2021, 12:32 AM
This revision was landed with ongoing or failed builds.Feb 22 2021, 1:01 AM
This revision was automatically updated to reflect the committed changes.