[ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}

[erik.pilkington]

Summary

This attribute can be applied to typedefs of BOOL in Objective-C. It causes loads, stores, and casts to BOOL to clamp into {0,1}. This is useful for us because a lot of code is going to be moving from a platform where BOOL is a typedef for a native bool type (iOS) to a platform where its a typedef for signed char (macOS) because of macCatalyst. We can't change the type of the BOOL, since its ABI.

rdar://6510042

Thanks for taking a look!
Erik

[aaron.ballman]

clang/include/clang/Basic/Attr.td

3279

Is there a desire for similar functionality for C's _Bool datatype, or other, custom datatypes (perhaps BOOL from the Win32 APIs)? It seems like this attribute could be generalized to apply to any typedef to an integral type.

[erik.pilkington]

Address review comments.

clang/include/clang/Basic/Attr.td

3279

Sure, I guess there isn't anything Objective-C specific about this. The new patches renames it to clamping_integral_bool (and permits it on any integral type).

[dexonsmith]

clang/include/clang/Basic/Attr.td

3279

Given that "clamping" isn't exactly what's going on -- since -1 becomes 1, not 0 -- is there another word we can use here?

[aaron.ballman]

clang/include/clang/Basic/Attr.td

3279

Given that "clamping" isn't exactly what's going on -- since -1 becomes 1, not 0 -- is there another word we can use here?

converting_integral_bool is a bit closer to what it's doing. Or mapping. But neither of those names really strike me as more amazing than Erik's current name. boolify.

clang/include/clang/Basic/AttrDocs.td

4153

Its -> It's or It is, your call.

clang/include/clang/Basic/DiagnosticSemaKinds.td

2712

"'clamping_integral_bool' attribute only applies to a typedef to an integral type"

clang/lib/CodeGen/CodeGenFunction.h

3400

Please give the parameter a name.

clang/include/clang/Basic/Attr.td

 }
 
 def ObjCExternallyRetained : InheritableAttr {
   let LangOpts = [ObjCAutoRefCount];
   let Spellings = [Clang<"objc_externally_retained">];
   let Subjects = SubjectList<[NonParmVar, Function, Block, ObjCMethod]>;
   let Documentation = [ObjCExternallyRetainedDocs];
 }
+
+def ObjCClampingBool : TypeAttr {
+  let Spellings = [Clang<"objc_clamping_bool">];
+  let Subjects = SubjectList<[TypedefName]>;
+  let Documentation = [ObjCClampingBoolDocs];
+}

clang/include/clang/Basic/AttrDocs.td

   }
 
 Likewise, when applied to a strong local variable, that variable becomes
 ``const`` and is considered externally-retained.
 
 When compiled without ``-fobjc-arc``, this attribute is ignored.
 }]; }
 
+def ObjCClampingBoolDocs : Documentation {
+  let Category = DocCatType;
+  let Content = [{
+The ``objc_clamping_bool`` attribute appertains only to typedefs that alias type
+'signed char'. This narrow scope means that its only useful to implement
+Objective-C BOOL.
+
+On macOS, BOOL is a typedef for signed char, but on other platforms, its a
+typedef for a native boolean type. This introduces incompatibilities when
+porting code from iOS to macOS, which happens frequently (especially with
+macCatalyst). When this attribute is used, stores, loads, and casts to BOOL
+become "clamped", so non-zero values become YES, and zero values become NO.
+
+This attribute is just sugar on the underlying type, which is still signed char.
+If used with a language feature that canonicalizes the type (such as C++
+templates or auto) the sugar is stripped away, and with it the clamping
+behaviour. For this reason, its preferable to update code to perform the
+conversion manually, like: `some_value ? YES : NO`.
+  }];
+}
+
 def MIGConventionDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
   The Mach Interface Generator release-on-success convention dictates
 functions that follow it to only release arguments passed to them when they
 return "success" (a ``kern_return_t`` error code that indicates that
 no errors have occured). Otherwise the release is performed by the MIG client
 that called the function. The annotation ``__attribute__((mig_server_routine))``

   let Content = [{
 The GNU style attribute __attribute__((hip_pinned_shadow)) or MSVC style attribute
 __declspec(hip_pinned_shadow) can be added to the definition of a global variable
 to indicate it is a HIP pinned shadow variable. A HIP pinned shadow variable can
 be accessed on both device side and host side. It has external linkage and is
 not initialized on device side. It has internal linkage and is initialized by
 the initializer on host side.
   }];
