rjmccall added inline comments.

================
Comment at: lib/Sema/SemaExprObjC.cpp:534
+            NSStringPointer, NSStringPointer);
+        return new (Context) ObjCBoxedExpr(SL, BoxedType, nullptr, SR);
+      }
----------------
ahatanak wrote:
> rjmccall wrote:
> > 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?
> 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'}}
> ```
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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

Reply via email to