On Jul 27, 2011, at 10:47 AM, Chris Lattner wrote: > > On Jul 26, 2011, at 3:34 PM, Anna Zaks wrote: > >> Hi, >> >> Attached are two patches, which add a fixit to remove an unused label. >> >> 1) Add a utility function to the Lexer, which makes it easier to find the >> ':' after the label as well as the new line and trailing spaces (It is a >> generalized version of trans::findLocationAfterSemi from ArcMigrate, which >> will be changed to use the Lexer utility). >> <unusedLabel_LexSupport.diff> > > The Lexer part of this looks quite nice, with a few comments. It's great to > have this in a centralized place where lots of clients can use it. > > In Lexer.h, please also add a non-static version of FindLocationAfterToken > which doesn't require LangOpts and SourceMgr, just as a convenience (it > should forward to the version you already have). >
Lexer does not currently have a SourceMgr member. Adding a non-static method just for the convenience of not passing LangOpts might be an overkill? > SourceLocation loc.. > > Please capitalize local variable names. > > + // Lex from the start of the given location. > + Lexer lexer(SM.getLocForStartOfFile(locInfo.first), LangOpts, file.begin(), > + tokenBegin, file.end()); > > it seems that it would be more convenient to use this Lexer ctor: > > Lexer(FileID FID, const llvm::MemoryBuffer *InputBuffer, > const SourceManager &SM, const LangOptions &Features); > We need the lexer to start processing the buffer starting from the specific position Loc. This constructor seems to be convenient if we were to start lexing from the beginning of the file. > + const char *tokenEnd = file.data() + > + SM.getDecomposedLoc(TokenLoc).second + > + tok.getLength(); > > This can use SM.getCharacterData(TokenLoc) > > > Otherwise, this looks great! Thanks for the review! Anna. > > -Chris > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