-}
+}
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

   "'%0' parameter can only be used with swiftcall calling convention">;
 def err_swift_indirect_result_not_first : Error<
   "'swift_indirect_result' parameters must be first parameters of function">;
 def err_swift_error_result_not_after_swift_context : Error<
   "'swift_error_result' parameter must follow 'swift_context' parameter">;
 def err_swift_abi_parameter_wrong_type : Error<
   "'%0' parameter must have pointer%select{| to unqualified pointer}1 type; "
   "type here is %2">;
+def err_objc_clamping_bool_bad_type : Error<
+  "objc_clamping_bool must appertain to a signed char type">;
 
 def err_attribute_argument_invalid : Error<
   "%0 attribute argument is invalid: %select{max must be 0 since min is 0|"
   "min must not be greater than max}1">;
 def err_attribute_argument_is_zero : Error<
   "%0 attribute must be greater than 0">;
 def warn_attribute_argument_n_negative : Warning<
   "%0 attribute parameter %1 is negative and will be ignored">,

clang/lib/AST/TypePrinter.cpp

   case attr::UPtr:
   case attr::AddressSpace:
     llvm_unreachable("This attribute should have been handled already");
 
   case attr::NSReturnsRetained:
     OS << "ns_returns_retained";
     break;
 
+  case attr::ObjCClampingBool:
+    OS << "objc_clamping_bool";
+    break;
+
   // FIXME: When Sema learns to form this AttributedType, avoid printing the
   // attribute again in printFunctionProtoAfter.
   case attr::AnyX86NoCfCheck: OS << "nocf_check"; break;
   case attr::CDecl: OS << "cdecl"; break;
   case attr::FastCall: OS << "fastcall"; break;
   case attr::StdCall: OS << "stdcall"; break;
   case attr::ThisCall: OS << "thiscall"; break;
   case attr::SwiftCall: OS << "swiftcall"; break;

clang/lib/CodeGen/CGExpr.cpp

     // attach range metadata to the load.
   } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
     if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
       Load->setMetadata(llvm::LLVMContext::MD_range, RangeInfo);
 
   return EmitFromMemory(Load, Ty);
 }
 
+llvm::Value *CodeGenFunction::EmitObjCBoolClamp(llvm::Value *Value) {
+  assert(Value->getType()->isIntegerTy(8) && "non-signed char ObjC BOOL?");
+
+  // Avoid double-clamping if we know that Value is already boolean.
+  if (auto *ZExt = dyn_cast<llvm::ZExtInst>(Value))
+    if (ZExt->getOperand(0)->getType()->isIntegerTy(1))
+      return Value;
+
+  llvm::Value *NonZero = Builder.CreateIsNotNull(Value);
+  return Builder.CreateZExt(NonZero, Value->getType(), "BOOL.clamp");
+}
+
 llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value *Value, QualType Ty) {
   // Bool has a different representation in memory than in registers.
   if (hasBooleanRepresentation(Ty)) {
     // This should really always be an i1, but sometimes it's already
     // an i8, and it's awkward to track those cases down.
     if (Value->getType()->isIntegerTy(1))
       return Builder.CreateZExt(Value, ConvertTypeForMem(Ty), "frombool");
     assert(Value->getType()->isIntegerTy(getContext().getTypeSize(Ty)) &&
            "wrong value rep of bool");
   }
 
+  if (Ty->hasAttr(attr::ObjCClampingBool))
+    Value = EmitObjCBoolClamp(Value);
+
   return Value;
 }
 
 llvm::Value *CodeGenFunction::EmitFromMemory(llvm::Value *Value, QualType Ty) {
   // Bool has a different representation in memory than in registers.
   if (hasBooleanRepresentation(Ty)) {
     assert(Value->getType()->isIntegerTy(getContext().getTypeSize(Ty)) &&
            "wrong value rep of bool");
     return Builder.CreateTrunc(Value, Builder.getInt1Ty(), "tobool");
   }
 
+  if (Ty->hasAttr(attr::ObjCClampingBool))
+    Value = EmitObjCBoolClamp(Value);
+
   return Value;
 }
 
 void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, Address Addr,
                                         bool Volatile, QualType Ty,
                                         LValueBaseInfo BaseInfo,
                                         TBAAAccessInfo TBAAInfo,
                                         bool isInit, bool isNontemporal) {

       Val = Builder.CreateLShr(Val, Info.Offset, "bf.lshr");
     if (static_cast<unsigned>(Info.Offset) + Info.Size < Info.StorageSize)
       Val = Builder.CreateAnd(Val, llvm::APInt::getLowBitsSet(Info.StorageSize,
                                                               Info.Size),
                               "bf.clear");
   }
   Val = Builder.CreateIntCast(Val, ResLTy, Info.IsSigned, "bf.cast");
   EmitScalarRangeCheck(Val, LV.getType(), Loc);
