[clang] 85cca94 - [SemaObjC] Forbid storing an unboxed integer literal in an NSNumber

Erik Pilkington via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 20 12:23:26 PDT 2020


Author: Erik Pilkington
Date: 2020-04-20T15:22:51-04:00
New Revision: 85cca945b4c93ecfd3f3bb7a7b2d5fa4104c3560

URL: https://github.com/llvm/llvm-project/commit/85cca945b4c93ecfd3f3bb7a7b2d5fa4104c3560
DIFF: https://github.com/llvm/llvm-project/commit/85cca945b4c93ecfd3f3bb7a7b2d5fa4104c3560.diff

LOG: [SemaObjC] Forbid storing an unboxed integer literal in an NSNumber

This fixes a common mistake (the 3 should be @3): NSNumber *n = 3. This extends
an existing check for NSString. Also, this only errs if the initializer isn't a
null pointer constant, so NSNumber *n = 0; continues to work. rdar://47029572

Differential revision: https://reviews.llvm.org/D78066

Added: 
    clang/test/SemaObjC/objc-literal-fixit.m

Modified: 
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/include/clang/Sema/Sema.h
    clang/lib/Sema/SemaExpr.cpp
    clang/lib/Sema/SemaExprObjC.cpp
    clang/lib/Sema/SemaInit.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a64e313bf271..014ee1c2f2d7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2945,7 +2945,7 @@ def warn_objc_literal_comparison : Warning<
   "a numeric literal|a boxed expression|}0 has undefined behavior">,
   InGroup<ObjCLiteralComparison>;
 def err_missing_atsign_prefix : Error<
-  "string literal must be prefixed by '@' ">;
+  "%select{string|numeric}0 literal must be prefixed by '@'">;
 def warn_objc_string_literal_comparison : Warning<
   "direct comparison of a string literal has undefined behavior">,
   InGroup<ObjCStringComparison>;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index af58b0ec4e82..5cd75b176761 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9455,8 +9455,8 @@ class Sema final {
                                          QualType DestType, QualType SrcType,
                                          Expr *&SrcExpr, bool Diagnose = true);
 
