Can reproduce test failure locally, reverted in 8fb7cfcea97af440830d256cc18ccd978f218e1d
Thanks for noticing the problem and pointing me to it. > On Apr 6, 2020, at 23:28, Mikael Holmén <mikael.hol...@ericsson.com> wrote: > > We see flakiness in the test in our bots too. Fails one time and then > passes again. > > /Mikael > > On Mon, 2020-04-06 at 21:03 -0400, Nico Weber via cfe-commits wrote: >> 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 <vsap...@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 <tha...@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@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 >>>>> +@interface ParameterizedContainer<T : id<NSCopying>> >>>>> +- (ParameterizedContainer<T> *)inInterface; >>>>> +@end >>>>> +@interface ParameterizedContainer<T> (Cat) >>>>> +- (ParameterizedContainer<T> *)inCategory; >>>>> +@end >>>>> +@interface ParameterizedContainer<U> () >>>>> +- (ParameterizedContainer<U> *)inExtension; >>>>> +@end >>>>> >>>>> >>>>> >>>>> _______________________________________________ >>>>> cfe-commits mailing list >>>>> cfe-commits@lists.llvm.org >>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