+  if (LV.getType()->hasAttr(attr::ObjCClampingBool))
+    Val = EmitObjCBoolClamp(Val);
   return RValue::get(Val);
 }
 
 // If this is a reference to a subset of the elements of a vector, create an
 // appropriate shufflevector.
 RValue CodeGenFunction::EmitLoadOfExtVectorElementLValue(LValue LV) {
   llvm::Value *Vec = Builder.CreateLoad(LV.getExtVectorAddress(),
                                         LV.isVolatileQualified());

clang/lib/CodeGen/CGExprScalar.cpp

   llvm::Type *SrcTy = Src->getType();
 
   // Handle conversions to bool first, they are special: comparisons against 0.
   if (DstType->isBooleanType())
     return EmitConversionToBool(Src, SrcType);
 
   llvm::Type *DstTy = ConvertType(DstType);
 
+  // If we're converting a value to an ObjC BOOL thats annotated with
+  // objc_clamping_bool, then emit the same code as a native bool conversion.
+  if (NoncanonicalDstType->hasAttr(attr::ObjCClampingBool)) {
+    assert(DstType->isSpecificBuiltinType(BuiltinType::SChar) &&
+           "objc_clamping_bool only appertains to signed char!");
+    Value *ToBool = EmitConversionToBool(Src, SrcType);
+    return Builder.CreateZExt(ToBool, DstTy, "BOOL.clamp");
+  }
+
   // Cast from half through float if half isn't a native type.
   if (SrcType->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType) {
     // Cast to FP using the intrinsic if the half type itself isn't supported.
     if (DstTy->isFloatingPointTy()) {
       if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics())
         return Builder.CreateCall(
             CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_from_fp16, DstTy),
             Src);

         IntToPtr = Builder.CreateLaunderInvariantGroup(IntToPtr);
     }
     return IntToPtr;
   }
   case CK_PointerToIntegral: {
     assert(!DestTy->isBooleanType() && "bool should use PointerToBool");
     auto *PtrExpr = Visit(E);
 
+    // If we're converting a pointer to an ObjC BOOL thats annotated with
+    // objc_clamping_bool, then emit the same code as a native bool conversion.
+    if (DestTy->hasAttr(attr::ObjCClampingBool)) {
+      assert(DestTy->isSpecificBuiltinType(BuiltinType::SChar) &&
+             "objc_clamping_bool only appertains to signed char!");
+      llvm::Value *ToBool = EmitPointerToBoolConversion(PtrExpr, E->getType());
+      return Builder.CreateZExt(ToBool, ConvertType(DestTy), "clamped.bool");
+    }
+
     if (CGF.CGM.getCodeGenOpts().StrictVTablePointers) {
       const QualType SrcType = E->getType();
 
       // Casting to integer requires stripping dynamic information as it does
       // not carries it.
       if (SrcType.mayBeDynamicClass())
         PtrExpr = Builder.CreateStripInvariantGroup(PtrExpr);
     }

clang/lib/CodeGen/CodeGenFunction.h

       llvm::AtomicOrdering Failure =
           llvm::AtomicOrdering::SequentiallyConsistent,
       bool IsWeak = false, AggValueSlot Slot = AggValueSlot::ignored());
 
   void EmitAtomicUpdate(LValue LVal, llvm::AtomicOrdering AO,
                         const llvm::function_ref<RValue(RValue)> &UpdateOp,
                         bool IsVolatile);
 
+  /// Emit code to "clamp" an i8 BOOL in Objective-C to {0,1}.
+  llvm::Value *EmitObjCBoolClamp(llvm::Value *);
+
   /// EmitToMemory - Change a scalar value from its value
   /// representation to its in-memory representation.
   llvm::Value *EmitToMemory(llvm::Value *Value, QualType Ty);
 
   /// EmitFromMemory - Change a scalar value from its memory
   /// representation to its value representation.
   llvm::Value *EmitFromMemory(llvm::Value *Value, QualType Ty);
 

clang/lib/Sema/SemaType.cpp

     CurType = State.getAttributedType(
         createSimpleAttr<LifetimeBoundAttr>(State.getSema().Context, Attr),
         CurType, CurType);
   } else {
     Attr.diagnoseAppertainsTo(State.getSema(), nullptr);
   }
 }
 
