[Patch][ObjC][Proposal] NSValue literals

jahanian fjahanian at apple.com
Mon Dec 8 12:11:54 PST 2014


Overall looks very nice. Some comments below. (I have tried to make them easier to find by prepending them with >>).

Some general comments.
- Need distinct tests for iOS and macosx.
- Need to make sure that unavailability of the APIs are diagnosed. We do this with existing APIs.
  (We either need to error on older deployment targets—which in at least one case 
   means < OS X 10.10—or implement entrypoints in libarclite. )
- Sema tests need be expanded for issuing diagnostics for every new added error or warning.
- __has_feature test need be added.
- Fariborz

diff --git a/docs/ObjectiveCLiterals.rst b/docs/ObjectiveCLiterals.rst
index 8907c1e..a223fba 100644
--- a/docs/ObjectiveCLiterals.rst
+++ b/docs/ObjectiveCLiterals.rst
@@ -119,8 +119,8 @@ Objective-C provides a new syntax for boxing C expressions:
 
     @( <expression> )
 
-Expressions of scalar (numeric, enumerated, BOOL) and C string pointer
-types are supported:
+Expressions of scalar (numeric, enumerated, BOOL), C string pointer
+and NSValue constructible types are supported:
 
 .. code-block:: objc
 
@@ -136,6 +136,18 @@ types are supported:
     NSString *path = @(getenv("PATH"));       // [NSString stringWithUTF8String:(getenv("PATH"))]
     NSArray *pathComponents = [path componentsSeparatedByString:@":"];
 
+    // NS structs
+    NSValue *center = @(view.center);         // [NSValue valueWithPoint:view.center]
+    NSValue *frame = @(view.frame);           // [NSValue valueWithRect:view.frame]
+
+    // Pointers
+    NSValue *memObject = @( malloc(42) );     // [NSValue valueWithPointer:malloc(42)]
+
+    // Objective-C objects
+    id obj = [NSObject new];
+    NSValue *nonretainedObject = @(obj);      // [NSValue valueWithNonretainedObject:obj]
+    NSValue *nonretainedString = @(@"Hello"); // [NSValue valueWithNonretainedObject:@"Hello"]
+
 Boxed Enums
 -----------
 
@@ -218,6 +230,28 @@ character data is valid. Passing ``NULL`` as the character pointer will
 raise an exception at runtime. When possible, the compiler will reject
 ``NULL`` character pointers used in boxed expressions.
 
+Boxed C Strucutres, pointers and NSObjects
+------------------
+
+Boxed expressions support construction of NSValue objects.
+It said that some C structures, pointers and NSObjects can be used:
+
+.. code-block:: objc
+
+    NSPoint p;
+    NSValue *point = @(p);          // valueWithPoint:
+    NSSize s;
+    NSValue *size = @(s);           // valueWithSize:
+    const void *p = malloc(42);
+    NSValue *memory = @(p);         // valueWithPointer:
+    id obj = [NSObject new];
+    NSValue *nonretained = @(obj);  // valueWithNonretainedObject:
+
+The following list of structs supported, depends on target system:
+
+  - OSX: ``NSPoint``, ``NSSize``, ``NSRect``, ``NSRange``
+  - iOS: ``CGPoint``, ``CGSize``, ``CGRect``, ``NSRange``

>> Have you considered NSEdgeInsets? With this API:

@interface NSValue (NSValueGeometryExtensions)
…
+ (NSValue *)valueWithEdgeInsets:(NSEdgeInsets)insets __attribute__((availability(macosx,introduced=10.10)));
…

+
 Container Literals
 ==================
 
diff --git a/include/clang/AST/ExprObjC.h b/include/clang/AST/ExprObjC.h
index 817c0cc..f296e8f 100644
--- a/include/clang/AST/ExprObjC.h
+++ b/include/clang/AST/ExprObjC.h
@@ -124,6 +124,15 @@ public:
   
   // Iterators
   child_range children() { return child_range(&SubExpr, &SubExpr+1); }
+
+  typedef ConstExprIterator const_arg_iterator;
+
+  const_arg_iterator arg_begin() const {
+    return reinterpret_cast<Stmt const * const*>(&SubExpr);
+  }
+  const_arg_iterator arg_end() const {
+    return reinterpret_cast<Stmt const * const*>(&SubExpr + 1);
+  }
   
   friend class ASTStmtReader;
 };
diff --git a/include/clang/AST/NSAPI.h b/include/clang/AST/NSAPI.h
index 33fcce2..97654b5 100644
--- a/include/clang/AST/NSAPI.h
+++ b/include/clang/AST/NSAPI.h
@@ -33,9 +33,10 @@ public:
     ClassId_NSMutableArray,
     ClassId_NSDictionary,
     ClassId_NSMutableDictionary,
-    ClassId_NSNumber
+    ClassId_NSNumber,
+    ClassId_NSValue
   };
-  static const unsigned NumClassIds = 7;
+  static const unsigned NumClassIds = 8;
 
   enum NSStringMethodKind {
     NSStr_stringWithString,
@@ -158,12 +159,29 @@ public:
   };
   static const unsigned NumNSNumberLiteralMethods = 15;
 
+  /// \brief Enumerates the NSValue methods used to generate literals.
+  enum NSValueLiteralMethodKind {
+    NSValueWithPoint,
+    NSValueWithSize,
+    NSValueWithRect,
+    NSValueWithCGPoint,
+    NSValueWithCGSize,
+    NSValueWithCGRect,
+    NSValueWithRange,
+    NSValueWithPointer,
+    NSValueWithNonretainedObject
+  };
+  static const unsigned NumNSValueLiteralMethods = 9;
+
   /// \brief The Objective-C NSNumber selectors used to create NSNumber literals.
   /// \param Instance if true it will return the selector for the init* method
   /// otherwise it will return the selector for the number* method.
   Selector getNSNumberLiteralSelector(NSNumberLiteralMethodKind MK,
                                       bool Instance) const;
 