-  bool ConversionToObjCStringLiteralCheck(QualType DstType, Expr *&SrcExpr,
-                                          bool Diagnose = true);
+  bool CheckConversionToObjCLiteral(QualType DstType, Expr *&SrcExpr,
+                                    bool Diagnose = true);
 
   bool checkInitMethod(ObjCMethodDecl *method, QualType receiverTypeIfCall);
 

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fbb5d4b05bbf..75d80c0cc45c 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9283,7 +9283,7 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
     if (getLangOpts().ObjC &&
         (CheckObjCBridgeRelatedConversions(E->getBeginLoc(), LHSType,
                                            E->getType(), E, Diagnose) ||
-         ConversionToObjCStringLiteralCheck(LHSType, E, Diagnose))) {
+         CheckConversionToObjCLiteral(LHSType, E, Diagnose))) {
       if (!Diagnose)
         return Incompatible;
       // Replace the expression with a corrected version and continue so we
@@ -15228,21 +15228,15 @@ ExprResult Sema::BuildSourceLocExpr(SourceLocExpr::IdentKind Kind,
       SourceLocExpr(Context, Kind, BuiltinLoc, RPLoc, ParentContext);
 }
 
-bool Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *&Exp,
-                                              bool Diagnose) {
+bool Sema::CheckConversionToObjCLiteral(QualType DstType, Expr *&Exp,
+                                        bool Diagnose) {
   if (!getLangOpts().ObjC)
     return false;
 
   const ObjCObjectPointerType *PT = DstType->getAs<ObjCObjectPointerType>();
   if (!PT)
     return false;
-
-  if (!PT->isObjCIdType()) {
-    // Check if the destination is the 'NSString' interface.
-    const ObjCInterfaceDecl *ID = PT->getInterfaceDecl();
-    if (!ID || !ID->getIdentifier()->isStr("NSString"))
-      return false;
-  }
+  const ObjCInterfaceDecl *ID = PT->getInterfaceDecl();
 
   // Ignore any parens, implicit casts (should only be
   // array-to-pointer decays), and not-so-opaque values.  The last is
@@ -15252,15 +15246,41 @@ bool Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *&Exp,
     if (OV->getSourceExpr())
       SrcExpr = OV->getSourceExpr()->IgnoreParenImpCasts();
 
-  StringLiteral *SL = dyn_cast<StringLiteral>(SrcExpr);
-  if (!SL || !SL->isAscii())
-    return false;
-  if (Diagnose) {
-    Diag(SL->getBeginLoc(), diag::err_missing_atsign_prefix)
-        << FixItHint::CreateInsertion(SL->getBeginLoc(), "@");
-    Exp = BuildObjCStringLiteral(SL->getBeginLoc(), SL).get();
+  if (auto *SL = dyn_cast<StringLiteral>(SrcExpr)) {
+    if (!PT->isObjCIdType() &&
+        !(ID && ID->getIdentifier()->isStr("NSString")))
+      return false;
+    if (!SL->isAscii())
+      return false;
+
+    if (Diagnose) {
+      Diag(SL->getBeginLoc(), diag::err_missing_atsign_prefix)
+          << /*string*/0 << FixItHint::CreateInsertion(SL->getBeginLoc(), "@");
+      Exp = BuildObjCStringLiteral(SL->getBeginLoc(), SL).get();
+    }
+    return true;
   }
-  return true;
+
+  if ((isa<IntegerLiteral>(SrcExpr) || isa<CharacterLiteral>(SrcExpr) ||
+      isa<FloatingLiteral>(SrcExpr) || isa<ObjCBoolLiteralExpr>(SrcExpr) ||
+      isa<CXXBoolLiteralExpr>(SrcExpr)) &&
+      !SrcExpr->isNullPointerConstant(
+          getASTContext(), Expr::NPC_NeverValueDependent)) {
+    if (!ID || !ID->getIdentifier()->isStr("NSNumber"))
+      return false;
+    if (Diagnose) {
+      Diag(SrcExpr->getBeginLoc(), diag::err_missing_atsign_prefix)
+          << /*number*/1
+          << FixItHint::CreateInsertion(SrcExpr->getBeginLoc(), "@");
+      Expr *NumLit =
+          BuildObjCNumericLiteral(SrcExpr->getBeginLoc(), SrcExpr).get();
+      if (NumLit)
+        Exp = NumLit;
+    }
+    return true;
+  }
+
+  return false;
 }
 
 static bool maybeDiagnoseAssignmentToFunction(Sema &S, QualType DstType,

diff  --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp
index 44b518f4c0a2..a9153322bda5 100644
--- a/clang/lib/Sema/SemaExprObjC.cpp
+++ b/clang/lib/Sema/SemaExprObjC.cpp
@@ -4363,7 +4363,7 @@ Sema::CheckObjCConversion(SourceRange castRange, QualType castType,
   // to 'NSString *', instead of falling through to report a "bridge cast"
   // diagnostic.
   if (castACTC == ACTC_retainable && exprACTC == ACTC_none &&
-      ConversionToObjCStringLiteralCheck(castType, castExpr, Diagnose))
+      CheckConversionToObjCLiteral(castType, castExpr, Diagnose))
     return ACR_error;
 
   // Do not issue "bridge cast" diagnostic when implicit casting

diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index cefedc63c341..314b3772f5f9 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -5625,7 +5625,7 @@ void InitializationSequence::InitializeFrom(Sema &S,
       if (S.CheckObjCBridgeRelatedConversions(Initializer->getBeginLoc(),
                                               DestType, Initializer->getType(),
                                               Initializer) ||
-          S.ConversionToObjCStringLiteralCheck(DestType, Initializer))
+          S.CheckConversionToObjCLiteral(DestType, Initializer))
         Args[0] = Initializer;
     }
     if (!isa<InitListExpr>(Initializer))

diff  --git a/clang/test/SemaObjC/objc-literal-fixit.m b/clang/test/SemaObjC/objc-literal-fixit.m
new file mode 100644
index 000000000000..fa117a342745
--- /dev/null
+++ b/clang/test/SemaObjC/objc-literal-fixit.m
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s                 -verify=c,expected
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s -xobjective-c++ -verify=cxx,expected
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s -fobjc-arc      -verify=c,arc,expected
+
+typedef signed char BOOL;
+#define YES __objc_yes
+#define NO __objc_no
+
+ at interface NSNumber
++(instancetype)numberWithChar:(char)value;
++(instancetype)numberWithInt:(int)value;
++(instancetype)numberWithDouble:(double)value;
++(instancetype)numberWithBool:(BOOL)value;
+ at end
+
+void test() {
+  NSNumber *n = YES; // expected-error{{numeric literal must be prefixed by '@'}}
+  NSNumber *n1 = 1; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  NSNumber *n2 = NO; // c-warning{{expression which evaluates to zero treated as a null pointer constant}}
+                     // cxx-error at -1{{numeric literal must be prefixed by '@'}}
+  NSNumber *n3 = 0;
+  NSNumber *n4 = 0.0; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  NSNumber *n5 = '\0'; // c-warning{{expression which evaluates to zero treated as a null pointer constant}}
+                       // cxx-error at -1{{numeric literal must be prefixed by '@'}}
+
+
+  NSNumber *n6 = '1'; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  int i;
+  NSNumber *n7 = i; // c-warning{{incompatible integer to pointer conversion}}
+                    // arc-error at -1{{implicit conversion of 'int' to 'NSNumber *' is disallowed with ARC}}
+                    // cxx-error at -2{{cannot initialize a variable of type 'NSNumber *' with an lvalue of type 'int'}}
+
+  id n8 = 1; // c-warning{{incompatible integer to pointer conversion}}
+             // arc-error at -1{{implicit conversion of 'int' to 'id' is disallowed with ARC}}
+             // cxx-error at -2{{cannot initialize a variable of type 'id' with an rvalue of type 'int'}}
+}


        


More information about the cfe-commits mailing list