r374202 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 22 14:16:33 PDT 2019
I've gone ahead and reverted in 4c539e8.
On Tue, Oct 22, 2019 at 10:43 AM Hans Wennborg <hans at chromium.org> 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 <vsapsai at apple.com> 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 <KeyType> : NSObject
> > - (void)test:(const KeyType <NSCopying> _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 <hans at chromium.org> wrote:
> > >
> > > Hi Volodymyr,
> > >
> > > This broke the Chrome build in an interesting way. Here's a reduced repro:
> > >
> > >
> > > $ cat /tmp/a.mm
> > > #import <AVFoundation/AVFoundation.h>
> > >
> > > 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<NSCopying> _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
> > > <cfe-commits at lists.llvm.org> 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<NSCopying>') 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<QualType> typeArgs,
> > >> ArrayRef<ObjCProtocolDecl *> 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<NSString *,
> > >> - (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
> > >> + at interface ParameterizedContainer<T : id<NSCopying>>
> > >> +- (ParameterizedContainer<T> *)inInterface;
> > >> + at end
> > >> + at interface ParameterizedContainer<T> (Cat)
> > >> +- (ParameterizedContainer<T> *)inCategory;
> > >> + at end
> > >> + at interface ParameterizedContainer<U> ()
> > >> +- (ParameterizedContainer<U> *)inExtension;
> > >> + at end
> > >>
> > >>
> > >> _______________________________________________
> > >> cfe-commits mailing list
> > >> cfe-commits at lists.llvm.org
> > >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
More information about the cfe-commits
mailing list