[clang] a8c8b62 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 5 10:10:12 PDT 2020


The test here flakily fails, maybe 1 in 10 times:
http://45.33.8.238/mac/11180/step_7.txt

error: 'error' diagnostics seen but not expected:
  File
/Users/thakis/src/llvm-project/clang/test/SemaObjC/parameterized_classes_subst.m
Line 479: type argument 'T' (aka 'id') does not satisfy the bound
('id<NSCopying>') of type parameter 'T'
error: 'note' diagnostics seen but not expected:
  File
/Users/thakis/src/llvm-project/clang/test/SemaObjC/parameterized_classes_subst.m
Line 475: type parameter 'T' declared here
2 errors generated.

Maybe if this is emitted depends on the order of something in a data
structure that has no deterministic order, or similar?

On Fri, Apr 3, 2020 at 7:29 PM Volodymyr Sapsai via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

>
> Author: Volodymyr Sapsai
> Date: 2020-04-03T16:29:02-07:00
> New Revision: a8c8b627f23f204fb621bd2a8c495cfc8bc16ae7
>
> URL:
> https://github.com/llvm/llvm-project/commit/a8c8b627f23f204fb621bd2a8c495cfc8bc16ae7
> DIFF:
> https://github.com/llvm/llvm-project/commit/a8c8b627f23f204fb621bd2a8c495cfc8bc16ae7.diff
>
> 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 wasn't updated and
> remains `id`.
>
> Fix by updating not only `ObjCTypeParamDecl` UnderlyingType but also
> TypeForDecl as we use the underlying type to create a canonical type for
> `ObjCTypeParamType` (see `ASTContext::getObjCTypeParamType`).
>
> This is a different approach to fixing the issue. The previous one was
> 02c2ab3d8872416589bd1a6ca3dfb96ba373a3b9 which was reverted in
> 4c539e8da1b3de38a53ef3f7497f5c45a3243b61. The problem with the previous
> approach was that `ObjCTypeParamType::desugar` was returning underlying
> type for `ObjCTypeParamDecl` without applying any protocols stored in
> `ObjCTypeParamType`. It caused inconsistencies in comparing types before
> and after desugaring.
>
> rdar://problem/54329242
>
> Reviewed By: erik.pilkington
>
> Differential Revision: https://reviews.llvm.org/D72872
>
> Added:
>
>
> Modified:
>     clang/include/clang/AST/ASTContext.h
>     clang/lib/AST/ASTContext.cpp
>     clang/lib/AST/Type.cpp
>     clang/lib/Sema/SemaDeclObjC.cpp
>     clang/test/SemaObjC/parameterized_classes_collection_literal.m
>     clang/test/SemaObjC/parameterized_classes_subst.m
>
> Removed:
>
>
>
>
> ################################################################################
> diff  --git a/clang/include/clang/AST/ASTContext.h
> b/clang/include/clang/AST/ASTContext.h
> index 6813ab58874e..6360f18217c7 100644
> --- a/clang/include/clang/AST/ASTContext.h
> +++ b/clang/include/clang/AST/ASTContext.h
> @@ -1442,6 +1442,8 @@ class ASTContext : public RefCountedBase<ASTContext>
> {
>
>    QualType getObjCTypeParamType(const ObjCTypeParamDecl *Decl,
>                                  ArrayRef<ObjCProtocolDecl *> protocols)
> const;
> +  void adjustObjCTypeParamBoundType(const ObjCTypeParamDecl *Orig,
> +                                    ObjCTypeParamDecl *New) const;
>
>    bool ObjCObjectAdoptsQTypeProtocols(QualType QT, ObjCInterfaceDecl
> *Decl);
>
>
> diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
> index 1e81e0a67b4d..06dcb6fa0580 100644
> --- a/clang/lib/AST/ASTContext.cpp
> +++ b/clang/lib/AST/ASTContext.cpp
> @@ -4874,6 +4874,17 @@ ASTContext::getObjCTypeParamType(const
> ObjCTypeParamDecl *Decl,
>    return QualType(newType, 0);
>  }
>
> +void ASTContext::adjustObjCTypeParamBoundType(const ObjCTypeParamDecl
> *Orig,
> +                                              ObjCTypeParamDecl *New)
> const {
> +
> New->setTypeSourceInfo(getTrivialTypeSourceInfo(Orig->getUnderlyingType()));
> +  // Update TypeForDecl after updating TypeSourceInfo.
> +  auto NewTypeParamTy = cast<ObjCTypeParamType>(New->getTypeForDecl());
> +  SmallVector<ObjCProtocolDecl *, 8> protocols;
> +  protocols.append(NewTypeParamTy->qual_begin(),
> NewTypeParamTy->qual_end());
> +  QualType UpdatedTy = getObjCTypeParamType(New, protocols);
> +  New->setTypeForDecl(UpdatedTy.getTypePtr());
> +}
> +
>  /// ObjCObjectAdoptsQTypeProtocols - Checks that protocols in IC's
>  /// protocol list adopt all protocols in QT's qualified-id protocol
>  /// list.
>
> diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
> index 3428437c3146..7c65378261ad 100644
> --- a/clang/lib/AST/Type.cpp
> +++ b/clang/lib/AST/Type.cpp
> @@ -3534,6 +3534,7 @@ void
> ObjCTypeParamType::Profile(llvm::FoldingSetNodeID &ID,
>                                  const ObjCTypeParamDecl *OTPDecl,
>                                  ArrayRef<ObjCProtocolDecl *> protocols) {
>    ID.AddPointer(OTPDecl);
> +  ID.AddPointer(OTPDecl->getUnderlyingType().getAsOpaquePtr());
>    ID.AddInteger(protocols.size());
>    for (auto proto : protocols)
>      ID.AddPointer(proto);
>
> diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp
> b/clang/lib/Sema/SemaDeclObjC.cpp
> index 934e1a23141c..6db57898e378 100644
> --- a/clang/lib/Sema/SemaDeclObjC.cpp
> +++ b/clang/lib/Sema/SemaDeclObjC.cpp
> @@ -938,8 +938,7 @@ static bool checkTypeParamListConsistency(Sema &S,
>
>        // Override the new type parameter's bound type with the previous
> type,
>        // so that it's consistent.
> -      newTypeParam->setTypeSourceInfo(
> -
> S.Context.getTrivialTypeSourceInfo(prevTypeParam->getUnderlyingType()));
> +      S.Context.adjustObjCTypeParamBoundType(prevTypeParam, newTypeParam);
>        continue;
>      }
>
> @@ -966,8 +965,7 @@ static bool checkTypeParamListConsistency(Sema &S,
>      }
>
>      // Update the new type parameter's bound to match the previous one.
> -    newTypeParam->setTypeSourceInfo(
> -
> S.Context.getTrivialTypeSourceInfo(prevTypeParam->getUnderlyingType()));
> +    S.Context.adjustObjCTypeParamBoundType(prevTypeParam, newTypeParam);
>    }
>
>    return false;
>
> diff  --git
> a/clang/test/SemaObjC/parameterized_classes_collection_literal.m
> b/clang/test/SemaObjC/parameterized_classes_collection_literal.m
> index 472746e09db9..034d2e8da217 100644
> --- a/clang/test/SemaObjC/parameterized_classes_collection_literal.m
> +++ b/clang/test/SemaObjC/parameterized_classes_collection_literal.m
> @@ -29,7 +29,9 @@ + (instancetype)arrayWithObjects:(const T [])objects
> count:(NSUInteger)cnt;
>  @end
>
>  @interface NSDictionary<K, V> : NSObject <NSCopying>
> -+ (instancetype)dictionaryWithObjects:(const V [])objects forKeys:(const
> K [])keys count:(NSUInteger)cnt;
> ++ (instancetype)dictionaryWithObjects:(const V [])objects
> +                              forKeys:(const K <NSCopying> [])keys
> +                                count:(NSUInteger)cnt;
>  @end
>
>  void testArrayLiteral(void) {
> @@ -50,3 +52,9 @@ void testDictionaryLiteral(void) {
>      @"world" : @"blah" // expected-warning{{object of type 'NSString *'
> is not compatible with dictionary value type 'NSNumber *'}}
>    };
>  }
> +
> +void testCastingInDictionaryLiteral(NSString *arg) {
> +  NSDictionary *dict = @{
> +    (id)arg : @"foo",
> +  };
> +}
>
> diff  --git a/clang/test/SemaObjC/parameterized_classes_subst.m
> b/clang/test/SemaObjC/parameterized_classes_subst.m
> index d14a6e9deb40..b6d884760d29 100644
> --- a/clang/test/SemaObjC/parameterized_classes_subst.m
> +++ b/clang/test/SemaObjC/parameterized_classes_subst.m
> @@ -467,3 +467,17 @@ - (void)mapUsingBlock:(id (^)(id))block {
>  - (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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200405/f1fc86a4/attachment-0001.html>


More information about the cfe-commits mailing list