+static void handleObjCClampingBoolAttr(TypeProcessingState &State,
+                                       QualType &CurType, ParsedAttr &Attr) {
+  if (!CurType->isSpecificBuiltinType(BuiltinType::SChar)) {
+    State.getSema().Diag(Attr.getLoc(), diag::err_objc_clamping_bool_bad_type);
+    Attr.setInvalid();
+    return;
+  }
+
+  ObjCClampingBoolAttr *ClampAttr = createSimpleAttr<ObjCClampingBoolAttr>(
+      State.getSema().getASTContext(), Attr);
+  CurType = State.getAttributedType(ClampAttr, CurType, CurType);
+}
 
 static void processTypeAttrs(TypeProcessingState &state, QualType &type,
                              TypeAttrLocation TAL,
                              ParsedAttributesView &attrs) {
   // Scan through and apply attributes to this type where it makes sense.  Some
   // attributes (such as __address_space__, __vector_size__, etc) apply to the
   // type, but others can be present in the type specifiers even though they
   // apply to the decl.  Here we apply type attributes and ignore the rest.

         break;
       }
 
       // Apply it regardless.
       if (checkObjCKindOfType(state, type, attr))
         attr.setInvalid();
       break;
 
+    case ParsedAttr::AT_ObjCClampingBool:
+      handleObjCClampingBoolAttr(state, type, attr);
+      attr.setUsedAsTypeAttr();
+      break;
+
     case ParsedAttr::AT_NoThrow:
     // Exception Specifications aren't generally supported in C mode throughout
     // clang, so revert to attribute-based handling for C.
       if (!state.getSema().getLangOpts().CPlusPlus)
         break;
       LLVM_FALLTHROUGH;
     FUNCTION_TYPE_ATTRS_CASELIST:
       attr.setUsedAsTypeAttr();

clang/test/CodeGenObjC/objc-clamping-bool.m

