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

[Richard Smith]

Hi Rafael,

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).

Thanks!
Richard