[clang] a8c8b62 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.
Volodymyr Sapsai via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 6 13:40:26 PDT 2020
Is there anything special in the builedbot configuration? Are you still observing intermittent failures?
I’m double checking if we see similar failures internally but so far looks like everything is working fine. I’ve only seen a single failure http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/18616 <http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-ubsan/builds/18616>
At the moment I don’t know what might be causing the non-determinism. The type checking depends on the parsing order. We rely that
@interface ParameterizedContainer<T> (Cat)
is parsed before
- (ParameterizedContainer<T> *)inCategory;
So we check type parameter T is consistent with original @interface before using it. What is also confusing is that there is only a single error. I expect both a category and an extension to have same errors.
> On Apr 5, 2020, at 10:10, Nico Weber <thakis at chromium.org> wrote:
>
> The test here flakily fails, maybe 1 in 10 times: http://45.33.8.238/mac/11180/step_7.txt <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 <mailto: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 <https://github.com/llvm/llvm-project/commit/a8c8b627f23f204fb621bd2a8c495cfc8bc16ae7>
> DIFF: 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 <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 <mailto:cfe-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <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/20200406/e4dda408/attachment-0001.html>
More information about the cfe-commits
mailing list