[PATCH] D118402: [C2x][ObjC] Do not crash when trying to encode a _BitInt type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Thanks for the reviews! I've landed in b6d9ca14c20f6f982a9fee4bebccf4761400f6aa 
 (with the 
FIXME removed). Self-accepting on the two LGs so that I can close the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118402

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

[PATCH] D118402: [C2x][ObjC] Do not crash when trying to encode a _BitInt type

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

But overall this seems fine, yeah.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118402

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

[PATCH] D118402: [C2x][ObjC] Do not crash when trying to encode a _BitInt type

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't think you really need the FIXME when we don't have it for all the other 
types that we don't have encodings for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118402

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

[PATCH] D118402: [C2x][ObjC] Do not crash when trying to encode a _BitInt type

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

These seems right to me, but I think @rjmccall knows more about ObjC to make 
sure nothing silly is happening here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118402

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

[PATCH] D118402: [C2x][ObjC] Do not crash when trying to encode a _BitInt type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rjmccall, ahatanak, erichkeane.
aaron.ballman requested review of this revision.
Herald added a project: clang.

Using a `_BitInt` (or `_ExtInt`) type as a block parameter or block return type 
hits an "unreachable" when trying to determine the encoding for the block. 
Instead of crashing, this patch handles it like some of the other types for 
which we don't yet have an encoding. The test case verifies we no longer crash, 
but does not verify that we provide any particular encoding (it can be updated 
once someone more familiar with ObjC steps in to define the encoding).

Fixes PR50503.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118402

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGenObjC/encode-test-bitint.m


Index: clang/test/CodeGenObjC/encode-test-bitint.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/encode-test-bitint.m
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple i686-apple-darwin9 
-fobjc-runtime=macosx-fragile-10.5 -fblocks -emit-llvm -o /dev/null %s
+
+// Using a _BitInt as a block parameter or return type previously would crash
+// when getting the ObjC encoding for the type. Verify that we no longer crash,
+// but do not verify any particular encoding (one has not yet been determined).
+void foo1(void)
+{
+__auto_type blk = ^int(unsigned _BitInt(64) len)
+{
+return 12;
+};
+}
+
+void foo2(void)
+{
+__auto_type blk = ^unsigned _BitInt(64)(int len)
+{
+return 12;
+};
+}
+
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -8286,6 +8286,12 @@
   *NotEncodedT = T;
 return;
 
+  // FIXME: We should do better than this, but this is better than crashing.
+  case Type::BitInt:
+if (NotEncodedT)
+  *NotEncodedT = T;
+return;
+
   // We could see an undeduced auto type here during error recovery.
   // Just ignore it.
   case Type::Auto:
@@ -8293,7 +8299,6 @@
 return;
 
   case Type::Pipe:
-  case Type::BitInt:
 #define ABSTRACT_TYPE(KIND, BASE)
 #define TYPE(KIND, BASE)
 #define DEPENDENT_TYPE(KIND, BASE) \


Index: clang/test/CodeGenObjC/encode-test-bitint.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/encode-test-bitint.m
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fobjc-runtime=macosx-fragile-10.5 -fblocks -emit-llvm -o /dev/null %s
+
+// Using a _BitInt as a block parameter or return type previously would crash
+// when getting the ObjC encoding for the type. Verify that we no longer crash,
+// but do not verify any particular encoding (one has not yet been determined).
+void foo1(void)
+{
+__auto_type blk = ^int(unsigned _BitInt(64) len)
+{
+return 12;
+};
+}
+
+void foo2(void)
+{
+__auto_type blk = ^unsigned _BitInt(64)(int len)
+{
+return 12;
+};
+}
+
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -8286,6 +8286,12 @@
   *NotEncodedT = T;
 return;
 
+  // FIXME: We should do better than this, but this is better than crashing.
+  case Type::BitInt:
+if (NotEncodedT)
+  *NotEncodedT = T;
+return;
+
   // We could see an undeduced auto type here during error recovery.
   // Just ignore it.
   case Type::Auto:
@@ -8293,7 +8299,6 @@
 return;
 
   case Type::Pipe:
-  case Type::BitInt:
 #define ABSTRACT_TYPE(KIND, BASE)
 #define TYPE(KIND, BASE)
 #define DEPENDENT_TYPE(KIND, BASE) \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits