[Patch] PR14995 Too eagerly rejects operator function templates
Richard Smith
metafoo at gmail.com
Thu Jan 30 13:56:30 PST 2014
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/20140130/17f48696/attachment.html>
More information about the cfe-commits
mailing list