aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/UseAutoCheck.cpp:378
+ Lexer::getLocForEndOfToken(Range.getEnd(), 0, SM,
+ bool Whitespace = isWhitespace(*FullSourceLoc(Next, SM).getCharacterData());
> aaron.ballman wrote:
> > malcolm.parsons wrote:
> > > aaron.ballman wrote:
> > > > Oye, this is deceptively expensive because you now have to go back to
> > > > the actual source file for this information. That source file may live
> > > > on a network share somewhere, for instance.
> > > >
> > > > Can you use `Token::hasLeadingSpace()` instead?
> > > >
> > > > Also, doesn't this still need to care about the `RemoveStars` option?
> > > Where would I get a Token from?
> > Hrm, might not be as trivial as I was hoping (I thought we had a way to go
> > from a `SourceLocation` back to a `Token`, but I'm not seeing one
> > off-hand). Regardless, I worry about the expense of going all the way back
> > to the source for this.
> > @alexfh -- should this functionality be a part of a more general "we've
> > made a fixit, now make it not look ugly?" pass? At least then, if we go
> > back to the source, we can do it in a more controlled manner and hopefully
> > get some performance back from that.
> `LineIsMarkedWithNOLINT` is going to read the source anyway, so I don't see
> any additional expense.
The additional expense comes from many checks needing similar functionality --
generally, fixit replacements are going to require formatting modifications of
some sort. It's better to handle all of those in the same place and
transparently rather than have every check with a fixit manually handle
formatting. Additionally, this means we can go back to the source as
infrequently as possible.
cfe-commits mailing list