[cfe-commits] [PATCH] Implementation quantity limits to prevent crashes

[Douglas Gregor]

On Sep 21, 2011, at 8:45 PM, Aaron Ballman wrote:

> On Wed, Sep 21, 2011 at 8:11 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>> Also, the diagnostic wording was taken from MSVC, but could likely be
>>> improved.  Suggestions welcome.
>> 
>> I would say something more along the lines of "parser recusion limit
>> reached"; using the term "stack overflow" makes it sound like a bug in
>> the compiler.
> 
> Here is a revised patch with the new diagnostic wording.

Minor stuff: there's a typo "recusion" in the diagnostic wording, and the new test case hasn't been updated for the new wording.

I like this general direction, but I do wonder about adding another RAII object for this. You've added the RAII object in two places, but aren't there many other similar places? Can we be more methodical, so we can be sure that we don't lose the benefits of this checking with later maintenance?

I have one idea: what if we expanded the mandate of this RAII object a bit, so that it was responsible for both balancing the delimiters (parentheses, square brackets, braces, etc.)? In other words, we introduce the RAII object to consume the opening paren/bracket/brace and also to find the matching ')':

  BalancedDelimiterTracket Parens(*this, tok::l_paren); // tries to consume the opening '('
  if (!Parens) // checks whether an error occurred in consuming the opening ')'
    return error;

  // parse whatever is in the parens

  if (Parens.close()) { // tries to consume the closing ')'
    // couldn't find closing ')'
  }

The idea here would be to encapsulate all of the behavior that now uses MatchRHSPunctuation into an RAII object, and have that also check for implementation limits. There are three benefits here:

  (1) It becomes clearer where we're matching delimited sets, so the structure of the parser improves in general.
  (2) Implementation limits checking comes "free" when we use the right abstraction, so it won't be forgotten or lost.
  (3) These RAII objects, in aggregate, outline the source as we're parsing it. We should be able to use that information to improve parser recovery.

What do you think?

	- Doug