Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[ahatanak]

Summary

clang currently emits an expression like @("abc") as a message send to stringWithUTF8String. This patch makes changes so that a compile-time constant is emitted instead when the expression inside the parentheses is a string literal. I think it's possible to do the same optimization for constant expressions that evaluate to zero-terminated strings (see the example below), but I'm leaving that for future work.

constexpr const char *foo() { return "abc"; }

void test() {
  NSString *s = @(foo());
}

The original motivation for the patch is to silence the -Wnullable-to-nonnull-conversion warning that clang started emitting after r317727:
The https://oleb.net/2018/@keypath/.

rdar://problem/42684601

[rjmccall]

lib/CodeGen/CGExprConstant.cpp

1793

IgnoreParens.

lib/Sema/SemaExprObjC.cpp

534

You're implicitly dropping sugar from the source expression here; I really think you should preserve that, and if it means you end up having to look through array-decay expressions, that's not the end of the world.

The need for a special case here is just to allow us to compile these even if there isn't a boxing method available?

Do you need to restrict the type of the string literal so that it doesn't apply to e.g. wide strings?

[ahatanak]

lib/Sema/SemaExprObjC.cpp

534

Line 516 checks that the pointee type has the same unqualified type as 'char'. If the string literal inside the parentheses is of a wider type (e.g., @(u"abc")), this piece of code won't be executed, and a diagnostic is emitted later ("illegal type 'const char16_t *' used in a boxed expression").

The original motivation for special-casing string literals inside boxed expressions was to silence the -Wnullable-to-nonnull-conversion warning mentioned here: https://oleb.net/2018/@keypath/. The warning is issued because the return type of stringWithUTF8String is nullable.

...
+ (nullable instancetype)stringWithUTF8String:(const char *)nullTerminatedCString;
...

NSString * _Nonnull ptr = @("abc");  // expected-error {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}

[rjmccall]

lib/Sema/SemaExprObjC.cpp

534

I see. So in addition to guaranteeing to skip the boxing call, you'd like to adjust the type to report the result as non-null, which we can do because we're unconditionally making a constant string. Makes sense.

However, I think you need to check that the string is valid UTF-8 or else this is semantics-changing, since such strings are rejected by +stringWithUTF8String: (which is why it returns a nullable pointer at all). It should be alright to warn about strings that are statically known to be invalid UTF-8, but you'll then need to emit them using the normal boxing method.

[ahatanak]

Address review comments.

• Preserve sugar when creating an ObjCBoxedExpr in Sema::BuildObjCBoxedExpr.
• Add a test case to test/SemaObjC/boxing-illegal.m that shows clang rejects a string literal in a boxed expression if its character encoding is incompatible with UTF-8.

lib/Sema/SemaExprObjC.cpp

534

clang already rejects boxed expressions that are string literals with a character encoding incompatible with UTF-8. Also, we reach this part of the code only if the string is UTF-8 or ASCII (see the assertion I added). I added a test case to test/SemaObjC/boxing-illegal.m that shows clang rejects strings incompatible with UTF-8.

[rjmccall]

test/SemaObjC/boxing-illegal.m

70 ↗

