<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jun 10, 2014 at 2:25 AM, Serge Pavlov <span dir="ltr"><<a href="mailto:sepavloff@gmail.com" target="_blank">sepavloff@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">>>! In D3653#10, @rsmith wrote:<br>
>> Fixing this case is a bit trickier because declspec is parsed with colon<br>
>> protection turned off, otherwise we cannot recover typos like:<br>
>><br>
>> A:B c;<br>
><br>
> 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?<br>
</div>With this patch declspec is parsed with colon protection turned off,</blockquote><div><br></div><div>This seems like the wrong design; we should have colon protection turned on for this. If there are cases where we know it's safe to treat a ':' as '::' anyway, then that should be special-cased.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> this allows make recover of errors like:<br>
<br>
<br>
struct S2b {<br>
C1:C2 f(C1::C2);<br>
};<br>
<br>
<br>
<br>
or<br>
<br>
<br>
<br>
struct S6b {<br>
C1:C2 m1 : B1::B2;<br>
};<br>
<br>
<br>
<br>
Colon protection is turned on when parsing declarator, - when parser expects nested-name-specifier of pointer to a member and when parsing direct-declarator.<br>
<div class=""><br>
><br>
> 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.<br>
<br>
</div>The patch uses a bit different way. If parser parses declspec and sees<br>
<br>
A:B<br>
<br>
it may be one of two things:<br>
<br>
<br>
- It is unnamed bit field. In this case `A` must be a type suitable for bit field and `B` must be a value.<br></blockquote><div><br></div><div>There's no need for 'B' to be a value here. It could be a type. It could be a function found by ADL. You can't assume that A:B is not a bit-field by looking up B.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
- It is typo for `A::B`. In this case `A::B` must be a type.<br>
<br>
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.</blockquote>
<div><br></div><div>I'm not sure this argument is correct. However, I think it is correct to say: if A is non-dependent, and not valid as the type of a bit-field, and we're parsing the decl-specifiers in a member declaration, then it is safe to treat A:B as a typo for A::B (doing so will never reject valid code).</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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.<br>
<div class=""><br>
================<br>
Comment at: lib/Parse/Parser.cpp:1692-1694<br>
@@ +1691,5 @@<br>
+///<br>
+/// Setting NeedType influences error recovery. If it is set, parser avoids<br>
+/// replacements ':' -> '::' if it produces non-type entity. With it we can<br>
+/// parse correctly code like this:<br>
+/// \code<br>
----------------<br>
</div><div class="">Richard Smith wrote:<br>
> 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.<br>
><br>
> 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.<br>
</div>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.<br>
<br>
<a href="http://reviews.llvm.org/D3653" target="_blank">http://reviews.llvm.org/D3653</a><br>
<br>
<br>
</blockquote></div><br></div></div>