[cfe-dev] Patch to allow comment translators implementation

[Abramo Bagnara]

Il 19/01/2010 02:07, Chris Lattner ha scritto:
> On Jan 18, 2010, at 5:02 PM, Abramo Bagnara wrote:
>>>> Looks fine to me. Chris?
>>>
>>> Looks good to me, committed in r93795.  I didn't commit the
>>> lib/Lex/PPDirectives.cpp hunk, because it didn't look necessary.  Please
>>> correct me if I'm wrong.
>>
>> This part of the patch is strictly related to the others: the way for
>> CommentHandler to push tokens is to call EnterTokenStream and this
>> method add a TokenLexer (that was ignored by SkipExcludedConditionalBlock)
>>
>> Without this part of the patch the pushed tokens related to comments
>> found inside skipped conditional block are not read until return from
>> Preprocessor::SkipExcludedConditionalBlock and this is clearly unwanted.
> 
> I don't see that. According to C, the contents of #if 0 blocks are
> *completely* skipped and may not even be syntactically valid. Using
> openmp as an example, it would be perfectly find to have completely
> malformed openmp comments in #if 0 blocks.

Perhaps I've explained badly, please follow this example:

#if 0
/* openmp comments */
int a;
#endif

The preprocessor see the #if 0 directive and then enter
SkipExcludedConditionalBlock where the lexing proceed.

When we lex the comment the CommentHandler is invoked and it push some
tokens using EnterTokenStream (i.e. setting CurTokenLexer), but none of
this tokens is read by SkipExcludedConditionalBlock (without the patch
you've discarded).

This means that when SkipExcludedConditionalBlock returns and normal
lexing proceed the Preprocessor will read the tokens previously inserted
in the bad context i.e. after the #endif.

> #if 0 skipping is also performance sensitive.

I hardly believe that between:

    if (CurLexer)
      CurLexer->Lex(Tok);
    else
      CurPTHLexer->Lex(Tok);

and

  void Lex(Token &Result) {
    if (CurLexer)
      CurLexer->Lex(Result);
    else if (CurPTHLexer)
      CurPTHLexer->Lex(Result);
    else if (CurTokenLexer)
      CurTokenLexer->Lex(Result);
    else
      CachingLex(Result);
  }

there is a performance difference... I'm missing something?