[cfe-dev] Bug 3444: Switch and cases

[Sebastian Redl]

Hi,

I'd like to get your input on bug 3444[1].
Short summary: when an error during the parsing of the compound
statement occurs (mainly a missing end brace), it is deallocated, and
all contained statements too. This means that the case statements are
deallocated, but they're still referenced by the switch's case list. So
when it comes to the switch statement examining these case statements,
there's trouble. Clang crashes.

In the bug discussion, we've identified three possible approaches.

1) Pass Action::OnFinishSwitchStmt a null pointer as the body instead of
a null statement. This causes Sema to just pop the switch stack and
return immediately.
2) Have ParseCompoundStmtBody return a valid statement even if there is
an error, thus keeping the cases alive.
3) Make the switch case chain a double-linked list so that case
statements can unhook themselves when they get destroyed.

The problem with option 1 is that it's not entirely clear that it is a
sufficient solution. Are there any circumstances where a case statement
could be thrown away, but the final compound statement appears valid?

The problem with option 2 is that it messes with other stuff using
compound statements. In particular, do...while statements start emitting
previously suppressed errors about missing the while clause if the
compound statement fails to be parsed. Also, the approach is fragile,
because some other part of validation could then throw the compound
statement away.

The problem with option 3 is that it is the biggest intrusion into the
system. The other two are 5-line patches that I had written within a
minute each. This one is big, because it requires some slight
restructuring of the AST hierarchy so that unlinking really works. Also,
it adds another pointer to switches, cases and default cases. Chris also
raised a concern about the destructor not being called when the bump
allocator is used and smart pointers are disabled, but this is not a
problem, since in that case the case statements aren't deallocated anyway.

Do you have comments about these options or further ideas?


[1] http://llvm.org/bugs/show_bug.cgi?id=3444