[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa8c8b627f23f: [ObjC generics] Fix not inheriting type bounds 
in categories/extensions. (authored by vsapsai).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872

Files:
  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

Index: clang/test/SemaObjC/parameterized_classes_subst.m
===
--- clang/test/SemaObjC/parameterized_classes_subst.m
+++ clang/test/SemaObjC/parameterized_classes_subst.m
@@ -467,3 +467,17 @@
 - (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>
+- (ParameterizedContainer *)inInterface;
+@end
+@interface ParameterizedContainer (Cat)
+- (ParameterizedContainer *)inCategory;
+@end
+@interface ParameterizedContainer ()
+- (ParameterizedContainer *)inExtension;
+@end
Index: clang/test/SemaObjC/parameterized_classes_collection_literal.m
===
--- clang/test/SemaObjC/parameterized_classes_collection_literal.m
+++ clang/test/SemaObjC/parameterized_classes_collection_literal.m
@@ -29,7 +29,9 @@
 @end
 
 @interface NSDictionary : NSObject 
-+ (instancetype)dictionaryWithObjects:(const V [])objects forKeys:(const K [])keys count:(NSUInteger)cnt;
++ (instancetype)dictionaryWithObjects:(const V [])objects
+  forKeys:(const K  [])keys
+count:(NSUInteger)cnt;
 @end
 
 void testArrayLiteral(void) {
@@ -50,3 +52,9 @@
 @"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",
+  };
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -938,8 +938,7 @@
 
   // 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 @@
 }
 
 // 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;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3534,6 +3534,7 @@
 const ObjCTypeParamDecl *OTPDecl,
 ArrayRef protocols) {
   ID.AddPointer(OTPDecl);
+  ID.AddPointer(OTPDecl->getUnderlyingType().getAsOpaquePtr());
   ID.AddInteger(protocols.size());
   for (auto proto : protocols)
 ID.AddPointer(proto);
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -4874,6 +4874,17 @@
   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(New->getTypeForDecl());
+  SmallVector 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.
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -1442,6 +1442,8 @@
 
   QualType getObjCTypeParamType(const ObjCTypeParamDecl 

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the review.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington accepted this revision.
erik.pilkington added a comment.
This revision is now accepted and ready to land.

LGTM, sorry for the delay in reviewing this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 246005.
vsapsai added a comment.

- Fix some clang-format checks. Ignore the check to add a space between a 
generic name and its type parameters as it's not the style the rest of the 
header uses and not what Apple SDK headers are using.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872

Files:
  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

Index: clang/test/SemaObjC/parameterized_classes_subst.m
===
--- clang/test/SemaObjC/parameterized_classes_subst.m
+++ clang/test/SemaObjC/parameterized_classes_subst.m
@@ -467,3 +467,17 @@
 - (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>
+- (ParameterizedContainer *)inInterface;
+@end
+@interface ParameterizedContainer (Cat)
+- (ParameterizedContainer *)inCategory;
+@end
+@interface ParameterizedContainer ()
+- (ParameterizedContainer *)inExtension;
+@end
Index: clang/test/SemaObjC/parameterized_classes_collection_literal.m
===
--- clang/test/SemaObjC/parameterized_classes_collection_literal.m
+++ clang/test/SemaObjC/parameterized_classes_collection_literal.m
@@ -29,7 +29,9 @@
 @end
 
 @interface NSDictionary : NSObject 
-+ (instancetype)dictionaryWithObjects:(const V [])objects forKeys:(const K [])keys count:(NSUInteger)cnt;
++ (instancetype)dictionaryWithObjects:(const V [])objects
+  forKeys:(const K  [])keys
+count:(NSUInteger)cnt;
 @end
 
 void testArrayLiteral(void) {
@@ -50,3 +52,9 @@
 @"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",
+  };
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -937,8 +937,7 @@
 
   // 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;
 }
 
@@ -965,8 +964,7 @@
 }
 
 // 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;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3537,6 +3537,7 @@
 const ObjCTypeParamDecl *OTPDecl,
 ArrayRef protocols) {
   ID.AddPointer(OTPDecl);
+  ID.AddPointer(OTPDecl->getUnderlyingType().getAsOpaquePtr());
   ID.AddInteger(protocols.size());
   for (auto proto : protocols)
 ID.AddPointer(proto);
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -4808,6 +4808,17 @@
   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(New->getTypeForDecl());
+  SmallVector 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.
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -1448,6 

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D72872#1872242 , @hans wrote:

> [...snip...]
>  Thanks! I applied the patch and was able to build large parts of Chromium 
> without any problems.


Thanks for checking!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D72872#1870256 , @vsapsai wrote:

> In D72872#1868989 , @hans wrote:
>
> > I don't have the context here. Was I added as a subscriber because it's 
> > related to the clang 10 release?
>
>
> It's not related to clang 10 release. I've added you because earlier you've 
> found a problem with a previous approach 
> https://github.com/llvm/llvm-project/commit/4c539e8da1b3de38a53ef3f7497f5c45a3243b61
>  So in case it breaks something else, you have extra visibility into the 
> change.


Thanks! I applied the patch and was able to build large parts of Chromium 
without any problems.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D72872#1868989 , @hans wrote:

> I don't have the context here. Was I added as a subscriber because it's 
> related to the clang 10 release?


It's not related to clang 10 release. I've added you because earlier you've 
found a problem with a previous approach 
https://github.com/llvm/llvm-project/commit/4c539e8da1b3de38a53ef3f7497f5c45a3243b61
 So in case it breaks something else, you have extra visibility into the change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I don't have the context here. Was I added as a subscriber because it's related 
to the clang 10 release?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61930 tests passed, 0 failed 
and 783 were skipped.

{icon question-circle color=gray} clang-tidy: unknown.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72872/new/

https://reviews.llvm.org/D72872



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72872: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: erik.pilkington, ahatanak.
Herald added subscribers: ributzka, dexonsmith, jkorous.
Herald added a project: clang.

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') 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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72872

Files:
  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

Index: clang/test/SemaObjC/parameterized_classes_subst.m
===
--- clang/test/SemaObjC/parameterized_classes_subst.m
+++ clang/test/SemaObjC/parameterized_classes_subst.m
@@ -467,3 +467,17 @@
 - (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>
+- (ParameterizedContainer *)inInterface;
+@end
+@interface ParameterizedContainer (Cat)
+- (ParameterizedContainer *)inCategory;
+@end
+@interface ParameterizedContainer ()
+- (ParameterizedContainer *)inExtension;
+@end
Index: clang/test/SemaObjC/parameterized_classes_collection_literal.m
===
--- clang/test/SemaObjC/parameterized_classes_collection_literal.m
+++ clang/test/SemaObjC/parameterized_classes_collection_literal.m
@@ -29,7 +29,7 @@
 @end
 
 @interface NSDictionary : NSObject 
-+ (instancetype)dictionaryWithObjects:(const V [])objects forKeys:(const K [])keys count:(NSUInteger)cnt;
++ (instancetype)dictionaryWithObjects:(const V [])objects forKeys:(const K  [])keys count:(NSUInteger)cnt;
 @end
 
 void testArrayLiteral(void) {
@@ -50,3 +50,9 @@
 @"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",
+  };
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -937,8 +937,7 @@
 
   // 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;
 }
 
@@ -965,8 +964,7 @@
 }
 
 // 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;
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -3535,6 +3535,7 @@
 const ObjCTypeParamDecl *OTPDecl,
 ArrayRef protocols) {
   ID.AddPointer(OTPDecl);
+  ID.AddPointer(OTPDecl->getUnderlyingType().getAsOpaquePtr());
   ID.AddInteger(protocols.size());
   for (auto proto : protocols)