Re: r374202 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.
I've gone ahead and reverted in 4c539e8. On Tue, Oct 22, 2019 at 10:43 AM Hans Wennborg wrote: > > Since it looks suspicious, can we revert your change in the meantime? > We have a number of instances where this fires, so it's not just one > place we'd need to work around it. > > On Mon, Oct 21, 2019 at 6:39 PM Volodymyr Sapsai wrote: > > > > That error looks strange and confusing to me too. My guess is that my > > change exposed some bug with literals. I’ve tried > > > > NSDictionary* videoSettingsDictionary2 = [NSDictionary > > dictionaryWithObject:@(best_fourcc) > > forKey:(id)kCVPixelBufferPixelFormatTypeKey]; > > > > and > > > > @interface GenericTest : NSObject > > - (void)test:(const KeyType _Nonnull)key; > > @end > > > > void anotherTest(GenericTest *t) { > > [t test:(id)kCVPixelBufferPixelFormatTypeKey]; > > } > > > > But they didn’t trigger the error. I need more time to investigate the > > issue but in the meantime you can fix the build by casting to the > > corresponding constant type, something like > > > > (NSString *)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc) > > > > Thanks, > > Volodymyr > > > > > On Oct 21, 2019, at 14:37, Hans Wennborg wrote: > > > > > > Hi Volodymyr, > > > > > > This broke the Chrome build in an interesting way. Here's a reduced repro: > > > > > > > > > $ cat /tmp/a.mm > > > #import > > > > > > void f(int width, int height) { > > > FourCharCode best_fourcc = kCMPixelFormat_422YpCbCr8_yuvs; > > > NSDictionary* videoSettingsDictionary = @{ > > >(id)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc), > > > }; > > > } > > > > > > $ build.release/bin/clang++ -isysroot > > > /work/chromium/src/build/mac_files/xcode_binaries/Contents/Developer/Platforms/MacOSX.platform/Developer/SDK > > > s/MacOSX10.14.sdk -c /tmp/a.mm > > > > > > /tmp/a.mm:6:5: error: cannot initialize a parameter of type > > > 'KeyType _Nonnull const' (aka 'const id') with an rvalue > > > of type 'id' > > >(id)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc), > > >^~~~ > > > 1 error generated. > > > > > > > > > To me, the "cannot initialize a parameter of type [nanana] (aka 'const > > > id') with an rvalue of type 'id'" message looks strange, but I'm not > > > an Obj-C expert. Is this expected behaviour, and if so how should we > > > change our code? > > > > > > Thanks, > > > Hans > > > > > > On Wed, Oct 9, 2019 at 12:26 PM Volodymyr Sapsai via cfe-commits > > > wrote: > > >> > > >> Author: vsapsai > > >> Date: Wed Oct 9 12:29:13 2019 > > >> New Revision: 374202 > > >> > > >> URL: http://llvm.org/viewvc/llvm-project?rev=374202&view=rev > > >> Log: > > >> [ObjC generics] Fix not inheriting type bounds in categories/extensions. > > >> > > >> When a category/extension doesn't repeat a type bound, corresponding > > >> type parameter is substituted with `id` when used as a type argument. As > > >> a result, in the added test case it was causing errors like > > >> > > >>> type argument 'T' (aka 'id') does not satisfy the bound > > >>> ('id') of type parameter 'T' > > >> > > >> We are already checking that type parameters should be consistent > > >> everywhere (see `checkTypeParamListConsistency`) and update > > >> `ObjCTypeParamDecl` to have correct underlying type. And when we use the > > >> type parameter as a method return type or a method parameter type, it is > > >> substituted to the bounded type. But when we use the type parameter as a > > >> type argument, we check `ObjCTypeParamType` that ignores the updated > > >> underlying type and remains `id`. > > >> > > >> Fix by desugaring `ObjCTypeParamType` to the underlying type, the same > > >> way we are doing with `TypedefType`. > > >> > > >> rdar://problem/54329242 > > >> > > >> Reviewers: erik.pilkington, ahatanak > > >> > > >> Reviewed By: erik.pilkington > > >> > > >> Subscribers: jkorous, dexonsmith, ributzka, cfe-commits > > >> > > >> Differential Revision: https://reviews.llvm.org/D66696 > > >> > > >> Modified: > > >>cfe/trunk/include/clang/AST/Type.h > > >>cfe/trunk/lib/AST/Type.cpp > > >>cfe/trunk/test/SemaObjC/parameterized_classes_subst.m > > >> > > >> Modified: cfe/trunk/include/clang/AST/Type.h > > >> URL: > > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=374202&r1=374201&r2=374202&view=diff > > >> == > > >> --- cfe/trunk/include/clang/AST/Type.h (original) > > >> +++ cfe/trunk/include/clang/AST/Type.h Wed Oct 9 12:29:13 2019 > > >> @@ -5569,7 +5569,7 @@ class ObjCTypeParamType : public Type, > > >> > > >> public: > > >> bool isSugared() const { return true; } > > >> - QualType desugar() const { return getCanonicalTypeInternal(); } > > >> + QualType desugar() const; > > >> > > >> static bool classof(const Type *T) { > > >> return T->getTypeClass() == ObjCTypeParam; > > >> > > >> Modified: cfe/trunk/lib/AST/Type.cpp > > >> URL: > > >> htt
Re: r374202 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.
Since it looks suspicious, can we revert your change in the meantime? We have a number of instances where this fires, so it's not just one place we'd need to work around it. On Mon, Oct 21, 2019 at 6:39 PM Volodymyr Sapsai wrote: > > That error looks strange and confusing to me too. My guess is that my change > exposed some bug with literals. I’ve tried > > NSDictionary* videoSettingsDictionary2 = [NSDictionary > dictionaryWithObject:@(best_fourcc) > forKey:(id)kCVPixelBufferPixelFormatTypeKey]; > > and > > @interface GenericTest : NSObject > - (void)test:(const KeyType _Nonnull)key; > @end > > void anotherTest(GenericTest *t) { > [t test:(id)kCVPixelBufferPixelFormatTypeKey]; > } > > But they didn’t trigger the error. I need more time to investigate the issue > but in the meantime you can fix the build by casting to the corresponding > constant type, something like > > (NSString *)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc) > > Thanks, > Volodymyr > > > On Oct 21, 2019, at 14:37, Hans Wennborg wrote: > > > > Hi Volodymyr, > > > > This broke the Chrome build in an interesting way. Here's a reduced repro: > > > > > > $ cat /tmp/a.mm > > #import > > > > void f(int width, int height) { > > FourCharCode best_fourcc = kCMPixelFormat_422YpCbCr8_yuvs; > > NSDictionary* videoSettingsDictionary = @{ > >(id)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc), > > }; > > } > > > > $ build.release/bin/clang++ -isysroot > > /work/chromium/src/build/mac_files/xcode_binaries/Contents/Developer/Platforms/MacOSX.platform/Developer/SDK > > s/MacOSX10.14.sdk -c /tmp/a.mm > > > > /tmp/a.mm:6:5: error: cannot initialize a parameter of type > > 'KeyType _Nonnull const' (aka 'const id') with an rvalue > > of type 'id' > >(id)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc), > >^~~~ > > 1 error generated. > > > > > > To me, the "cannot initialize a parameter of type [nanana] (aka 'const > > id') with an rvalue of type 'id'" message looks strange, but I'm not > > an Obj-C expert. Is this expected behaviour, and if so how should we > > change our code? > > > > Thanks, > > Hans > > > > On Wed, Oct 9, 2019 at 12:26 PM Volodymyr Sapsai via cfe-commits > > wrote: > >> > >> Author: vsapsai > >> Date: Wed Oct 9 12:29:13 2019 > >> New Revision: 374202 > >> > >> URL: http://llvm.org/viewvc/llvm-project?rev=374202&view=rev > >> Log: > >> [ObjC generics] Fix not inheriting type bounds in categories/extensions. > >> > >> When a category/extension doesn't repeat a type bound, corresponding > >> type parameter is substituted with `id` when used as a type argument. As > >> a result, in the added test case it was causing errors like > >> > >>> type argument 'T' (aka 'id') does not satisfy the bound ('id') > >>> of type parameter 'T' > >> > >> We are already checking that type parameters should be consistent > >> everywhere (see `checkTypeParamListConsistency`) and update > >> `ObjCTypeParamDecl` to have correct underlying type. And when we use the > >> type parameter as a method return type or a method parameter type, it is > >> substituted to the bounded type. But when we use the type parameter as a > >> type argument, we check `ObjCTypeParamType` that ignores the updated > >> underlying type and remains `id`. > >> > >> Fix by desugaring `ObjCTypeParamType` to the underlying type, the same > >> way we are doing with `TypedefType`. > >> > >> rdar://problem/54329242 > >> > >> Reviewers: erik.pilkington, ahatanak > >> > >> Reviewed By: erik.pilkington > >> > >> Subscribers: jkorous, dexonsmith, ributzka, cfe-commits > >> > >> Differential Revision: https://reviews.llvm.org/D66696 > >> > >> Modified: > >>cfe/trunk/include/clang/AST/Type.h > >>cfe/trunk/lib/AST/Type.cpp > >>cfe/trunk/test/SemaObjC/parameterized_classes_subst.m > >> > >> Modified: cfe/trunk/include/clang/AST/Type.h > >> URL: > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=374202&r1=374201&r2=374202&view=diff > >> == > >> --- cfe/trunk/include/clang/AST/Type.h (original) > >> +++ cfe/trunk/include/clang/AST/Type.h Wed Oct 9 12:29:13 2019 > >> @@ -5569,7 +5569,7 @@ class ObjCTypeParamType : public Type, > >> > >> public: > >> bool isSugared() const { return true; } > >> - QualType desugar() const { return getCanonicalTypeInternal(); } > >> + QualType desugar() const; > >> > >> static bool classof(const Type *T) { > >> return T->getTypeClass() == ObjCTypeParam; > >> > >> Modified: cfe/trunk/lib/AST/Type.cpp > >> URL: > >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=374202&r1=374201&r2=374202&view=diff > >> == > >> --- cfe/trunk/lib/AST/Type.cpp (original) > >> +++ cfe/trunk/lib/AST/Type.cpp Wed Oct 9 12:29:13 2019 > >> @@ -663,6 +663,10 @@ ObjCTypeParamType::ObjCTypeParamType(con >
Re: r374202 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.
That error looks strange and confusing to me too. My guess is that my change exposed some bug with literals. I’ve tried NSDictionary* videoSettingsDictionary2 = [NSDictionary dictionaryWithObject:@(best_fourcc) forKey:(id)kCVPixelBufferPixelFormatTypeKey]; and @interface GenericTest : NSObject - (void)test:(const KeyType _Nonnull)key; @end void anotherTest(GenericTest *t) { [t test:(id)kCVPixelBufferPixelFormatTypeKey]; } But they didn’t trigger the error. I need more time to investigate the issue but in the meantime you can fix the build by casting to the corresponding constant type, something like (NSString *)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc) Thanks, Volodymyr > On Oct 21, 2019, at 14:37, Hans Wennborg wrote: > > Hi Volodymyr, > > This broke the Chrome build in an interesting way. Here's a reduced repro: > > > $ cat /tmp/a.mm > #import > > void f(int width, int height) { > FourCharCode best_fourcc = kCMPixelFormat_422YpCbCr8_yuvs; > NSDictionary* videoSettingsDictionary = @{ >(id)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc), > }; > } > > $ build.release/bin/clang++ -isysroot > /work/chromium/src/build/mac_files/xcode_binaries/Contents/Developer/Platforms/MacOSX.platform/Developer/SDK > s/MacOSX10.14.sdk -c /tmp/a.mm > > /tmp/a.mm:6:5: error: cannot initialize a parameter of type > 'KeyType _Nonnull const' (aka 'const id') with an rvalue > of type 'id' >(id)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc), >^~~~ > 1 error generated. > > > To me, the "cannot initialize a parameter of type [nanana] (aka 'const > id') with an rvalue of type 'id'" message looks strange, but I'm not > an Obj-C expert. Is this expected behaviour, and if so how should we > change our code? > > Thanks, > Hans > > On Wed, Oct 9, 2019 at 12:26 PM Volodymyr Sapsai via cfe-commits > wrote: >> >> Author: vsapsai >> Date: Wed Oct 9 12:29:13 2019 >> New Revision: 374202 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=374202&view=rev >> Log: >> [ObjC generics] Fix not inheriting type bounds in categories/extensions. >> >> When a category/extension doesn't repeat a type bound, corresponding >> type parameter is substituted with `id` when used as a type argument. As >> a result, in the added test case it was causing errors like >> >>> type argument 'T' (aka 'id') does not satisfy the bound ('id') >>> of type parameter 'T' >> >> We are already checking that type parameters should be consistent >> everywhere (see `checkTypeParamListConsistency`) and update >> `ObjCTypeParamDecl` to have correct underlying type. And when we use the >> type parameter as a method return type or a method parameter type, it is >> substituted to the bounded type. But when we use the type parameter as a >> type argument, we check `ObjCTypeParamType` that ignores the updated >> underlying type and remains `id`. >> >> Fix by desugaring `ObjCTypeParamType` to the underlying type, the same >> way we are doing with `TypedefType`. >> >> rdar://problem/54329242 >> >> Reviewers: erik.pilkington, ahatanak >> >> Reviewed By: erik.pilkington >> >> Subscribers: jkorous, dexonsmith, ributzka, cfe-commits >> >> Differential Revision: https://reviews.llvm.org/D66696 >> >> Modified: >>cfe/trunk/include/clang/AST/Type.h >>cfe/trunk/lib/AST/Type.cpp >>cfe/trunk/test/SemaObjC/parameterized_classes_subst.m >> >> Modified: cfe/trunk/include/clang/AST/Type.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=374202&r1=374201&r2=374202&view=diff >> == >> --- cfe/trunk/include/clang/AST/Type.h (original) >> +++ cfe/trunk/include/clang/AST/Type.h Wed Oct 9 12:29:13 2019 >> @@ -5569,7 +5569,7 @@ class ObjCTypeParamType : public Type, >> >> public: >> bool isSugared() const { return true; } >> - QualType desugar() const { return getCanonicalTypeInternal(); } >> + QualType desugar() const; >> >> static bool classof(const Type *T) { >> return T->getTypeClass() == ObjCTypeParam; >> >> Modified: cfe/trunk/lib/AST/Type.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=374202&r1=374201&r2=374202&view=diff >> == >> --- cfe/trunk/lib/AST/Type.cpp (original) >> +++ cfe/trunk/lib/AST/Type.cpp Wed Oct 9 12:29:13 2019 >> @@ -663,6 +663,10 @@ ObjCTypeParamType::ObjCTypeParamType(con >> initialize(protocols); >> } >> >> +QualType ObjCTypeParamType::desugar() const { >> + return getDecl()->getUnderlyingType(); >> +} >> + >> ObjCObjectType::ObjCObjectType(QualType Canonical, QualType Base, >>ArrayRef typeArgs, >>ArrayRef protocols, >> >> Modified: cfe/trunk/test/SemaObjC/parameterized_classes_subst.m >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC
Re: r374202 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.
Hi Volodymyr, This broke the Chrome build in an interesting way. Here's a reduced repro: $ cat /tmp/a.mm #import void f(int width, int height) { FourCharCode best_fourcc = kCMPixelFormat_422YpCbCr8_yuvs; NSDictionary* videoSettingsDictionary = @{ (id)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc), }; } $ build.release/bin/clang++ -isysroot /work/chromium/src/build/mac_files/xcode_binaries/Contents/Developer/Platforms/MacOSX.platform/Developer/SDK s/MacOSX10.14.sdk -c /tmp/a.mm /tmp/a.mm:6:5: error: cannot initialize a parameter of type 'KeyType _Nonnull const' (aka 'const id') with an rvalue of type 'id' (id)kCVPixelBufferPixelFormatTypeKey : @(best_fourcc), ^~~~ 1 error generated. To me, the "cannot initialize a parameter of type [nanana] (aka 'const id') with an rvalue of type 'id'" message looks strange, but I'm not an Obj-C expert. Is this expected behaviour, and if so how should we change our code? Thanks, Hans On Wed, Oct 9, 2019 at 12:26 PM Volodymyr Sapsai via cfe-commits wrote: > > Author: vsapsai > Date: Wed Oct 9 12:29:13 2019 > New Revision: 374202 > > URL: http://llvm.org/viewvc/llvm-project?rev=374202&view=rev > Log: > [ObjC generics] Fix not inheriting type bounds in categories/extensions. > > When a category/extension doesn't repeat a type bound, corresponding > type parameter is substituted with `id` when used as a type argument. As > a result, in the added test case it was causing errors like > > > type argument 'T' (aka 'id') does not satisfy the bound ('id') > > of type parameter 'T' > > We are already checking that type parameters should be consistent > everywhere (see `checkTypeParamListConsistency`) and update > `ObjCTypeParamDecl` to have correct underlying type. And when we use the > type parameter as a method return type or a method parameter type, it is > substituted to the bounded type. But when we use the type parameter as a > type argument, we check `ObjCTypeParamType` that ignores the updated > underlying type and remains `id`. > > Fix by desugaring `ObjCTypeParamType` to the underlying type, the same > way we are doing with `TypedefType`. > > rdar://problem/54329242 > > Reviewers: erik.pilkington, ahatanak > > Reviewed By: erik.pilkington > > Subscribers: jkorous, dexonsmith, ributzka, cfe-commits > > Differential Revision: https://reviews.llvm.org/D66696 > > Modified: > cfe/trunk/include/clang/AST/Type.h > cfe/trunk/lib/AST/Type.cpp > cfe/trunk/test/SemaObjC/parameterized_classes_subst.m > > Modified: cfe/trunk/include/clang/AST/Type.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=374202&r1=374201&r2=374202&view=diff > == > --- cfe/trunk/include/clang/AST/Type.h (original) > +++ cfe/trunk/include/clang/AST/Type.h Wed Oct 9 12:29:13 2019 > @@ -5569,7 +5569,7 @@ class ObjCTypeParamType : public Type, > > public: >bool isSugared() const { return true; } > - QualType desugar() const { return getCanonicalTypeInternal(); } > + QualType desugar() const; > >static bool classof(const Type *T) { > return T->getTypeClass() == ObjCTypeParam; > > Modified: cfe/trunk/lib/AST/Type.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=374202&r1=374201&r2=374202&view=diff > == > --- cfe/trunk/lib/AST/Type.cpp (original) > +++ cfe/trunk/lib/AST/Type.cpp Wed Oct 9 12:29:13 2019 > @@ -663,6 +663,10 @@ ObjCTypeParamType::ObjCTypeParamType(con >initialize(protocols); > } > > +QualType ObjCTypeParamType::desugar() const { > + return getDecl()->getUnderlyingType(); > +} > + > ObjCObjectType::ObjCObjectType(QualType Canonical, QualType Base, > ArrayRef typeArgs, > ArrayRef protocols, > > Modified: cfe/trunk/test/SemaObjC/parameterized_classes_subst.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/parameterized_classes_subst.m?rev=374202&r1=374201&r2=374202&view=diff > == > --- cfe/trunk/test/SemaObjC/parameterized_classes_subst.m (original) > +++ cfe/trunk/test/SemaObjC/parameterized_classes_subst.m Wed Oct 9 12:29:13 > 2019 > @@ -467,3 +467,17 @@ void bar(MyMutableDictionary - (void)mapUsingBlock2:(id)block { // expected-warning{{conflicting > parameter types in implementation}} > } > @end > + > +// -- > +// Use a type parameter as a type argument. > +// -- > +// Type bounds in a category/extension are omitted. rdar://problem/54329242 > +@interface ParameterizedContainer> > +- (ParameterizedContainer *)inInterface; > +@end > +@interface ParameterizedContainer (Cat) > +- (Pa