[clang] a8c8b62 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 6 18:03:16 PDT 2020
This isn't bot-dependent, it's been flaking on many different bots over the
last few days. Here's one from just now:
http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/34705
On Mon, Apr 6, 2020 at 4:40 PM Volodymyr Sapsai <vsapsai at apple.com> wrote:
> 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
>
> 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
>
> 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/20200406/2ef6c089/attachment.html>
More information about the cfe-commits
mailing list