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