[PATCH] D123680: Add support for ignored bitfield conditional codegen.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/CodeGen/CGExpr.cpp:200
+          E->IgnoreParenNoopCasts(getContext()))) {
+    if (CondOp->getObjectKind() == OK_BitField)
+      return EmitIgnoredConditionalOperator(CondOp);
----------------
efriedma wrote:
> Is there some reason we need to special-case bitfields here?
I don't think it ends up being a problem to support the 'simple' case (or perhaps even the matrix/vector cases?), but I was not really motivated to write test cases for all of those cases (nor write error handling, since that is already so well handled).



================
Comment at: clang/lib/CodeGen/CGExpr.cpp:4635
+  eval.begin(CGF);
+  Info.LHS = EmitLValueOrThrowExpression(CGF, E->getTrueExpr());
+  eval.end(CGF);
----------------
efriedma wrote:
> If we're calling this from EmitIgnoredConditionalOperator, should we be calling back into EmitIgnoredExpr, instead of using EmitLValueOrThrowExpression?  Consider, e.g. `cond ? s1.field1 = val1 : cond2 ? s1.field2 = val2 : s1.field3 = val3;`.
First, this is the 'generic function' that is called by the normal ConditionalOperator handling as well, so there would be a bit of branching/etc perhaps to make this call 'ignored'.

That said, I'm not really sure what the implications of that would be.  I don't really see harm in the example you gave though, so I can put a patch together to do that.


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

https://reviews.llvm.org/D123680