+  /// \brief The Objective-C NSValue selectors used to create NSValue literals.
+  Selector getNSValueLiteralSelector(NSValueLiteralMethodKind MK) const;
+
   bool isNSNumberLiteralSelector(NSNumberLiteralMethodKind MK,
                                  Selector Sel) const {
     return Sel == getNSNumberLiteralSelector(MK, false) ||
@@ -179,12 +197,31 @@ public:
   Optional<NSNumberLiteralMethodKind>
       getNSNumberFactoryMethodKind(QualType T) const;
 
+  /// \brief Determine the appropriate NSValue factory method kind for a
+  /// literal of the given type.
+  Optional<NSValueLiteralMethodKind>
+      getNSValueFactoryMethodKind(QualType T) const;
+
   /// \brief Returns true if \param T is a typedef of "BOOL" in objective-c.
   bool isObjCBOOLType(QualType T) const;
   /// \brief Returns true if \param T is a typedef of "NSInteger" in objective-c.
   bool isObjCNSIntegerType(QualType T) const;
   /// \brief Returns true if \param T is a typedef of "NSUInteger" in objective-c.
   bool isObjCNSUIntegerType(QualType T) const;
+  /// \brief Returns true if \param T is a typedef of "NSPoint" in objective-c.
+  bool isObjCNSPointType(QualType T) const;
+  /// \brief Returns true if \param T is a typedef of "NSSize" in objective-c.
+  bool isObjCNSSizeType(QualType T) const;
+  /// \brief Returns true if \param T is a typedef of "NSRect" in objective-c.
+  bool isObjCNSRectType(QualType T) const;
+  /// \brief Returns true if \param T is a typedef of "CGPoint" in objective-c.
+  bool isObjCCGPointType(QualType T) const;
+  /// \brief Returns true if \param T is a typedef of "CGSize" in objective-c.
+  bool isObjCCGSizeType(QualType T) const;
+  /// \brief Returns true if \param T is a typedef of "CGRect" in objective-c.
+  bool isObjCCGRectType(QualType T) const;
+  /// \brief Returns true if \param T is a typedef of "NSRange" in objective-c.
+  bool isObjCNSRangeType(QualType T) const;
   /// \brief Returns one of NSIntegral typedef names if \param T is a typedef
   /// of that name in objective-c.
   StringRef GetNSIntegralKind(QualType T) const;
@@ -211,12 +248,18 @@ private:
   mutable Selector NSNumberClassSelectors[NumNSNumberLiteralMethods];
   mutable Selector NSNumberInstanceSelectors[NumNSNumberLiteralMethods];
 
+  /// \brief The Objective-C NSValue selectors used to create NSValue literals.
+  mutable Selector NSValueClassSelectors[NumNSValueLiteralMethods];
+
   mutable Selector objectForKeyedSubscriptSel, objectAtIndexedSubscriptSel,
                    setObjectForKeyedSubscriptSel,setObjectAtIndexedSubscriptSel,
                    isEqualSel;
 
   mutable IdentifierInfo *BOOLId, *NSIntegerId, *NSUIntegerId;
   mutable IdentifierInfo *NSASCIIStringEncodingId, *NSUTF8StringEncodingId;
+  mutable IdentifierInfo *NSPointId, *NSSizeId, *NSRectId;
+  mutable IdentifierInfo *CGPointId, *CGSizeId, *CGRectId;
+  mutable IdentifierInfo *NSRangeId;
 };
 
 }  // end namespace clang
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 28ac96d..515fd63 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2042,6 +2042,8 @@ def err_attr_objc_ownership_redundant : Error<
   "the type %0 is already explicitly ownership-qualified">;
 def err_undeclared_nsnumber : Error<
   "NSNumber must be available to use Objective-C literals">;
+def err_undeclared_nsvalue : Error<
+  "NSValue must be available to use Objective-C boxed expressions”>;
 def err_invalid_nsnumber_type : Error<
   "%0 is not a valid literal type for NSNumber">;
 def err_undeclared_nsstring : Error<
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 02aa984..dbf6cd4 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -660,12 +660,21 @@ public:
   /// \brief The declaration of the Objective-C NSNumber class.
   ObjCInterfaceDecl *NSNumberDecl;
 
+  /// \brief The declaration of the Objective-C NSValue class.
+  ObjCInterfaceDecl *NSValueDecl;
+
   /// \brief Pointer to NSNumber type (NSNumber *).
   QualType NSNumberPointer;
 
+  /// \brief Pointer to NSValue type (NSValue *).
+  QualType NSValuePointer;
+
   /// \brief The Objective-C NSNumber methods used to create NSNumber literals.
   ObjCMethodDecl *NSNumberLiteralMethods[NSAPI::NumNSNumberLiteralMethods];
 
+  /// \brief The Objective-C NSValue methods used to create NSValue literals.
+  ObjCMethodDecl *NSValueLiteralMethods[NSAPI::NumNSValueLiteralMethods];
+
   /// \brief The declaration of the Objective-C NSString class.
   ObjCInterfaceDecl *NSStringDecl;
 
diff --git a/lib/AST/NSAPI.cpp b/lib/AST/NSAPI.cpp
index 3dc750a..3b5b1b2 100644
--- a/lib/AST/NSAPI.cpp
+++ b/lib/AST/NSAPI.cpp
@@ -17,7 +17,9 @@ using namespace clang;
 NSAPI::NSAPI(ASTContext &ctx)
   : Ctx(ctx), ClassIds(), BOOLId(nullptr), NSIntegerId(nullptr),
     NSUIntegerId(nullptr), NSASCIIStringEncodingId(nullptr),
