[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/AST/ExprConstant.cpp:8829
+  return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() {
+    return ExprEvaluatorBaseTy::VisitBinaryOperator(E);
+  });
----------------
It'd be clearer to call `VisitBinCmp` rather than `VisitBinaryOperator`.


================
Comment at: lib/CodeGen/CGExprAgg.cpp:971
+  auto EmitCmpRes = [&](const VarDecl *VD) {
+    return CGF.CGM.GetAddrOfGlobalVar(VD);
+  };
----------------
Perhaps directly emit the constant value here rather than the address of the global? I think we should consider what IR we want to see coming out of Clang, and I don't think that IR should contain loads from globals to get the small constant integer that is the value of the conversion result.

I think it would be reasonable for us to say that we require the standard library types to contain exactly one non-static data member of integral type, and for us to form a select between the relevant integer values here. We really have no need to support all possible implementations of these types, and we can revisit this if some other standard library implementation ships types that don't follow that pattern. (If we find such a standard library, we could emit multiple selects, or a first-class aggregate select, or whatever generates the best code at -O0.)


https://reviews.llvm.org/D45476