[cfe-commits] r59666 - in /cfe/trunk: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorLexer.h lib/Lex/Lexer.cpp lib/Lex/PPDirectives.cpp lib/Lex/PPLexerChange.cpp lib/Lex/PTHLexer.cpp lib/Lex/PreprocessorLexer.cpp

[Chris Lattner]

On Nov 19, 2008, at 11:24 PM, Chris Lattner wrote:
>> +
>> +  static bool IsNonPragmaNonMacroLexer(const Lexer* L,
>> +                                       const PreprocessorLexer* P) {
>
> Please add doxygen comment to this and the others.

Actually, better yet, please invert the meaning of this method, naming  
it "isPragmaLexer()".  This is very simple and obvious.   
IsNonPragmaNonMacroLexer is not.

-Chris

>
>
>>
>> +    if (L)
>> +      return !L->isPragmaLexer();
>> +    else
>> +      return P != 0;
>
> How about:
>  return P != 0 || !L->isPragmaLexer(); ?
>
>> +public:
>> +  unsigned getFileID() const {
>> +    assert(PP &&
>> +      "PreprocessorLexer::getFileID() should only be used with a  
>> Preprocessor");
>> +    return FileID;
>> +  }
>
> Needs a doxygen comment.  Why the assert?
>
> -Chris
>