-    NSUTF8StringEncodingId(nullptr) {}
+    NSUTF8StringEncodingId(nullptr), NSPointId(nullptr),
+    NSSizeId(nullptr), NSRectId(nullptr), CGPointId(nullptr),
+    CGSizeId(nullptr), CGRectId(nullptr), NSRangeId(nullptr) {}
 
 IdentifierInfo *NSAPI::getNSClassId(NSClassIdKindKind K) const {
   static const char *ClassName[NumClassIds] = {
@@ -27,7 +29,8 @@ IdentifierInfo *NSAPI::getNSClassId(NSClassIdKindKind K) const {
     "NSMutableArray",
     "NSDictionary",
     "NSMutableDictionary",
-    "NSNumber"
+    "NSNumber",
+    "NSValue"
   };
 
   if (!ClassIds[K])
@@ -279,6 +282,25 @@ Selector NSAPI::getNSNumberLiteralSelector(NSNumberLiteralMethodKind MK,
   return Sels[MK];
 }
 
+Selector NSAPI::getNSValueLiteralSelector(NSValueLiteralMethodKind MK) const {
+  static const char *ClassSelectorName[NumNSValueLiteralMethods] = {
+    "valueWithPoint",
+    "valueWithSize",
+    "valueWithRect",
+    "valueWithCGPoint",
+    "valueWithCGSize",
+    "valueWithCGRect",
+    "valueWithRange",
+    "valueWithPointer",
+    "valueWithNonretainedObject"
+  };
+
+  if (NSValueClassSelectors[MK].isNull())
+    NSValueClassSelectors[MK] =
+      Ctx.Selectors.getUnarySelector(&Ctx.Idents.get(ClassSelectorName[MK]));
+  return NSValueClassSelectors[MK];
+}
+
 Optional<NSAPI::NSNumberLiteralMethodKind>
 NSAPI::getNSNumberLiteralMethodKind(Selector Sel) const {
   for (unsigned i = 0; i != NumNSNumberLiteralMethods; ++i) {
@@ -371,6 +393,34 @@ NSAPI::getNSNumberFactoryMethodKind(QualType T) const {
   return None;
 }
 
+Optional<NSAPI::NSValueLiteralMethodKind>
+NSAPI::getNSValueFactoryMethodKind(QualType T) const {
+  const RecordType *RT = T->getAsStructureType();
+  if (RT) {
+    if (isObjCNSPointType(T))
+      return NSAPI::NSValueWithPoint;
+    if (isObjCNSSizeType(T))
+      return NSAPI::NSValueWithSize;
+    if (isObjCNSRectType(T))
+      return NSAPI::NSValueWithRect;
+    if (isObjCCGPointType(T))
+      return NSAPI::NSValueWithCGPoint;
+    if (isObjCCGSizeType(T))
+      return NSAPI::NSValueWithCGSize;
+    if (isObjCCGRectType(T))
+      return NSAPI::NSValueWithCGRect;
+    if (isObjCNSRangeType(T))
+      return NSAPI::NSValueWithRange;

>> Better to return None; here instead of falling though with unnecessary extra checks. It is also more verbose as to the intention.

+  }
+
+  if (T->isVoidPointerType())
+    return NSAPI::NSValueWithPointer;
+  if (T->isObjCObjectPointerType())

>> You are intentionally using this API instead of T->isObjCIdType(), right? If so, please add comment.

+    return NSAPI::NSValueWithNonretainedObject;
+
+  return None;
+}
+
 /// \brief Returns true if \param T is a typedef of "BOOL" in objective-c.
 bool NSAPI::isObjCBOOLType(QualType T) const {
   return isObjCTypedef(T, "BOOL", BOOLId);
@@ -384,6 +434,41 @@ bool NSAPI::isObjCNSUIntegerType(QualType T) const {
   return isObjCTypedef(T, "NSUInteger", NSUIntegerId);
 }
 
+/// \brief Returns true if \param T is a typedef of "NSPoint" in objective-c.
+bool NSAPI::isObjCNSPointType(QualType T) const {
+  return isObjCTypedef(T, "NSPoint", NSPointId);
+}
+
+/// \brief Returns true if \param T is a typedef of "NSSize" in objective-c.
+bool NSAPI::isObjCNSSizeType(QualType T) const {
+  return isObjCTypedef(T, "NSSize", NSSizeId);
+}
+
+/// \brief Returns true if \param T is a typedef of "NSRect" in objective-c.
+bool NSAPI::isObjCNSRectType(QualType T) const {
+  return isObjCTypedef(T, "NSRect", NSRectId);
+}
+
+/// \brief Returns true if \param T is a typedef of "CGPoint" in objective-c.
+bool NSAPI::isObjCCGPointType(QualType T) const {
+  return isObjCTypedef(T, "CGPoint", CGPointId);
+}
+
+/// \brief Returns true if \param T is a typedef of "CGSize" in objective-c.
+bool NSAPI::isObjCCGSizeType(QualType T) const {
+  return isObjCTypedef(T, "CGSize", CGSizeId);
+}
+
+/// \brief Returns true if \param T is a typedef of "CGRect" in objective-c.
+bool NSAPI::isObjCCGRectType(QualType T) const {
+  return isObjCTypedef(T, "CGRect", CGRectId);
+}
+
+/// \brief Returns true if \param T is a typedef of "NSRange" in objective-c.
+bool NSAPI::isObjCNSRangeType(QualType T) const {
+  return isObjCTypedef(T, "NSRange", NSRangeId);
+}
+
 StringRef NSAPI::GetNSIntegralKind(QualType T) const {
   if (!Ctx.getLangOpts().ObjC1 || T.isNull())
     return StringRef();
diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp
index ca67c4b..d2975b8 100644
--- a/lib/CodeGen/CGObjC.cpp
+++ b/lib/CodeGen/CGObjC.cpp
@@ -55,12 +55,12 @@ llvm::Value *CodeGenFunction::EmitObjCStringLiteral(const ObjCStringLiteral *E)
 
 /// EmitObjCBoxedExpr - This routine generates code to call
 /// the appropriate expression boxing method. This will either be
-/// one of +[NSNumber numberWith<Type>:], or +[NSString stringWithUTF8String:].
+/// one of +[NSNumber numberWith<Type>:], or +[NSString stringWithUTF8String:],
+/// or [NSValue valueWith<Type>:].
 ///
 llvm::Value *
 CodeGenFunction::EmitObjCBoxedExpr(const ObjCBoxedExpr *E) {
   // Generate the correct selector for this literal's concrete type.
-  const Expr *SubExpr = E->getSubExpr();
   // Get the method.
   const ObjCMethodDecl *BoxingMethod = E->getBoxingMethod();
   assert(BoxingMethod && "BoxingMethod is null");
@@ -73,12 +73,9 @@ CodeGenFunction::EmitObjCBoxedExpr(const ObjCBoxedExpr *E) {
   CGObjCRuntime &Runtime = CGM.getObjCRuntime();
   const ObjCInterfaceDecl *ClassDecl = BoxingMethod->getClassInterface();
   llvm::Value *Receiver = Runtime.GetClass(*this, ClassDecl);
-  
-  const ParmVarDecl *argDecl = *BoxingMethod->param_begin();
-  QualType ArgQT = argDecl->getType().getUnqualifiedType();
-  RValue RV = EmitAnyExpr(SubExpr);
+
   CallArgList Args;
-  Args.add(RV, ArgQT);
+  EmitCallArgs(Args, BoxingMethod, E->arg_begin(), E->arg_end());

>> Isn’t this something you can check-in before the overall patch can be checked in?
 
   RValue result = Runtime.GenerateMessageSend(
       *this, ReturnValueSlot(), BoxingMethod->getReturnType(), Sel, Receiver,
diff --git a/lib/Lex/PPMacroExpansion.cpp b/lib/Lex/PPMacroExpansion.cpp
index b402a93..a17052f 100644
--- a/lib/Lex/PPMacroExpansion.cpp
+++ b/lib/Lex/PPMacroExpansion.cpp
@@ -911,6 +911,7 @@ static bool HasFeature(const Preprocessor &PP, const IdentifierInfo *II) {
       .Case("objc_array_literals", LangOpts.ObjC2)
       .Case("objc_dictionary_literals", LangOpts.ObjC2)
       .Case("objc_boxed_expressions", LangOpts.ObjC2)
+      .Case("objc_boxed_nsvalue_expressions", LangOpts.ObjC2)

>> Presumable the new __has_feature need be documented somewhere.
>> Also, why can’t place this under the umbrella objc_boxed_expressions?

       .Case("arc_cf_code_audited", true)
       // C11 features
       .Case("c_alignas", LangOpts.C11)
diff --git a/lib/Sema/SemaExprObjC.cpp b/lib/Sema/SemaExprObjC.cpp
index eeee352..2d3fa5b 100644
--- a/lib/Sema/SemaExprObjC.cpp
+++ b/lib/Sema/SemaExprObjC.cpp
@@ -255,6 +255,86 @@ static ObjCMethodDecl *getNSNumberFactoryMethod(Sema &S, SourceLocation Loc,
   return Method;
 }
 
+/// \brief Retrieve the NSValue factory method that should be used to create
+/// an Objective-C literal for the given type.
+static ObjCMethodDecl *getNSValueFactoryMethod(Sema &S, SourceLocation Loc,
+                                               QualType ValueType) {
+  Optional<NSAPI::NSValueLiteralMethodKind> Kind =
+  S.NSAPIObj->getNSValueFactoryMethodKind(ValueType);
+
+  if (!Kind) {
+    return nullptr;
+  }
+
+  // If we already looked up this method, we're done.
+  if (S.NSValueLiteralMethods[*Kind])
+    return S.NSValueLiteralMethods[*Kind];
+
+  Selector Sel = S.NSAPIObj->getNSValueLiteralSelector(*Kind);
+
+  ASTContext &CX = S.Context;
+
+  // Look up the NSValue class, if we haven't done so already. It's cached
+  // in the Sema instance.
+  if (!S.NSValueDecl) {
+    IdentifierInfo *NSValueId =
+    S.NSAPIObj->getNSClassId(NSAPI::ClassId_NSValue);
+    NamedDecl *IF = S.LookupSingleName(S.TUScope, NSValueId,
+                                       Loc, Sema::LookupOrdinaryName);
+    S.NSValueDecl = dyn_cast_or_null<ObjCInterfaceDecl>(IF);
+    if (!S.NSValueDecl) {
+      if (S.getLangOpts().DebuggerObjCLiteral) {
+        // Create a stub definition of NSValue.
+        S.NSValueDecl = ObjCInterfaceDecl::Create(CX,
+                                                  CX.getTranslationUnitDecl(),
+                                                  SourceLocation(), NSValueId,
+                                                  nullptr, SourceLocation());
+      } else {
+        // Otherwise, require a declaration of NSValue.
+        S.Diag(Loc, diag::err_undeclared_nsvalue);
+        return nullptr;
+      }
+    } else if (!S.NSValueDecl->hasDefinition()) {
+      S.Diag(Loc, diag::err_undeclared_nsvalue);

>> Maybe we should have a clearer diagnostic here.

+      return nullptr;
+    }
+
+    // generate the pointer to NSValue type.
+    QualType NSValueObject = CX.getObjCInterfaceType(S.NSValueDecl);
+    S.NSValuePointer = CX.getObjCObjectPointerType(NSValueObject);
+  }
+
+  // Look for the appropriate method within NSValue.
+  ObjCMethodDecl *Method = S.NSValueDecl->lookupClassMethod(Sel);
+  if (!Method && S.getLangOpts().DebuggerObjCLiteral) {
+    // create a stub definition this NSValue factory method.
+    TypeSourceInfo *ReturnTInfo = nullptr;
+    Method =
+    ObjCMethodDecl::Create(CX, SourceLocation(), SourceLocation(), Sel,
+                           S.NSValuePointer, ReturnTInfo, S.NSValueDecl,
+                           /*isInstance=*/false, /*isVariadic=*/false,
+                           /*isPropertyAccessor=*/false,
+                           /*isImplicitlyDeclared=*/true,
+                           /*isDefined=*/false, ObjCMethodDecl::Required,
+                           /*HasRelatedResultType=*/false);
+    ParmVarDecl *value = ParmVarDecl::Create(S.Context, Method,
+                                             SourceLocation(), SourceLocation(),
+                                             &CX.Idents.get("value"),
+                                             ValueType, /*TInfo=*/nullptr,
+                                             SC_None, nullptr);
+    Method->setMethodParams(S.Context, value, None);
+  }
+
+  if (!validateBoxingMethod(S, Loc, S.NSValueDecl, Sel, Method))
+    return nullptr;
+
+  // Note: if the parameter type is out-of-line, we'll catch it later in the
+  // implicit conversion.
+
+  S.NSValueLiteralMethods[*Kind] = Method;
+  return Method;
+}
+
 /// BuildObjCNumericLiteral - builds an ObjCBoxedExpr AST node for the
 /// numeric literal expression. Type of the expression will be "NSNumber *”.

>> Comment need be updated.

 ExprResult Sema::BuildObjCNumericLiteral(SourceLocation AtLoc, Expr *Number) {
@@ -456,10 +536,55 @@ ExprResult Sema::BuildObjCBoxedExpr(SourceRange SR, Expr *ValueExpr) {
   }
   ValueExpr = RValue.get();
   QualType ValueType(ValueExpr->getType());
-  if (const PointerType *PT = ValueType->getAs<PointerType>()) {
-    QualType PointeeType = PT->getPointeeType();
-    if (Context.hasSameUnqualifiedType(PointeeType, Context.CharTy)) {
+  if (ValueType->isBuiltinType() || ValueType->isEnumeralType()) {

>> You are adding an extra check for isEnumeralType() beyond the original source. Please add
>> comment for it and add test to back up the comment.

+    // We support numeric, char and BOOL/bool types.
+    // Check for a top-level character literal.
+    if (const CharacterLiteral *Char =
+        dyn_cast<CharacterLiteral>(ValueExpr->IgnoreParens())) {
+      // In C, character literals have type 'int'. That's not the type we want
+      // to use to determine the Objective-c literal kind.
+      switch (Char->getKind()) {
+      case CharacterLiteral::Ascii:
+        ValueType = Context.CharTy;
+        break;
 
+      case CharacterLiteral::Wide:
+        ValueType = Context.getWideCharType();
+        break;
+
+      case CharacterLiteral::UTF16:
+        ValueType = Context.Char16Ty;
+        break;
+
+      case CharacterLiteral::UTF32:
+        ValueType = Context.Char32Ty;
+        break;
+      }
+    }
+
+    if (const EnumType *ET = ValueType->getAs<EnumType>()) {
+      if (!ET->getDecl()->isComplete()) {
+        Diag(SR.getBegin(), diag::err_objc_incomplete_boxed_expression_type)
+        << ValueType << ValueExpr->getSourceRange();
+        return ExprError();
+      }
+
+      ValueType = ET->getDecl()->getIntegerType();
+    }
+
+    CheckForIntOverflow(ValueExpr);
+    // FIXME:  Do I need to do anything special with BoolTy expressions?
+
+    // Look for the appropriate method within NSNumber.
+    BoxingMethod = getNSNumberFactoryMethod(*this, SR.getBegin(), ValueType);
+    BoxedType = NSNumberPointer;
+
+  } else if (ValueType->isStructureType() || ValueType->isPointerType() || ValueType->isObjCObjectPointerType()) {
+    // Support of NSValue and NSString construction
+
+    // Check if we can construct NSString from chars
+    const PointerType *PT = ValueType->getAs<PointerType>();
+    if (PT && Context.hasSameUnqualifiedType(PT->getPointeeType(), Context.CharTy)) {
       if (!NSStringDecl) {
         IdentifierInfo *NSStringId =
           NSAPIObj->getNSClassId(NSAPI::ClassId_NSString);
@@ -486,7 +611,7 @@ ExprResult Sema::BuildObjCBoxedExpr(SourceRange SR, Expr *ValueExpr) {
         QualType NSStringObject = Context.getObjCInterfaceType(NSStringDecl);
         NSStringPointer = Context.getObjCObjectPointerType(NSStringObject);
       }
-      
+

>> Please remove these diffs

       if (!StringWithUTF8StringMethod) {
         IdentifierInfo *II = &Context.Idents.get("stringWithUTF8String");
         Selector stringWithUTF8String = Context.Selectors.getUnarySelector(II);
@@ -518,60 +643,22 @@ ExprResult Sema::BuildObjCBoxedExpr(SourceRange SR, Expr *ValueExpr) {
 
         if (!validateBoxingMethod(*this, SR.getBegin(), NSStringDecl,
                                   stringWithUTF8String, BoxingMethod))
-           return ExprError();
+          return ExprError();
 >> Please remove these diffs

         StringWithUTF8StringMethod = BoxingMethod;
       }
-      
+
>> Please remove these diffs

       BoxingMethod = StringWithUTF8StringMethod;
       BoxedType = NSStringPointer;
-    }
-  } else if (ValueType->isBuiltinType()) {
-    // The other types we support are numeric, char and BOOL/bool. We could also
-    // provide limited support for structure types, such as NSRange, NSRect, and
-    // NSSize. See NSValue (NSValueGeometryExtensions) in <Foundation/NSGeometry.h>
-    // for more details.
-
-    // Check for a top-level character literal.
-    if (const CharacterLiteral *Char =
-        dyn_cast<CharacterLiteral>(ValueExpr->IgnoreParens())) {
-      // In C, character literals have type 'int'. That's not the type we want
-      // to use to determine the Objective-c literal kind.
-      switch (Char->getKind()) {
-      case CharacterLiteral::Ascii:
-        ValueType = Context.CharTy;
-        break;
-        
-      case CharacterLiteral::Wide:
-        ValueType = Context.getWideCharType();
-        break;
-        
-      case CharacterLiteral::UTF16:
-        ValueType = Context.Char16Ty;
-        break;
-        
-      case CharacterLiteral::UTF32:
-        ValueType = Context.Char32Ty;
-        break;
-      }
-    }
-    CheckForIntOverflow(ValueExpr);
-    // FIXME:  Do I need to do anything special with BoolTy expressions?
-    
-    // Look for the appropriate method within NSNumber.
-    BoxingMethod = getNSNumberFactoryMethod(*this, SR.getBegin(), ValueType);
-    BoxedType = NSNumberPointer;
+    } else {
+      // Support for +valueWithNonretaintedObject and +valueWithPointer.
+      // Limited support for structure types, such as NSRange,
+      // NS/CG Rect, Size and Point.
 
-  } else if (const EnumType *ET = ValueType->getAs<EnumType>()) {
-    if (!ET->getDecl()->isComplete()) {
-      Diag(SR.getBegin(), diag::err_objc_incomplete_boxed_expression_type)
-        << ValueType << ValueExpr->getSourceRange();
-      return ExprError();
+      // Look for the appropriate method within NSValue.
+      BoxingMethod = getNSValueFactoryMethod(*this, SR.getBegin(), ValueType);
+      BoxedType = NSValuePointer;
     }
-
-    BoxingMethod = getNSNumberFactoryMethod(*this, SR.getBegin(),
-                                            ET->getDecl()->getIntegerType());
-    BoxedType = NSNumberPointer;
   }
 
   if (!BoxingMethod) {
diff --git a/test/CodeGenObjC/Inputs/nsvalue-boxed-expressions-support.h b/test/CodeGenObjC/Inputs/nsvalue-boxed-expressions-support.h
new file mode 100644
index 0000000..fe8a9a2
--- /dev/null
+++ b/test/CodeGenObjC/Inputs/nsvalue-boxed-expressions-support.h
@@ -0,0 +1,66 @@
+#ifndef OBJC_NSVALUE_LITERAL_SUPPORT_H
+#define OBJC_NSVALUE_LITERAL_SUPPORT_H
+
+typedef unsigned long NSUInteger;
+typedef double CGFloat;
+
+typedef struct _NSRange {
+    NSUInteger location;
+    NSUInteger length;
+} NSRange;
+
+// OS X Specific
+
+typedef struct _NSPoint {
+    CGFloat x;
+    CGFloat y;
+} NSPoint;
+
+typedef struct _NSSize {
+    CGFloat width;
+    CGFloat height;
+} NSSize;
+
+typedef struct _NSRect {
+    NSPoint origin;
+    NSSize size;
+} NSRect;
+
+// iOS Specific
+
+struct CGPoint {
+  CGFloat x;
+  CGFloat y;
+};
+typedef struct CGPoint CGPoint;
+
+struct CGSize {
+  CGFloat width;
+  CGFloat height;
+};
+typedef struct CGSize CGSize;
+
+struct CGRect {
+  CGPoint origin;
+  CGSize size;
+};
+typedef struct CGRect CGRect;
+
+ at interface NSValue
++ (NSValue *)valueWithRange:(NSRange)range;
+
+// OS X Specific
++ (NSValue *)valueWithPoint:(NSPoint)point;
++ (NSValue *)valueWithSize:(NSSize)size;
++ (NSValue *)valueWithRect:(NSRect)rect;
+

>> It is better to add macosx availability attribute here.

+// iOS Specific
++ (NSValue *)valueWithCGPoint:(CGPoint)point;
++ (NSValue *)valueWithCGSize:(CGSize)size;
++ (NSValue *)valueWithCGRect:(CGRect)rect;
+

>> It is better to add iOS availability attribute here.

++ (NSValue *)valueWithPointer:(const void *)pointer;
++ (NSValue *)valueWithNonretainedObject:(id)object;
+ at end
+
+#endif // OBJC_NSVALUE_LITERAL_SUPPORT_H
diff --git a/test/CodeGenObjC/nsvalue-boxed-expressions-arc.m b/test/CodeGenObjC/nsvalue-boxed-expressions-arc.m
new file mode 100644
index 0000000..9c30161
--- /dev/null
+++ b/test/CodeGenObjC/nsvalue-boxed-expressions-arc.m
@@ -0,0 +1,190 @@
+// RUN: %clang_cc1 -I %S/Inputs -triple x86_64-apple-darwin10 -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -O2 -disable-llvm-optzns -o - %s | FileCheck %s

>> If you are not using ‘weak’ objects no need to provide -fobjc-runtime-has-weak 

>> Need one for iOS with its own checks.

+
+#import "nsvalue-boxed-expressions-support.h"
+
+// CHECK:      [[CLASS:@.*]]        = external global %struct._class_t
+// CHECK:      [[NSVALUE:@.*]]      = {{.*}}[[CLASS]]{{.*}}
+
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithRange:{{.*}}
+// CHECK-NEXT: [[RANGE_SEL:@.*]]    = {{.*}}[[METH]]{{.*}}
+
+// OS X Specific
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithPoint:{{.*}}
+// CHECK-NEXT: [[POINT_SEL:@.*]]    = {{.*}}[[METH]]{{.*}}
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithSize:{{.*}}
+// CHECK-NEXT: [[SIZE_SEL:@.*]]     = {{.*}}[[METH]]{{.*}}
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithRect:{{.*}}
+// CHECK-NEXT: [[RECT_SEL:@.*]]     = {{.*}}[[METH]]{{.*}}

>> No. Please have two distinct runs and checks for iOS and macosx. 

+
+// iOS Specfific
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithCGPoint:{{.*}}
+// CHECK-NEXT: [[CGPOINT_SEL:@.*]]  = {{.*}}[[METH]]{{.*}}
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithCGSize:{{.*}}
+// CHECK-NEXT: [[CGSIZE_SEL:@.*]]   = {{.*}}[[METH]]{{.*}}
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithCGRect:{{.*}}
+// CHECK-NEXT: [[CGRECT_SEL:@.*]]   = {{.*}}[[METH]]{{.*}}
+

>> No. Please have two distinct runs and checks for iOS and macosx. 

+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithPointer:{{.*}}
+// CHECK-NEXT: [[POINTER_SEL:@.*]]  = {{.*}}[[METH]]{{.*}}
+
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithNonretainedObject:{{.*}}
+// CHECK-NEXT: [[NONRET_SEL:@.*]]   = {{.*}}[[METH]]{{.*}}
+
+// CHECK-LABEL: define void @doRange()
+void doRange() {
+  // CHECK:      [[RANGE:%.*]]     = bitcast %struct._NSRange* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]]  = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]       = load i8** [[RANGE_SEL]]
+  // CHECK:      [[RECV:%.*]]      = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[RANGE_PTR:%.*]] = bitcast %struct._NSRange* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]    = getelementptr {{.*}} [[RANGE_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]        = load i64* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]    = getelementptr {{.*}} [[RANGE_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]        = load i64* [[P2_PTR]], align 1
+  NSRange ns_range = { .location = 0, .length = 42 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], i64 [[P1]], i64 [[P2]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *range = @(ns_range);
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
+
+// CHECK-LABEL: define void @doPoint()
+void doPoint() {
+  // CHECK:      [[POINT:%.*]]     = bitcast %struct._NSPoint* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]]  = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]       = load i8** [[POINT_SEL]]
+  // CHECK:      [[RECV:%.*]]      = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[POINT_PTR:%.*]] = bitcast %struct._NSPoint* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]    = getelementptr {{.*}} [[POINT_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]        = load double* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]    = getelementptr {{.*}} [[POINT_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]        = load double* [[P2_PTR]], align 1
+  NSPoint ns_point = { .x = 42, .y = 24 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], double [[P1]], double [[P2]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *point = @(ns_point);
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
+
+// CHECK-LABEL: define void @doSize()
+void doSize() {
+  // CHECK:      [[SIZE:%.*]]     = bitcast %struct._NSSize* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[SIZE_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[SIZE_PTR:%.*]] = bitcast %struct._NSSize* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]   = getelementptr {{.*}} [[SIZE_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]       = load double* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]   = getelementptr {{.*}} [[SIZE_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]       = load double* [[P2_PTR]], align 1
+  NSSize ns_size = { .width = 42, .height = 24 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], double [[P1]], double [[P2]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *size = @(ns_size);
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
+
+// CHECK-LABEL: define void @doRect()
+void doRect() {
+  // CHECK:      [[RECT:%.*]]     = alloca %struct._NSRect{{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[RECT_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  NSPoint ns_point = { .x = 42, .y = 24 };
+  NSSize ns_size = { .width = 42, .height = 24 };
+  NSRect ns_rect = { .origin = ns_point, .size = ns_size };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], %struct._NSRect* byval align 8 [[RECT]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *rect = @(ns_rect);
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
+
+
+// CHECK-LABEL: define void @doCGPoint()
+void doCGPoint() {
+  // CHECK:      [[POINT:%.*]]     = bitcast %struct.CGPoint* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]]  = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]       = load i8** [[CGPOINT_SEL]]
+  // CHECK:      [[RECV:%.*]]      = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[POINT_PTR:%.*]] = bitcast %struct.CGPoint* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]    = getelementptr {{.*}} [[POINT_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]        = load double* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]    = getelementptr {{.*}} [[POINT_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]        = load double* [[P2_PTR]], align 1
+  CGPoint cg_point = { .x = 42, .y = 24 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], double [[P1]], double [[P2]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *point = @(cg_point);
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
+
+// CHECK-LABEL: define void @doCGSize()
+void doCGSize() {
+  // CHECK:      [[SIZE:%.*]]     = bitcast %struct.CGSize* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[CGSIZE_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[SIZE_PTR:%.*]] = bitcast %struct.CGSize* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]   = getelementptr {{.*}} [[SIZE_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]       = load double* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]   = getelementptr {{.*}} [[SIZE_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]       = load double* [[P2_PTR]], align 1
+  CGSize cg_size = { .width = 42, .height = 24 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], double [[P1]], double [[P2]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *size = @(cg_size);
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
+
+// CHECK-LABEL: define void @doCGRect()
+void doCGRect() {
+  // CHECK:      [[RECT:%.*]]     = alloca %struct.CGRect{{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[CGRECT_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  CGPoint cg_point = { .x = 42, .y = 24 };
+  CGSize cg_size = { .width = 42, .height = 24 };
+  CGRect cg_rect = { .origin = cg_point, .size = cg_size };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], %struct.CGRect* byval align 8 [[RECT]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *rect = @(cg_rect);
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
+
+// CHECK-LABEL: define void @doVoidPointer()
+void doVoidPointer() {
+  // CHECK:      [[POINTER:%.*]]  = alloca i8*{{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[PARAM:%.*]]    = load i8** [[POINTER]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[POINTER_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  const void *pointer = 0;
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], i8* [[PARAM]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *value = @(pointer);
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
+
+// CHECK-LABEL: define void @doNonretainedObject()
+void doNonretainedObject() {
+  // CHECK:      [[OBJ:%.*]]      = alloca i8*{{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[PARAM:%.*]]    = load i8** [[OBJ]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[NONRET_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  id obj;
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], i8* [[PARAM]])
+  // CHECK:      call i8* @objc_retainAutoreleasedReturnValue
+  NSValue *object = @(obj);
+  // CHECK:      call void @objc_release
+  // CHECK:      call void @objc_release
+  // CHECK-NEXT: ret void
+}
diff --git a/test/CodeGenObjC/nsvalue-boxed-expressions.m b/test/CodeGenObjC/nsvalue-boxed-expressions.m
new file mode 100644
index 0000000..c078fbe
--- /dev/null
+++ b/test/CodeGenObjC/nsvalue-boxed-expressions.m
@@ -0,0 +1,171 @@
+// RUN: %clang_cc1 -I %S/Inputs -triple x86_64-apple-darwin10 -emit-llvm -fblocks -fobjc-runtime-has-weak -O2 -disable-llvm-optzns -o - %s | FileCheck %s
+
+#import "nsvalue-boxed-expressions-support.h"
+
+// CHECK:      [[CLASS:@.*]]        = external global %struct._class_t
+// CHECK:      [[NSVALUE:@.*]]      = {{.*}}[[CLASS]]{{.*}}
+
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithRange:{{.*}}
+// CHECK-NEXT: [[RANGE_SEL:@.*]]    = {{.*}}[[METH]]{{.*}}
+
+// OS X Specific

>> See comment above.

+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithPoint:{{.*}}
+// CHECK-NEXT: [[POINT_SEL:@.*]]    = {{.*}}[[METH]]{{.*}}
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithSize:{{.*}}
+// CHECK-NEXT: [[SIZE_SEL:@.*]]     = {{.*}}[[METH]]{{.*}}
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithRect:{{.*}}
+// CHECK-NEXT: [[RECT_SEL:@.*]]     = {{.*}}[[METH]]{{.*}}
+
+// iOS Specfific
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithCGPoint:{{.*}}
+// CHECK-NEXT: [[CGPOINT_SEL:@.*]]  = {{.*}}[[METH]]{{.*}}
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithCGSize:{{.*}}
+// CHECK-NEXT: [[CGSIZE_SEL:@.*]]   = {{.*}}[[METH]]{{.*}}
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithCGRect:{{.*}}
+// CHECK-NEXT: [[CGRECT_SEL:@.*]]   = {{.*}}[[METH]]{{.*}}
+

>> See comment above.

+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithPointer:{{.*}}
+// CHECK-NEXT: [[POINTER_SEL:@.*]]  = {{.*}}[[METH]]{{.*}}
+
+// CHECK:      [[METH:@.*]]         = private global{{.*}}valueWithNonretainedObject:{{.*}}
+// CHECK-NEXT: [[NONRET_SEL:@.*]]   = {{.*}}[[METH]]{{.*}}
+
+// CHECK-LABEL: define void @doRange()
+void doRange() {
+  // CHECK:      [[RANGE:%.*]]     = bitcast %struct._NSRange* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]]  = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]       = load i8** [[RANGE_SEL]]
+  // CHECK:      [[RECV:%.*]]      = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[RANGE_PTR:%.*]] = bitcast %struct._NSRange* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]    = getelementptr {{.*}} [[RANGE_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]        = load i64* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]    = getelementptr {{.*}} [[RANGE_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]        = load i64* [[P2_PTR]], align 1
+  NSRange ns_range = { .location = 0, .length = 42 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], i64 [[P1]], i64 [[P2]])
+  NSValue *range = @(ns_range);
+  // CHECK:      ret void
+}
+
+// CHECK-LABEL: define void @doPoint()
+void doPoint() {
+  // CHECK:      [[POINT:%.*]]     = bitcast %struct._NSPoint* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]]  = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]       = load i8** [[POINT_SEL]]
+  // CHECK:      [[RECV:%.*]]      = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[POINT_PTR:%.*]] = bitcast %struct._NSPoint* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]    = getelementptr {{.*}} [[POINT_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]        = load double* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]    = getelementptr {{.*}} [[POINT_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]        = load double* [[P2_PTR]], align 1
+  NSPoint ns_point = { .x = 42, .y = 24 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], double [[P1]], double [[P2]])
+  NSValue *point = @(ns_point);
+  // CHECK:      ret void
+}
+
+// CHECK-LABEL: define void @doSize()
+void doSize() {
+  // CHECK:      [[SIZE:%.*]]     = bitcast %struct._NSSize* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[SIZE_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[SIZE_PTR:%.*]] = bitcast %struct._NSSize* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]   = getelementptr {{.*}} [[SIZE_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]       = load double* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]   = getelementptr {{.*}} [[SIZE_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]       = load double* [[P2_PTR]], align 1
+  NSSize ns_size = { .width = 42, .height = 24 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], double [[P1]], double [[P2]])
+  NSValue *size = @(ns_size);
+  // CHECK:      ret void
+}
+
+// CHECK-LABEL: define void @doRect()
+void doRect() {
+  // CHECK:      [[RECT:%.*]]     = alloca %struct._NSRect{{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[RECT_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  NSPoint ns_point = { .x = 42, .y = 24 };
+  NSSize ns_size = { .width = 42, .height = 24 };
+  NSRect ns_rect = { .origin = ns_point, .size = ns_size };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], %struct._NSRect* byval align 8 [[RECT]])
+  NSValue *rect = @(ns_rect);
+  // CHECK:      ret void
+}
+
+
+// CHECK-LABEL: define void @doCGPoint()
+void doCGPoint() {
+  // CHECK:      [[POINT:%.*]]     = bitcast %struct.CGPoint* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]]  = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]       = load i8** [[CGPOINT_SEL]]
+  // CHECK:      [[RECV:%.*]]      = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[POINT_PTR:%.*]] = bitcast %struct.CGPoint* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]    = getelementptr {{.*}} [[POINT_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]        = load double* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]    = getelementptr {{.*}} [[POINT_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]        = load double* [[P2_PTR]], align 1
+  CGPoint cg_point = { .x = 42, .y = 24 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], double [[P1]], double [[P2]])
+  NSValue *point = @(cg_point);
+  // CHECK:      ret void
+}
+
+// CHECK-LABEL: define void @doCGSize()
+void doCGSize() {
+  // CHECK:      [[SIZE:%.*]]     = bitcast %struct.CGSize* {{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[CGSIZE_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  // CHECK:      [[SIZE_PTR:%.*]] = bitcast %struct.CGSize* {{.*}}
+  // CHECK-NEXT: [[P1_PTR:%.*]]   = getelementptr {{.*}} [[SIZE_PTR]], i32 0, i32 0
+  // CHECK-NEXT: [[P1:%.*]]       = load double* [[P1_PTR]], align 1
+  // CHECK-NEXT: [[P2_PTR:%.*]]   = getelementptr {{.*}} [[SIZE_PTR]], i32 0, i32 1
+  // CHECK-NEXT: [[P2:%.*]]       = load double* [[P2_PTR]], align 1
+  CGSize cg_size = { .width = 42, .height = 24 };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], double [[P1]], double [[P2]])
+  NSValue *size = @(cg_size);
+  // CHECK:      ret void
+}
+
+// CHECK-LABEL: define void @doCGRect()
+void doCGRect() {
+  // CHECK:      [[RECT:%.*]]     = alloca %struct.CGRect{{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[CGRECT_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  CGPoint cg_point = { .x = 42, .y = 24 };
+  CGSize cg_size = { .width = 42, .height = 24 };
+  CGRect cg_rect = { .origin = cg_point, .size = cg_size };
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], %struct.CGRect* byval align 8 [[RECT]])
+  NSValue *rect = @(cg_rect);
+  // CHECK:      ret void
+}
+
+// CHECK-LABEL: define void @doVoidPointer()
+void doVoidPointer() {
+  // CHECK:      [[POINTER:%.*]]  = alloca i8*{{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[PARAM:%.*]]    = load i8** [[POINTER]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[POINTER_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  const void *pointer = 0;
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], i8* [[PARAM]])
+  NSValue *value = @(pointer);
+  // CHECK:      ret void
+}
+
+// CHECK-LABEL: define void @doNonretainedObject()
+void doNonretainedObject() {
+  // CHECK:      [[OBJ:%.*]]      = alloca i8*{{.*}}
+  // CHECK:      [[RECV_PTR:%.*]] = load {{.*}} [[NSVALUE]]
+  // CHECK:      [[PARAM:%.*]]    = load i8** [[OBJ]]
+  // CHECK:      [[SEL:%.*]]      = load i8** [[NONRET_SEL]]
+  // CHECK:      [[RECV:%.*]]     = bitcast %struct._class_t* [[RECV_PTR]] to i8*
+  id obj;
+  // CHECK:      call {{.*objc_msgSend.*}}(i8* [[RECV]], i8* [[SEL]], i8* [[PARAM]])
+  NSValue *object = @(obj);
+  // CHECK:      ret void
+}
diff --git a/test/SemaObjC/boxing-illegal.m b/test/SemaObjC/boxing-illegal.m

>> Sema tests should check for every erroneous situations. Like when NSValue is not available or does not have definition.
>> Or when APIs is not available for iOS or Macosx. 
>> Also need a test for __has_feature.

index 59b5c8b..a196370 100644
--- a/test/SemaObjC/boxing-illegal.m
+++ b/test/SemaObjC/boxing-illegal.m
@@ -4,6 +4,8 @@ typedef long NSInteger;
 typedef unsigned long NSUInteger;
 typedef signed char BOOL;
 
+ at interface NSValue @end
+
 @interface NSNumber
 @end
 @interface NSNumber (NSNumberCreation)
@@ -34,8 +36,8 @@ void testStruct() {
 }
 
 void testPointers() {
-    void *null = 0;
-    id boxed_null = @(null);        // expected-error {{illegal type 'void *' used in a boxed expression}}
+    float *null = 0;
+    id boxed_null = @(null);        // expected-error {{illegal type 'float *' used in a boxed expression}}
     int numbers[] = { 0, 1, 2 };
     id boxed_numbers = @(numbers);  // expected-error {{illegal type 'int *' used in a boxed expression}}
 }
diff --git a/test/SemaObjC/objc-boxed-expressions-nsvalue.m b/test/SemaObjC/objc-boxed-expressions-nsvalue.m
new file mode 100644
index 0000000..d7033d4
--- /dev/null
+++ b/test/SemaObjC/objc-boxed-expressions-nsvalue.m
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1  -fsyntax-only -fblocks -triple x86_64-apple-darwin10 -verify %s
+
+typedef struct _NSPoint {
+  int dummy;
+} NSPoint;
+
+typedef struct _NSSize {
+  int dummy;
+} NSSize;
+
+typedef struct _NSRect {
+  int dummy;
+} NSRect;
+
+typedef struct _CGPoint {
+  int dummy;
+} CGPoint;
+
+typedef struct _CGSize {
+  int dummy;
+} CGSize;
+
+typedef struct _CGRect {
+  int dummy;
+} CGRect;
+
+typedef struct _NSRange {
+  int dummy;
+} NSRange;
+
+typedef struct _SomeStruct {
+  double d;
+} SomeStruct;
+
+ at interface NSObject @end
+
+ at interface NSValue
++ (NSValue *)valueWithPoint:(NSPoint)point;
++ (NSValue *)valueWithSize:(NSSize)size;
++ (NSValue *)valueWithRect:(NSRect)rect;
+
++ (NSValue *)valueWithCGPoint:(CGPoint)point;
++ (NSValue *)valueWithCGSize:(CGSize)size;
++ (NSValue *)valueWithCGRect:(CGRect)rect;
+
++ (NSValue *)valueWithRange:(NSRange)range;
+
++ (NSValue *)valueWithPointer:(const void *)pinter;
++ (NSValue *)valueWithNonretainedObject:(id)anObject;
+ at end
+
+int main() {
+  NSPoint ns_point;
+  id ns_point_value = @(ns_point);
+
+  NSSize ns_size;
+  id ns_size_value = @(ns_size);
+
+  NSRect ns_rect;
+  id ns_rect_value = @(ns_rect);
+
+  CGPoint cg_point;
+  id cg_point_value = @(cg_point);
+
+  CGSize cg_size;
+  id cg_size_value = @(cg_size);
+
+  CGRect cg_rect;
+  id cg_rect_value = @(cg_rect);
+
+  NSRange ns_range;
+  id ns_range_value = @(ns_range);
+
+  const void *void_pointer;
+  id void_pointer_value = @(void_pointer);
+
+  id id_object;
+  id id_object_value = @(id_object);
+
+  NSObject *ns_object;
+  id ns_object_value = @(ns_object);
+
+  SomeStruct s;
+  id err = @(s); // expected-error{{illegal type 'SomeStruct' (aka 'struct _SomeStruct') used in a boxed expression}}
+}
diff --git a/test/SemaObjCXX/boxing-illegal-types.mm b/test/SemaObjCXX/boxing-illegal-types.mm
index 7729753..70e30c5 100644
--- a/test/SemaObjCXX/boxing-illegal-types.mm
+++ b/test/SemaObjCXX/boxing-illegal-types.mm
@@ -4,6 +4,8 @@ typedef long NSInteger;
 typedef unsigned long NSUInteger;
 typedef signed char BOOL;
 
+ at interface NSValue @end
+
 @interface NSNumber
 @end
 @interface NSNumber (NSNumberCreation)
@@ -34,8 +36,8 @@ void testStruct() {
 }
 
 void testPointers() {
-    void *null = 0;
-    id boxed_null = @(null);        // expected-error {{illegal type 'void *' used in a boxed expression}}
+    float *null = 0;
+    id boxed_null = @(null);        // expected-error {{illegal type 'float *' used in a boxed expression}}
     int numbers[] = { 0, 1, 2 };
     id boxed_numbers = @(numbers);  // expected-error {{illegal type 'int *' used in a boxed expression}}
 }

> On Dec 7, 2014, at 9:24 AM, AlexDenisov <1101.debian at gmail.com> wrote:
> 
> No worries, that’s fine. At least I have time to improve the patch.
> Here is the new version, final one, I’d say.
> I renamed the patch to reflect what it really does.
> 
> In this version:
> 
>  - support for NSRange
>  - support for NS/CG Rect, Size, Point
>  - support for Nonretained Object (+ valueWithNonretainedObject:(id)obj )
>  - support for Pointer (+ valueWithPointer:(const void *)pointer )
>  - CodeGenObjC tests, both ARC and non-ARC
>  - SemaObjC tests
>  - cleanups and fixes based on previous comments
> 
> P.S. Previously I mentioned support for structures based on method name (valueWith<StructName>), I don’t know is it a good idea or not and still expect to get any feedback, if it’s possible of course. Otherwise, I would stick to current implementation.
> -- 
> AlexDenisov
> Software Engineer, https://github.com/AlexDenisov <https://github.com/AlexDenisov><nsvalue_boxed_expressions.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141208/8534f442/attachment.html>


More information about the cfe-commits mailing list