[PATCH] Improve error recovery around colon.

[Serge Pavlov]

>>! In D3653#10, @rsmith wrote:
>> Fixing this case is a bit trickier because declspec is parsed with colon
>> protection turned off, otherwise we cannot recover typos like:
>>
>>   A:B c;
> 
> With your patch, it's parsed with colon protection turned on, right? Or do we also need to add colon protection to tentative parsing of member-declarations?
With this patch declspec is parsed with colon protection turned off, this allows make recover of errors like:


  struct S2b {
    C1:C2 f(C1::C2);
  };
  


or



  struct S6b {
    C1:C2 m1 : B1::B2; 
  };



Colon protection is turned on when parsing declarator, - when parser expects nested-name-specifier of pointer  to a member and when parsing direct-declarator.

> 
> If you want to detect and recover from the above case, you need more lookahead than you're currently using: you need to check if the colon is followed by *two* identifiers. If it is, you can temporarily turn off colon protection.

The patch uses a bit different way. If parser parses declspec and sees 

  A:B

it may be one of two things:


  - It is unnamed bit field. In this case `A` must be a type suitable for bit field and `B` must be a value.
  - It is typo for `A::B`. In this case `A::B` must be a type.
 
In both cases parser must read type first. As there is no case when both `A` and `A::B` are types suitable for declaring bit fields, these cases can be distinguished unambiguously, just by trying to build longest nested name specifier, which is still used to refer to a type. If `A::B` is a type or namespace, `A` cannot be used as a type of a bit field and recovery `A:B` -> `A::B` is valid. Without scoped enums it would be enough to check if `A::B` exists, that is why we need to pass `NeedType` when analyzing C++ scope.

================
Comment at: lib/Parse/Parser.cpp:1692-1694
@@ +1691,5 @@
+///
+/// Setting NeedType influences error recovery. If it is set, parser avoids
+/// replacements ':' -> '::' if it produces non-type entity. With it we can
+/// parse correctly code like this:
+/// \code
----------------
Richard Smith wrote:
> This doesn't seem correct. Even if `A::B::C` is a type, we still should not do the recovery here, because this may still be an unnamed bitfield.
> 
> Also, I'm concerned about whether this `NeedType` thing is correct, since we use `TryAnnotateCXXScopeSpec` from tentative parsing when we don't know whether we expect a type or not.
Whether the recovery is attempted is determined by `ColonIsSacred` field. `NeedType` only assists the recovering when  `ColonIsSacred` is false. By default `NeedType` is false and behavior is identical to the current one. In the patch it is set only when parsing declspec, to correctly handle case when `A` is scoped enum  and `A::B` is valid.

http://reviews.llvm.org/D3653