[cfe-commits] [patch][PR11797] Call ActOnFinishNamespaceDef before consumeClose

[Richard Smith]

On Wed, January 25, 2012 21:14, Richard Smith wrote:
> On Sat, January 21, 2012 07:03, Rafael Ávila de Espíndola wrote:
>> On 20/01/12 12:40 AM, Richard Smith wrote:
>>
>>> Perhaps it's time to review the handling of #pragma visibility. The
>>> existing mechanism is very flaky -- this is the second subtle bug we've
>>> had with it in recent months (the previous bug, which probably can still
>>> be observed in some cases, is that lookahead / tentative parsing which
>>> looks past such a #pragma applies its effects immediately).
>>
>> The attached patch implements your suggestion. It also fixes some old
>> FIXMEs in pragma-visibility.cpp.
>
> Thanks for digging into this! I have one high-level worry: I can't find any
> gcc documentation specifying where "#pragma GCC visibility" is legal -- but my
>  (superficial) testing shows it seems to reject the pragma in the same places
>  that we do once your patch is applied, so this seems a step towards GCC
> compatibility. I'm running this patch over a big pile of code, and I'll let
> you know if it turns up any other places where we should be accepting the
> pragma.
>
> The patch itself looks fine to me (though please hold back on the commit
> until my test run finishes). It's a pity we dynamically allocate a Token to
> carry the annotation (as we do for #pragma unused), but it looks like changing
> that would be invasive (and in any case, should be a separate change).

OK, this patch doesn't cause any problems with the code in my test corpus.
LGTM! If there are discrepancies between this and gcc's behavior (or whatever
we determine is the appropriate sane behavior), this seems to be close enough
to fix those in later patches.

- Richard