[Patch] PR14995 Too eagerly rejects operator function templates
Rahul Jain
1989.rahuljain at gmail.com
Thu Jan 30 21:55:27 PST 2014
Thanks Richard, I have closed the bug.
Thanks to Suyog Sarda for working with me on this!
On Fri, Jan 31, 2014 at 4:36 AM, Richard Smith <metafoo at gmail.com> wrote:
> On Thu Jan 30 2014 at 2:07:48 PM, rahul <1989.rahuljain at gmail.com> wrote:
>
>>
>> Thanks Richard!
>>
>> Yes, please if you could commit it for me.
>> I will go ahead and close the bug once the
>> fix is committed!
>>
>
> Thanks, committed as r200496.
>
>
>> Thanks,
>> Rahul
>>
>>
>> On 31-Jan-2014, at 3:26 am, Richard Smith <metafoo at gmail.com> wrote:
>>
>> LGTM, thanks. The new test coverage looks great. Do you need someone to
>> commit this for you?
>>
>> On Thu Jan 30 2014 at 1:48:13 PM, Rahul Jain <1989.rahuljain at gmail.com>
>> wrote:
>>
>>
>> Hi Richard,
>>
>> Thanks for the inputs.
>>
>> I have updated the patch as per your review comments.
>> Please if you could help by reviewing the same!
>>
>>
>> Index: lib/Sema/SemaDeclCXX.cpp
>> ===================================================================
>> --- lib/Sema/SemaDeclCXX.cpp (revision 200475)
>> +++ lib/Sema/SemaDeclCXX.cpp (working copy)
>> @@ -10909,11 +10909,10 @@
>> // increment operator ++ for objects of that type.
>> if ((Op == OO_PlusPlus || Op == OO_MinusMinus) && NumParams == 2) {
>> ParmVarDecl *LastParam = FnDecl->getParamDecl(FnDecl->getNumParams()
>> - 1);
>> - bool ParamIsInt = false;
>> - if (const BuiltinType *BT =
>> LastParam->getType()->getAs<BuiltinType>())
>> - ParamIsInt = BT->getKind() == BuiltinType::Int;
>> + QualType ParamType = LastParam->getType();
>>
>> - if (!ParamIsInt)
>> + if (!ParamType->isSpecificBuiltinType(BuiltinType::Int) &&
>> + !ParamType->isDependentType())
>> return Diag(LastParam->getLocation(),
>> diag::err_operator_overload_post_incdec_must_be_int)
>> << LastParam->getType() << (Op == OO_MinusMinus);
>> Index: test/SemaCXX/overloaded-operator.cpp
>> ===================================================================
>> --- test/SemaCXX/overloaded-operator.cpp (revision 200475)
>> +++ test/SemaCXX/overloaded-operator.cpp (working copy)
>> @@ -452,3 +452,58 @@
>> Result = 1; // expected-error {{no viable overloaded '='}} //
>> expected-note {{type 'PointerUnion<int *, float *>' is incomplete}}
>> }
>> }
>> +
>> +namespace PR14995 {
>> + struct B {};
>> + template<typename ...T> void operator++(B, T...) {}
>> +
>> + void f() {
>> + B b;
>> + b++; // ok
>> + ++b; // ok
>> + }
>> +
>> + template<typename... T>
>> + struct C {
>> + void operator-- (T...) {}
>> + };
>> +
>> + void g() {
>> + C<int> postfix;
>> + C<> prefix;
>> + postfix--; // ok
>> + --prefix; // ok
>> + }
>> +
>> + struct D {};
>> + template<typename T> void operator++(D, T) {}
>> +
>> + void h() {
>> + D d;
>> + d++; // ok
>> + ++d; // expected-error{{cannot increment value of type 'PR14995::D'}}
>> + }
>> +
>> + template<typename...T> struct E {
>> + void operator++(T...) {} // expected-error{{parameter of overloaded
>> post-increment operator must have type 'int' (not 'char')}}
>> + };
>> +
>> + E<char> e; // expected-note {{in instantiation of template class
>> 'PR14995::E<char>' requested here}}
>> +
>> + struct F {
>> + template<typename... T>
>> + int operator++ (T...) {}
>> + };
>> +
>> + int k1 = F().operator++(0, 0);
>> + int k2 = F().operator++('0');
>> + // expected-error at -5 {{overloaded 'operator++' must be a unary or
>> binary operator}}
>> + // expected-note at -3 {{in instantiation of function template
>> specialization 'PR14995::F::operator++<int, int>' requested here}}
>> + // expected-error at -4 {{no matching member function for call to
>> 'operator++'}}
>> + // expected-note at -8 {{candidate template ignored: substitution
>> failure}}
>> + // expected-error at -9 {{parameter of overloaded post-increment
>> operator must have type 'int' (not 'char')}}
>> + // expected-note at -6 {{in instantiation of function template
>> specialization 'PR14995::F::operator++<char>' requested here}}
>> + // expected-error at -7 {{no matching member function for call to
>> 'operator++'}}
>> + // expected-note at -12 {{candidate template ignored: substitution
>> failure}}
>> +} // namespace PR14995
>>
>>
>> Thanks,
>> Rahul
>>
>>
>> On Fri, Jan 31, 2014 at 12:35 AM, Richard Smith <metafoo at gmail.com>wrote:
>>
>> Please do add those other testcases. Please also add some tests which
>> fail when we come to instantiate the operator++, such as:
>>
>> template<typename...T> struct A { void operator++(T...); };
>> A<char> a;
>>
>> ... and ...
>>
>> struct A { template<typename...T> int operator++(T...); };
>> int k1 = A().operator++(0, 0);
>> int k2 = A().operator++('0');
>>
>>
>> Also:
>>
>> - if (!ParamIsInt)
>> + bool ParamIsDependent = false;
>> + if (LastParam->getType()->isDependentType())
>> + ParamIsDependent = true;
>> +
>> + if (!ParamIsInt && !ParamIsDependent)
>>
>> This seems overly verbose. Please tidy this up; something like:
>>
>> QualType ParamType = LastParam->getType();
>> if (!ParamType->isSpecificBuiltinType(BuiltinType::Int) &&
>> !ParamType->isDependentType())
>>
>> ... would be better.
>>
>> On Thu Jan 30 2014 at 10:07:33 AM, rahul <1989.rahuljain at gmail.com>
>> wrote:
>>
>> Hi Arthur,
>>
>> Yes it does fix the other two examples as well. I did check them too
>> while working out the fix.
>>
>> I will add those too once I get a green signal for the fix :)
>>
>> Thanks,
>> Rahul
>>
>>
>> On 30-Jan-2014, at 11:29 pm, "Arthur O'Dwyer" <arthur.j.odwyer at gmail.com>
>> wrote:
>>
>> Hi Rahul,
>>
>> Does your patch fix the other two examples given in PR14995? Perhaps
>> they should also be added as regression tests.
>> (Just from eyeballing the code, I think your patch *does* fix them,
>> including my example where the T is dependent on a class template instead
>> of on a function template; but it might be nice to test explicitly.)
>>
>> LGTM.
>>
>> -Arthur
>>
>> On Thu, Jan 30, 2014 at 6:55 AM, Rahul Jain <rahul1.jain at samsung.com>wrote:
>>
>>
>>
>> Hi,
>>
>> This patch fixes PR14995 - Too eagerly rejects operator function
>> templates.
>>
>> Please help review the same.
>>
>>
>>
>> Index: lib/Sema/SemaDeclCXX.cpp
>> ===================================================================
>> --- lib/Sema/SemaDeclCXX.cpp (revision 200465)
>> +++ lib/Sema/SemaDeclCXX.cpp (working copy)
>> @@ -10913,7 +10913,11 @@
>> if (const BuiltinType *BT =
>> LastParam->getType()->getAs<BuiltinType>())
>> ParamIsInt = BT->getKind() == BuiltinType::Int;
>>
>> - if (!ParamIsInt)
>> + bool ParamIsDependent = false;
>> + if (LastParam->getType()->isDependentType())
>> + ParamIsDependent = true;
>> +
>> + if (!ParamIsInt && !ParamIsDependent)
>> return Diag(LastParam->getLocation(),
>> diag::err_operator_overload_post_incdec_must_be_int)
>> << LastParam->getType() << (Op == OO_MinusMinus);
>> Index: test/SemaCXX/overloaded-operator.cpp
>> ===================================================================
>> --- test/SemaCXX/overloaded-operator.cpp (revision 200465)
>> +++ test/SemaCXX/overloaded-operator.cpp (working copy)
>> @@ -452,3 +452,16 @@
>> Result = 1; // expected-error {{no viable overloaded '='}} //
>> expected-note {{type 'PointerUnion<int *, float *>' is incomplete}}
>> }
>> }
>> +
>> +namespace PR14995 {
>> +
>> + struct B {};
>> + template<typename ...T> void operator++(B, T...) {}
>> +
>> + void f() {
>> + B b;
>> + b++; // ok
>> + ++b; // ok
>> + }
>> +}
>> +
>>
>>
>>
>>
>>
>> Thanks,
>>
>> Rahul
>>
>>
>>
>> <201401302024474_BGFC2LL5.gif>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140131/abbf20c0/attachment.html>
More information about the cfe-commits
mailing list