alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
+ callExpr(callee(functionDecl(TwoDoubleArgFns, parameterCountIs(2),
+ hasBuiltinTyParam(0, DoubleTy),
+ hasBuiltinTyParam(1, DoubleTy))),
+ hasBuiltinTyArg(0, FloatTy), hasBuiltinTyArg(1, FloatTy))
> alexfh wrote:
> > I guess, functions with arbitrary number of parameters can all be handled
> > using `forEachArgumentWithParam`. Roughly like this:
> > forEachArgumentWithParam(
> > hasType(isBuiltinType(FloatTy)),
> > parmVarDecl(hasType(isBuiltinType(DoubleTy)))
> > One difference to your existing implementation will be that it will match
> > calls where at least one parameter is double and argument is float, not all
> > of them. Do you expect this to make the check more noisy?
> > Do you expect this to make the check more noisy?
> Yes, to the point of not being worth doing, I think.
> Specifically, I think there is nothing wrong with calling a two-arg function
> double function with one float and one double arg and expecting the float arg
> to be promoted: `::hypot(3.f, 4.)` probably *should* call the double version.
> So checking that the arg types are all `float` is important, I think.
> Checking that the parameter types are all `double` is less important but also
> worth doing, I think, so if you declare some bizarre function in the global
> namespace called e.g. `::hypot`, we won't suggest changing that to `::hypotf`.
Fair enough. I don't like the verbosity of the code here, but it may really be
valuable to be strict here.
+ // Skip the "::" following the qualifier.
+ FnNameStart = D->getQualifierLoc().getEndLoc().getLocWithOffset(2);
> alexfh wrote:
> > `getLocWithOffset` makes the code quite brittle. Imagine whitespace around
> > `::`, for example. Same below. In order to make this kind of code more
> > robust, you can operate on tokens (using Lexer).
> > Same below.
> Hm. I agree this is super-brittle. But I am having a lot of difficulty
> using the Lexer successfully.
> For one thing, there may be comments basically anywhere in this stream, and I
> have to skip over them. I see getPreviousNonCommentToken, but it doesn't
> quite work if I need to go *forward* in the stream. I looked at a bunch of
> other uses of the Lexer in clang-tidy and they all looked pretty different
> from what I'm trying to do here, which also suggested that maybe I'm doing
> the wrong thing.
> Is there no way to get the source location of "sin" out of the DeclRefExpr
> for "::sin"? I don't see it, but it seems bizarre that it wouldn't be there.
> Any tips would be much appreciated.
Skipping from `::` to the next identifier should be straightforward using
Lexer::getRawToken and Lexer::getLocForEndOfToken. Tell me if you need more
cfe-commits mailing list