[cfe-commits] r59723 - /cfe/trunk/lib/Lex/PPLexerChange.cpp

[Chris Lattner]

On Nov 19, 2008, at 11:56 PM, Ted Kremenek wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=59723&view=rev
> Log:
> Add ugly "test harness" for PTHLexer that is not enabled by default.  
> The
> (temporary hack) to test the PTHLexer is that whenever we would  
> create a Lexer
> object we instead raw lex a memory buffer first and then use the  
> PTHLexer. This
> logic exists only to driver the PTHLexer and will be removed/changed  
> in the
> future. Note that the regular path using normal Lexer objects is  
> what is used by
> default.

Ok, this is a great way to test this.

>
> +#if 1
>   Lexer *TheLexer = new Lexer(SourceLocation::getFileLoc(FileID, 0),  
> *this);
>   EnterSourceFileWithLexer(TheLexer, CurDir);
> +#else

Please split the #else body out into a helper method: GetPTHLexer(...)  
that returns the PTHLexer.  This code will eventually end up being the  
"create PTH" routine for a file.

> +  const llvm::MemoryBuffer* B = getSourceManager().getBuffer(FileID);
> +
> +  // Create a raw lexer.
> +  Lexer L(SourceLocation::getFileLoc(FileID, 0), getLangOptions(),
> +          B->getBufferStart(), B->getBufferEnd(), B);
> +
> +  // Ignore whitespace.
> +  L.SetKeepWhitespaceMode(false);
> +  L.SetCommentRetentionState(false);
> +
> +  // Lex the file, populating our data structures.
> +  std::vector<Token>* Tokens = new std::vector<Token>();

This vector is getting leaked.  PTHLexer should own it.

> +  Token Tok;
> +
> +  do {
> +    L.LexFromRawLexer(Tok);
> +
> +    if (Tok.is(tok::identifier))
> +      Tok.setIdentifierInfo(LookUpIdentifierInfo(Tok));
> +
> +    // Store the token.
> +    Tokens->push_back(Tok);
> +  }
> +  while (Tok.isNot(tok::eof));

This logic will eventually need to get a bit more complex to handle  
angled strings, skip-to-the-end-of-line etc.

> +  if (CurPPLexer || CurTokenLexer)
> +    PushIncludeMacroStack();
> +
> +  CurDirLookup = CurDir;
> +  SourceLocation Loc = SourceLocation::getFileLoc(FileID, 0);
> +  CurPTHLexer.reset(new PTHLexer(*this, Loc, &(*Tokens)[0], Tokens- 
> >size()));

I think the PTHLexer ctor should take a fileid not a SLoc.

>
>       if (CurLexer) {
> -        // FIXME: Should we use the location of 'Result'?
>         Callbacks->FileChanged(CurLexer->getSourceLocation(CurLexer- 
> >BufferPtr),
>                                PPCallbacks::ExitFile, FileType);
>       }
>       else {
> -        assert (0 && "FIXME: Add callback support for PTHLexer.");
> +        // FIXME: Is this okay to use the location of 'Result'?
> +        Callbacks->FileChanged(Result.getLocation(),  
> PPCallbacks::ExitFile,
> +                               FileType);

The answer is "no".  Please use assert(0) or fix this.

-Chris