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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
