ymandel added inline comments.
================
Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:93
+ Diagnostics << FixItHint::CreateRemoval(
+ CharSourceRange::getTokenRange(*PrevLoc, *PrevLoc));
+ } else {
----------------
JonasToth wrote:
> ymandel wrote:
> > JonasToth wrote:
> > > Twice `*PrevLoc`?
> > Is there a better alternative? I thought that, since token ranges closed on
> > both ends, this constructs a SourceRange that spans the single token at
> > PrevLoc. Instead, I could rework `getConstTokLocations` to return the
> > actual tokens instead, and then create CharSourceRanges from
> > Tok.getLocation(), Tok.getLastLocation().
> The Fixits work with sourcelocations, you dont need to get the tokenrange and
> CharSourceRange things. This looks more complicated then necessary to me.
> Your `findConstToRemove` can return a `Optional<SourceRange>` that encloses
> the `const` token. This range can then be used for the FixIts. I think this
> would make the code a bit clearer as well.
After reworking to pipe the Token all the way through to the use, I ended up
sticking with CharSourceRange::getCharRange(), because otherwise the fixithint
will convert a plain SourceRange into a token range, which is not what we want
(since we already have the lower-level char range from the lexed token).
================
Comment at: clang-tidy/utils/LexerUtils.cpp:41
+ const char *TokenBegin = File.data() + LocInfo.second;
+ Lexer RawLexer(SM.getLocForStartOfFile(LocInfo.first), Context.getLangOpts(),
+ File.begin(), TokenBegin, File.end());
----------------
JonasToth wrote:
> I think this function can be simplified as the manual lexing stuff seems
> unnecessary.
> Take a look at https://reviews.llvm.org/D51949#change-B_4XlTym3KPw that uses
> `clang::Lexer` functionality quite a bit to do manual lexing.
>
> You can use the public static methods from `clang::Lexer` to simplify this
> function.
(FWIW, this code was taken (almost directly) from
clang-tidy/readability/AvoidConstParamsInDecls.cpp.)
I agree that would potentially simplify the code here, but I'm not sure its the
right thing to do. Basically all of the relevant functions create a
clang::Lexer internally and do a bunch more work. So, it seems like this case,
where we are incrementally proceeding through a source, token by token, we
really should be using a (stateful) clang::Lexer directly, rather than
repeatedly calling into the static methods and creating/destroying a Lexer with
each such call.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53025
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
