tahonermann added inline comments.

================
Comment at: clang/lib/AST/ExprConstant.cpp:12951-12954
+      // ObjC's @encode()
+      if (isa<ObjCEncodeExpr>(E->getLHS()->IgnoreParenImpCasts()) ||
+          isa<ObjCEncodeExpr>(E->getRHS()->IgnoreParenImpCasts()))
         return Error(E);
----------------
tbaeder wrote:
> tahonermann wrote:
> > tbaeder wrote:
> > > tahonermann wrote:
> > > > A comment to explain this change would be helpful. It isn't intuitive 
> > > > (for me anyway) why Objective-C's `@encode` would require special 
> > > > handling here. Was this needed to avoid a test failure?
> > > In `../clang/test/CodeGenObjC/encode-test-4.m`:
> > > 
> > > ```
> > > int a(void) {
> > >   return @encode(int) == @encode(int) &&
> > >     @encode(Color) == @encode(long) &&
> > >     @encode(PrintColor) == @encode(int);
> > > }
> > > ```
> > > 
> > > The comparisons need to be rejected here and are folded to a `1` later 
> > > on, it seems. Letting the comparison happen will lead to a `0`.
> > Apologies, I meant it would be helpful to add a comment to the code :)
> > 
> > I think `@encode(...)` expressions should be treated the same as string 
> > literals; the former is lowered to the latter; both contribute to the 
> > string table.
> You mean the special case for them here should go away?
> 
> I've tracked this down to `CodeGenFunction::ConstantFoldsToSimpleInteger()`. 
> Without this special case for endcode expressions, we will return `true` here 
> and fold the comparison to `false`, so we will just emit that. 
> 
> This is a funny case though, which also exists in C++. With this patch, `"a" 
> == "a"` evaluates to `true`, as one would expect.
> However, `"a" == "a" && "b" == "b"` will evaluate to `false`.
> 
> That's pretty wrong so I think the patch needs more work.
Should `"a" == "a"` evaluate to `true`? The standard seems clear that the 
behavior is unspecified ([[ http://eel.is/c++draft/lex.string#9 | 
(lex.string)p9 ]]). Is there a special case for constexpr context somewhere? I 
see that gcc, icc, and MSVC all compare them equally in constexpr context, but 
that doesn't mean that Clang needs to.

Gcc appears to treat Objective-C `@encode()` expressions equivalently to string 
literals in constexpr context. https://godbolt.org/z/Kxz3E8xKa


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137826

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

Reply via email to