(On Diff #188794)

I don't know what \p is supposed to be or why it apparently changes the type of the literal to unsigned char *, but none of these are ordinary string literals that are invalid as UTF-8. I mean something like "\xFF", which still has type char * but will fail to parse as UTF-8, which will cause normal boxing to fail and return nil.

[ahatanak]

If the string literal used for the boxed expression isn't a valid UTF-8 string, don't emit a compile-time constant.

[ahatanak]

test/SemaObjC/boxing-illegal.m

70 ↗

(On Diff #188794)

Fixed and added test cases. Boxed expressions now have to be valid UTF-8 string literals in order to be emitted as compile-time constants.

If the string literal in a boxed expression is an invalid UTF-8 string, should we reject it the same way we reject other kinds of string literals (e.g., UTF-16)?

[ahatanak]

test/SemaObjC/boxing-illegal.m

70 ↗

(On Diff #188794)

Or at least warn about it.

[rjmccall]

test/SemaObjC/boxing-illegal.m

70 ↗

(On Diff #188794)

Thanks, fix looks good. I think a warning would be extremely reasonable.

[ahatanak]

Diagnose invalid UTF-8 strings in boxed expressions.

[rjmccall]

include/clang/Basic/DiagnosticGroups.td

322 ↗

(On Diff #189630)

I think this warning group should mention that this is specific to ObjC boxing somehow.

include/clang/Basic/DiagnosticSemaKinds.td

5807 ↗

(On Diff #189630)

Maybe "string is ill-formed as UTF-8 and will become a null NSString* when boxed", where the type is whatever the type of the box-expression would be?

[ahatanak]

Improve diagnostic message.

[rjmccall]

include/clang/Basic/DiagnosticGroups.td

407 ↗

(On Diff #189657)

Sure. If we decide to add more things to this warning group, we can pull this one out into a sub-group with a more targeted name.

include/clang/Basic/DiagnosticSemaKinds.td

922 ↗

(On Diff #189657)

Might as well use the NSStringPointer type we stash on Sema so that this will be the right type even in more obscure environments.

[ahatanak]

Use the type of NSStringPointer in the diagnostic string.

[ahatanak]

include/clang/Basic/DiagnosticSemaKinds.td

922 ↗

(On Diff #189657)

I wasn't exactly sure when NSStringPointer can be anything other than NSString *.

[rjmccall]

include/clang/Basic/DiagnosticSemaKinds.td

922 ↗

(On Diff #189657)

Hmm. It's possible that it can't be for boxing, but I'm pretty sure it can be overridden for string literals, and it's not ridiculous that we'd allow that to be changed for arbitrary boxing as well, so this is good idea regardless.

[ahatanak]

include/clang/Basic/DiagnosticSemaKinds.td

922 ↗

(On Diff #189657)

I agree, it's probably a good idea.

include/clang/AST/ExprObjC.h

public:
 
   Expr *getSubExpr() { return cast<Expr>(SubExpr); }
   const Expr *getSubExpr() const { return cast<Expr>(SubExpr); }
 
   ObjCMethodDecl *getBoxingMethod() const {
     return BoxingMethod;
   }
 
+  // Indicates whether this boxed expression can be emitted as a compile-time
+  // constant.
+  bool isExpressibleAsConstantInitializer() const {
+    return !BoxingMethod && SubExpr;
+  }
+
   SourceLocation getAtLoc() const { return Range.getBegin(); }
 
   SourceLocation getBeginLoc() const LLVM_READONLY { return Range.getBegin(); }
   SourceLocation getEndLoc() const LLVM_READONLY { return Range.getEnd(); }
 
   SourceRange getSourceRange() const LLVM_READONLY {
     return Range;
   }

lib/AST/ExprConstant.cpp

static bool IsGlobalLValue(APValue::LValueBase B) {
   // A string literal has static storage duration.
   case Expr::StringLiteralClass:
   case Expr::PredefinedExprClass:
   case Expr::ObjCStringLiteralClass:
   case Expr::ObjCEncodeExprClass:
   case Expr::CXXTypeidExprClass:
   case Expr::CXXUuidofExprClass:
     return true;
+  case Expr::ObjCBoxedExprClass:
+    return cast<ObjCBoxedExpr>(E)->isExpressibleAsConstantInitializer();
   case Expr::CallExprClass:
     return IsStringLiteralCall(cast<CallExpr>(E));
   // For GCC compatibility, &&label has static storage duration.
   case Expr::AddrLabelExprClass:
     return true;
   // A Block literal expression may be used as the initialization value for
   // Block variables at global or local static scope.
   case Expr::BlockExprClass:
public:
   }
 
   bool VisitBinaryOperator(const BinaryOperator *E);
   bool VisitCastExpr(const CastExpr* E);
   bool VisitUnaryAddrOf(const UnaryOperator *E);
   bool VisitObjCStringLiteral(const ObjCStringLiteral *E)
       { return Success(E); }
   bool VisitObjCBoxedExpr(const ObjCBoxedExpr *E) {
+    if (E->isExpressibleAsConstantInitializer())
+      return Success(E);
     if (Info.noteFailure())
       EvaluateIgnoredValue(Info, E->getSubExpr());
     return Error(E);
   }
   bool VisitAddrLabelExpr(const AddrLabelExpr *E)
       { return Success(E); }
   bool VisitCallExpr(const CallExpr *E);
   bool VisitBuiltinCallExpr(const CallExpr *E, unsigned BuiltinOp);

lib/CodeGen/CGExprConstant.cpp

 private:
   llvm::Constant *tryEmitAbsolute(llvm::Type *destTy);
   ConstantLValue tryEmitBase(const APValue::LValueBase &base);
 
   ConstantLValue VisitStmt(const Stmt *S) { return nullptr; }
   ConstantLValue VisitConstantExpr(const ConstantExpr *E);
   ConstantLValue VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   ConstantLValue VisitStringLiteral(const StringLiteral *E);
+  ConstantLValue VisitObjCBoxedExpr(const ObjCBoxedExpr *E);
   ConstantLValue VisitObjCEncodeExpr(const ObjCEncodeExpr *E);
   ConstantLValue VisitObjCStringLiteral(const ObjCStringLiteral *E);
   ConstantLValue VisitPredefinedExpr(const PredefinedExpr *E);
   ConstantLValue VisitAddrLabelExpr(const AddrLabelExpr *E);
   ConstantLValue VisitCallExpr(const CallExpr *E);
   ConstantLValue VisitBlockExpr(const BlockExpr *E);
   ConstantLValue VisitCXXTypeidExpr(const CXXTypeidExpr *E);
   ConstantLValue VisitCXXUuidofExpr(const CXXUuidofExpr *E);
ConstantLValueEmitter::VisitStringLiteral(const StringLiteral *E) {
   return CGM.GetAddrOfConstantStringFromLiteral(E);
 }
 
 ConstantLValue
 ConstantLValueEmitter::VisitObjCEncodeExpr(const ObjCEncodeExpr *E) {
   return CGM.GetAddrOfConstantStringFromObjCEncode(E);
 }
 
+static ConstantLValue emitConstantObjCStringLiteral(const StringLiteral *S,
+                                                    QualType T,
+                                                    CodeGenModule &CGM) {
+  auto C = CGM.getObjCRuntime().GenerateConstantString(S);
+  return C.getElementBitCast(CGM.getTypes().ConvertTypeForMem(T));
+}
+
 ConstantLValue
 ConstantLValueEmitter::VisitObjCStringLiteral(const ObjCStringLiteral *E) {
-  auto C = CGM.getObjCRuntime().GenerateConstantString(E->getString());
-  return C.getElementBitCast(CGM.getTypes().ConvertTypeForMem(E->getType()));
+  return emitConstantObjCStringLiteral(E->getString(), E->getType(), CGM);
+}
+
+ConstantLValue
+ConstantLValueEmitter::VisitObjCBoxedExpr(const ObjCBoxedExpr *E) {
+  assert(E->isExpressibleAsConstantInitializer() &&
+         "this boxed expression can't be emitted as a compile-time constant");
+  return emitConstantObjCStringLiteral(cast<StringLiteral>(E->getSubExpr()),
+                                       E->getType(), CGM);
 }
 
 ConstantLValue
 ConstantLValueEmitter::VisitPredefinedExpr(const PredefinedExpr *E) {
   return CGM.GetAddrOfConstantStringFromLiteral(E->getFunctionName());
 }
 
 ConstantLValue

lib/CodeGen/CGObjC.cpp

 //===---- CGObjC.cpp - Emit LLVM Code for Objective-C ---------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This contains code to emit Objective-C code as LLVM code.
 //
 //===----------------------------------------------------------------------===//
 
 #include "CGDebugInfo.h"
 #include "CGObjCRuntime.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
+#include "ConstantEmitter.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/DataLayout.h"

 /// or [NSValue valueWithBytes:objCType:].
 ///
 llvm::Value *
 CodeGenFunction::EmitObjCBoxedExpr(const ObjCBoxedExpr *E) {
   // Generate the correct selector for this literal's concrete type.
   // Get the method.
   const ObjCMethodDecl *BoxingMethod = E->getBoxingMethod();
   const Expr *SubExpr = E->getSubExpr();
-  assert(BoxingMethod && "BoxingMethod is null");
+
+  if (E->isExpressibleAsConstantInitializer()) {
+    ConstantEmitter ConstEmitter(CGM);
+    return ConstEmitter.tryEmitAbstract(E, E->getType());
+  }
+
   assert(BoxingMethod->isClassMethod() && "BoxingMethod must be a class method");
   Selector Sel = BoxingMethod->getSelector();
 
   // Generate a reference to the class pointer, which will be the receiver.
   // Assumes that the method was introduced in the class that should be
   // messaged (avoids pulling it out of the result type).
   CGObjCRuntime &Runtime = CGM.getObjCRuntime();
   const ObjCInterfaceDecl *ClassDecl = BoxingMethod->getClassInterface();

lib/Sema/SemaExprObjC.cpp

 ExprResult Sema::BuildObjCBoxedExpr(SourceRange SR, Expr *ValueExpr) {
   if (ValueExpr->isTypeDependent()) {
     ObjCBoxedExpr *BoxedExpr =
       new (Context) ObjCBoxedExpr(ValueExpr, Context.DependentTy, nullptr, SR);
     return BoxedExpr;
   }
   ObjCMethodDecl *BoxingMethod = nullptr;
   QualType BoxedType;
+  Expr *ValueSubExpr = ValueExpr->IgnoreParens();
   // Convert the expression to an RValue, so we can check for pointer types...
   ExprResult RValue = DefaultFunctionArrayLvalueConversion(ValueExpr);
   if (RValue.isInvalid()) {
     return ExprError();
   }
   SourceLocation Loc = SR.getBegin();
   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 (!NSStringDecl) {
         NSStringDecl = LookupObjCInterfaceDeclForLiteral(*this, Loc,
                                                          Sema::LK_String);
         if (!NSStringDecl) {
           return ExprError();
         }
         QualType NSStringObject = Context.getObjCInterfaceType(NSStringDecl);
         NSStringPointer = Context.getObjCObjectPointerType(NSStringObject);
       }
 
+      // The boxed expression can be emitted as a compile time constant if this
+      // is a string literal.
+      if (auto *SL = dyn_cast<StringLiteral>(ValueSubExpr)) {
+        BoxedType = Context.getAttributedType(
+            AttributedType::getNullabilityAttrKind(NullabilityKind::NonNull),
+            NSStringPointer, NSStringPointer);
+        return new (Context) ObjCBoxedExpr(SL, BoxedType, nullptr, SR);
+      }
+
       if (!StringWithUTF8StringMethod) {
         IdentifierInfo *II = &Context.Idents.get("stringWithUTF8String");
         Selector stringWithUTF8String = Context.Selectors.getUnarySelector(II);
 
         // Look for the appropriate method within NSString.
         BoxingMethod = NSStringDecl->lookupClassMethod(stringWithUTF8String);
         if (!BoxingMethod && getLangOpts().DebuggerObjCLiteral) {
           // Debugger needs to work even if NSString hasn't been defined.

test/CodeGenObjC/boxing.m

 
 @interface NSString : NSObject
 @end
 
 @interface NSString (NSStringExtensionMethods)
 + (id)stringWithUTF8String:(const char *)nullTerminatedCString;
 @end
 
+// CHECK: [[V0:%.*]] = type opaque
+// CHECK: [[STRUCT_NSCONSTANT_STRING_TAG:%.*]] = type { i32*, i32, i8*, i64 }
+
 // CHECK: [[WithIntMeth:@.*]] = private unnamed_addr constant [15 x i8] c"numberWithInt:\00"
 // CHECK: [[WithIntSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds ([15 x i8], [15 x i8]* [[WithIntMeth]]
 // CHECK: [[WithCharMeth:@.*]] = private unnamed_addr constant [16 x i8] c"numberWithChar:\00"
 // CHECK: [[WithCharSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds ([16 x i8], [16 x i8]* [[WithCharMeth]]
 // CHECK: [[WithBoolMeth:@.*]] = private unnamed_addr constant [16 x i8] c"numberWithBool:\00"
 // CHECK: [[WithBoolSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds ([16 x i8], [16 x i8]* [[WithBoolMeth]]
 // CHECK: [[WithIntegerMeth:@.*]] = private unnamed_addr constant [19 x i8] c"numberWithInteger:\00"
 // CHECK: [[WithIntegerSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds ([19 x i8], [19 x i8]* [[WithIntegerMeth]]
 // CHECK: [[WithUnsignedIntegerMeth:@.*]] = private unnamed_addr constant [27 x i8] c"numberWithUnsignedInteger:\00"
 // CHECK: [[WithUnsignedIntegerSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds ([27 x i8], [27 x i8]* [[WithUnsignedIntegerMeth]]
 // CHECK: [[stringWithUTF8StringMeth:@.*]] = private unnamed_addr constant [22 x i8] c"stringWithUTF8String:\00"
 // CHECK: [[stringWithUTF8StringSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds ([22 x i8], [22 x i8]* [[stringWithUTF8StringMeth]]
+// CHECK: [[STR0:.*]] = private unnamed_addr constant [4 x i8] c"abc\00", section "__TEXT,__cstring,cstring_literals", align 1
+// CHECK: [[UNNAMED_CFSTRING:.*]] = private global [[STRUCT_NSCONSTANT_STRING_TAG]] { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 1992, i8* getelementptr inbounds ([4 x i8], [4 x i8]* [[STR0]], i32 0, i32 0), i64 3 }, section "__DATA,__cfstring", align 8
 
 int main() {
+  // CHECK: [[T:%.*]] = alloca [[V0]]*, align 8
+
   // CHECK: load i8*, i8** [[WithIntSEL]]
   int i; @(i);
   // CHECK: load i8*, i8** [[WithCharSEL]]
   signed char sc; @(sc);
   // CHECK: load i8*, i8** [[WithBoolSEL]]
   BOOL b; @(b);
   // CHECK: load i8*, i8** [[WithBoolSEL]]
   typeof(b) b2; @(b2);
int main() {
   const char *s; @(s);
 
   typedef enum : NSInteger { Red, Green, Blue } Color;
   // CHECK: load i8*, i8** [[WithIntegerSEL]]
   @(Red);
   Color col = Red;
   // CHECK: load i8*, i8** [[WithIntegerSEL]]
   @(col);
+
+  // CHECK: store [[V0]]* bitcast ([[STRUCT_NSCONSTANT_STRING_TAG]]* [[UNNAMED_CFSTRING]] to [[V0]]*), [[V0]]** [[T]], align 8
+  NSString *t = @("abc");
 }

test/SemaObjC/objc-literal-sig.m

 @interface NSDictionary
 + (id)dictionaryWithObjects:(const id [])objects
                     forKeys:(const int [])keys // expected-note 2 {{second parameter has unexpected type 'const int *' (should be 'const id *')}}
                       count:(unsigned long)cnt;
 @end
 
 // All tests are doubled to make sure that a bad method is not saved
 // and then used un-checked.
+const char *getStr(void);
+
 void test_sig() {
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   id array = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id array2 = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id dict = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
   id dict2 = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
-  id str = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
-  id str2 = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str2 = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
 }

test/SemaObjC/transfer-boxed-string-nullability.m

 #endif
 ) stringWithUTF8String:(const char*)x;
 
 @end
 
 void takesNonNull(NSString * _Nonnull ptr);
 
 void testBoxedString() {
+  // No diagnostic emitted as this doesn't need a stringWithUTF8String message
+  // send.
+  takesNonNull(@("hey"));
+
   const char *str = "hey";
   takesNonNull([NSString stringWithUTF8String:str]);
   takesNonNull(@(str));
 #ifndef NOWARN
   // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
   // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
 #else
   // expected-no-diagnostics
 #endif
 }

test/SemaObjCXX/literals.mm

 @interface NSArray (NSArrayCreation)
 + (id)arrayWithObjects:(const id [])objects count:(unsigned long)cnt;
 @end
 
 @interface NSDictionary
 + (id)dictionaryWithObjects:(const id [])objects forKeys:(const id<NSCopying> [])keys count:(unsigned long)cnt;
 @end
 
+@interface NSString
+@end
+
 template<typename T>
 struct ConvertibleTo {
   operator T();
 };
 
 template<typename T>
 struct ExplicitlyConvertibleTo {
   explicit operator T();

 };
 
 id key;
 id value;
 
 void test_dictionary_colon() {
   id dict = @{ key : value };
 }
+
+void testConstExpr() {
+  constexpr NSString *t0 = @"abc";
+  constexpr NSString *t1 = @("abc");
+}