Fix detecting type responsible for operator function for function templates #399
Conversation
Check function parameters types instead of types of arguments at call site. This way we are able to detect template substitutions for the function itself, not for the caller.
operator== calls I1_const_ptr<T>::operator* and after Clang r289250 we
correctly detect that the return type of
const T& operator*() { return *ptr_; }
is LValueReferenceType and doesn't require full type use.
| // Handle free functions. | ||
| int params_count = callee_decl->getNumParams(); | ||
| for (int i = 0; i < params_count; i++) { | ||
| // View argument types from the perspective of function body, not from |
| for (int i = 0; i < params_count; i++) { | ||
| // View argument types from the perspective of function body, not from | ||
| // the caller's perspective. For example, function parameter can have | ||
| // template type but function argument is not necesserily a template |
There was a problem hiding this comment.
Fixed. Thanks for catching this.
| // In this case ('myclass >> os'), we want to be returning the | ||
| // type of os, not of myclass, and we do, because myclass will be | ||
| // a SubstTemplateTypeParmType, not a RecordType. | ||
| if (isa<SubstTemplateTypeParmType>(param_type)) |
There was a problem hiding this comment.
Is this path ever hit now? It's a bit surprising that a parameter (as opposed to an argument) would be a SubstTemplateTypeParmType. I would have expected STTPT to only show up in the AST when instantiating a template/invoking a function template, but it's hard to tell from the docs what the respective nodes mean.
There was a problem hiding this comment.
This is still required. Tried to remove it and GetFirstClassArgument wasn't doing its job.
There was a problem hiding this comment.
OK, let's leave it at that :-)
| return expr->getArg(0); | ||
| } | ||
| // Handle free functions. | ||
| int params_count = callee_decl->getNumParams(); |
There was a problem hiding this comment.
Can we put in a CHECK_ here that params_count == expr's arg count?
There was a problem hiding this comment.
... and actually, is that a strong invariant? I would expect things like:
void f(int a, int b=0);
f(10);
to break that.
Can't think of a case with operators and default arguments, so not sure if it's relevant.
There was a problem hiding this comment.
Added CHECK_. And regarding default arguments, I've tried it and hit a compiler error
tests/cxx/repo.cpp:28:39: error: parameter of overloaded 'operator==' cannot have a default argument
bool operator==(Cc_Class& object, int x = 0) {
^ ~
|
Addressed the comments, please check again. |
|
Looks good to me, please merge. |
Check function parameters types instead of types of arguments at call site. This way we are able to detect template substitutions for the function itself, not for the caller. PR include-what-you-use#399
operator== calls I1_const_ptr<T>::operator* and after Clang r289250 we
correctly detect that the return type of
const T& operator*() { return *ptr_; }
is LValueReferenceType and doesn't require full type use.
PR include-what-you-use#399
Check function parameters types instead of types of arguments at call site. This way we are able to detect template substitutions for the function itself, not for the caller. PR include-what-you-use/include-what-you-use#399
operator== calls I1_const_ptr<T>::operator* and after Clang r289250 we
correctly detect that the return type of
const T& operator*() { return *ptr_; }
is LValueReferenceType and doesn't require full type use.
PR include-what-you-use/include-what-you-use#399
Relevant discussion can be found in issue #384: Build fails after Clang r289250.