+// RUN: %clang_cc1 %s -triple x86_64-apple-macosx10.14 -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+typedef __attribute__((objc_clamping_bool)) signed char BOOL;
+
+BOOL b;
+
+void store() {
+  // CHECK-LABEL: define void @store
+
+  b = 2;
+  // CHECK: store i8 1, i8* @b, align 1
+
+  b = 1;
+  // CHECK: store i8 1, i8* @b, align 1
+
+  b = -1;
+  // CHECK: store i8 1, i8* @b, align 1
+
+  b = 0;
+  // CHECK: store i8 0, i8* @b, align 1
+
+  b = 256;
+  // CHECK: store i8 1, i8* @b, align 1
+
+  int unknown_value;
+  b = unknown_value;
+
+  // CHECK: [[CMPNE:%.*]] = icmp ne i32 %{{.*}}, 0
+  // CHECK-NEXT: [[ZEXT:%.*]] = zext i1 [[CMPNE]] to i8
+}
+
+void load() {
+  // CHECK-LABEL: define void @load
+  int load = b;
+
+  // CHECK: [[LOAD:%.*]] = load i8, i8* @b, align 1
+  // CHECK-NEXT: [[CMP:%.*]] = icmp ne i8 %0, 0
+  // CHECK-NEXT: zext i1 [[CMP]] to i8
+}
+
+void cast() {
+  // CHECK-LABEL: define void @cast
+  int i;
+  float f;
+  long double ld;
+  int* p;
+  _Bool real_bool;
+
+  (BOOL)i;
+  // CHECK: [[CMP:%.*]] = icmp ne i32 %{{.*}}, 0
+  // CHECK-NEXT: zext i1 [[CMP]] to i8
+
+  (BOOL)f;
+  // CHECK: [[CMP:%.*]] = fcmp une float %{{.*}}, 0.000000e+00
+  // CHECK-NEXT: zext i1 [[CMP]] to i8
+
+  (BOOL)ld;
+  // CHECK: [[CMP:%.*]] = fcmp une x86_fp80 %{{.*}}, 0xK00000000000000000000
+  // CHECK-NEXT: zext i1 [[CMP]] to i8
+
+  (BOOL)p;
+  // CHECK: [[CMP:%.*]] = icmp ne i32* %{{.*}}, null
+  // CHECK-NEXT: zext i1 [[CMP]] to i8
+
+  (BOOL)real_bool;
+  // CHECK: [[CMP:%.*]] = icmp ne i1 %{{.*}}, false
+  // CHECK-NEXT: zext i1 [[CMP]] to i8
+}
+
+void bits() {
+  // CHECK-LABEL: define void @bits
+  struct bit_field {
+    BOOL b : 1;
+  };
+
+  struct bit_field bf;
+
+  bf.b = 1;
+  // CHECK: [[SET_BIT:%.*]] = or i8 %{{.*}}, 1
+  // CHECK-NEXT: store i8 [[SET_BIT]]
+
+  bf.b = 2;
+  // CHECK: [[SET_BIT:%.*]] = or i8 %{{.*}}, 1
+  // CHECK-NEXT: store i8 [[SET_BIT]]
+
+  bf.b = 0;
+  // CHECK: [[ZERO_BIT:%.*]] = and i8 %{{.*}}, -2
+  // CHECK-NEXT: store i8 [[ZERO_BIT]]
+
+  bf.b = -1;
+  // CHECK: [[SET_BIT:%.*]] = or i8 %{{.*}}, 1
+  // CHECK-NEXT: store i8 [[SET_BIT]]
+
+  (int)bf.b;
+  // CHECK: shl i8 %{{.*}}, 7
+  // CHECK-NEXT: [[LOAD:%.*]] = ashr i8 %{{.*}}, 7
+  // CHECK-NEXT: [[CMP:%.*]] = icmp ne i8 [[LOAD]], 0
+  // CHECK-NEXT: zext i1 [[CMP]] to i8
+  // CHECK-NOT: icmp
+
+  (BOOL)bf.b;
+  // CHECK: shl i8 %{{.*}}, 7
+  // CHECK-NEXT: [[LOAD:%.*]] = ashr i8 %{{.*}}, 7
+  // CHECK-NEXT: [[CMP:%.*]] = icmp ne i8 [[LOAD]], 0
+  // CHECK-NEXT: zext i1 [[CMP]] to i8
+  // CHECK-NOT: icmp
+}
+
+@interface HasProp
+@property BOOL b;
+@property int i;
+@end
+
+void prop(HasProp *p) {
+  // CHECK-LABEL: define void @prop
+  p.b = 43;
+  // CHECK: call void {{.*}} @objc_msgSend {{.*}}(i8* {{.*}}, i8*{{.*}}, i8 signext 1)
+  p.b = 0;
+  // CHECK: call void {{.*}} @objc_msgSend {{.*}}(i8* {{.*}}, i8*{{.*}}, i8 signext 0)
+  p.b = -1;
+  // CHECK: call void {{.*}} @objc_msgSend {{.*}}(i8* {{.*}}, i8*{{.*}}, i8 signext 1)
+  p.b = 1;
+  // CHECK: call void {{.*}} @objc_msgSend {{.*}}(i8* {{.*}}, i8*{{.*}}, i8 signext 1)
+
+  BOOL b;
+
+  p.i = b;
+  // CHECK: [[CMP:%.*]] = icmp ne i8 %{{.*}}, 0
+  // CHECK-NEXT: [[ZEXT:%.*]] = zext i1 [[CMP]] to i8
+  // CHECK-NEXT: [[SEXT:%.*]] = sext i8 [[ZEXT]] to i32
+  // CHECK: call void {{.*}} @objc_msgSend {{.*}}(i8* {{.*}}, i8*{{.*}}, i32 [[SEXT]])
+}

clang/test/Misc/pragma-attribute-supported-attributes-list.test

 // CHECK-NEXT: OSReturnsNotRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: OSReturnsRetained (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: OSReturnsRetainedOnNonZero (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: OSReturnsRetainedOnZero (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ObjCBoxable (SubjectMatchRule_record)
 // CHECK-NEXT: ObjCBridge (SubjectMatchRule_record, SubjectMatchRule_type_alias)
 // CHECK-NEXT: ObjCBridgeMutable (SubjectMatchRule_record)
 // CHECK-NEXT: ObjCBridgeRelated (SubjectMatchRule_record)
+// CHECK-NEXT: ObjCClampingBool (SubjectMatchRule_type_alias)
 // CHECK-NEXT: ObjCClassStub (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCDesignatedInitializer (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCException (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)

clang/test/SemaObjC/objc-clamping-bool.m

+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+typedef __attribute__((objc_clamping_bool)) signed char BOOL;
+
+// expected-error@+1 {{'objc_clamping_bool' attribute takes no arguments}}
+typedef __attribute__((objc_clamping_bool(1))) signed char BOOL2;
+
+// expected-warning@+1 {{'objc_clamping_bool' attribute only applies to typedefs}}
+__attribute__((objc_clamping_bool)) signed char x;
+
+// expected-error@+1 {{objc_clamping_bool must appertain to a signed char type}}
+typedef __attribute__((objc_clamping_bool)) unsigned char UNSIGNED_BOOL;