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