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

Eric Fiselier via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 3 23:24:02 PDT 2018


EricWF added inline comments.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:8931
+                          /*ConstArg*/ true, false, false, false, false);
+  auto *CopyCtor = cast_or_null<CXXConstructorDecl>(SMI.getMethod());
+
----------------
rjmccall wrote:
> Sorry, I didn't realize you'd go off and write this code manually.
> 
> The way we generally handle this sort of thing is just to synthesize an expression in Sema that performs the copy-construction.  That way, the stdlib can be as crazy as it wants — default arguments on the copy-constructor or whatever else — and it just works.  The pattern to follow here is the code in Sema::BuildExceptionDeclaration, except that in your case you can completely dispense with faking up an OpaqueValueExpr and instead just build a DeclRefExpr to the actual variable.  (You could even use ActOnIdExpression for this, although just synthesizing the DRE shouldn't be too hard.)  Then the ComparisonCategoryInfo can just store expressions for each of the three (four?) variables, and in IRGen you just evaluate the appropriate one into the destination.
I think this goes against the direction Richard and I decided to go, which was to not synthesize any expressions.

The problem is that the synthesized expressions cannot be stored in `ComparisonCategoryInfo`, because the data it contains is not serialized. So when we read the AST back, the `ComparisonCategoryInfo` will be empty. And for obvious reasons we can't cache the data in Sema either. Additionally, we probably don't want to waste space building and storing synthesized expressions in each AST node which needs them.

Although the checking here leaves something to be desired, I suspect it's more than enough to handle any conforming STL implementation.




https://reviews.llvm.org/D45476





More information about the cfe-commits mailing list