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

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:8888
 
+bool Sema::BuildComparisonCategoryData(SourceLocation Loc) {
+  using CCKT = ComparisonCategoryKind;
----------------
rsmith wrote:
> EricWF wrote:
> > rsmith wrote:
> > > If you put this on Sema, you'll need to serialize it, and it's no longer just a cache.
> > > 
> > > I would prefer to treat this information strictly as a cache, and build it in ASTContext. Sema should then just be checking that the information is "valid" when it first makes use of such a comparison category type.
> > I agree that it's preferable to just have a cache. However can you clarify what you mean by "build it in ASTContext"?
> > 
> > Building the expressions will potentially require emitting a diagnostic, and use of bits of `Sema` (like name lookup).
> > I'm not sure how to do this inside `ASTContext`.
> > 
> > The current implementation caches the data in `ASTContext` but builds it as-needed in `Sema`. Could you clarify how to go about with the implementation you're suggesting?
> You don't need "real" name lookup here, just calling lookup on the DeclContext should be fine. We don't need to support more exotic ways of declaring these members, nor access checks etc.
> 
> I was imagining that Sema would check that we can find suitable members on the comparison category types as part of semantically checking a <=> expression, and the ASTContext side of things would not emit diagnostics.
> 
> (Put another way, we'd act as if we look up the members each time we need them, Sema would check that all the necessary members actually exist, and ASTContext would have a caching layer only for convenience / to avoid repeatedly doing the same lookups.)
That's pretty much how this currently works, right?

The only difference is that Sema still eagerly builds the potential result values eagerly, because they'll be needed later.


https://reviews.llvm.org/D45476