On Sun, Jan 29, 2017 at 10:21 AM Richard Smith via Phabricator <
revi...@reviews.llvm.org> wrote:
> rsmith accepted this revision.
> rsmith added inline comments.
> This revision is now accepted and ready to land.
>
>
> ================
> Comment at: lib/AST/ExternalASTSource.cpp:33
> +ExternalASTSource::hasExternalDefinitions(unsigned ID) {
> + return EK_ReplyHazy;
> +}
> ----------------
> dblaikie wrote:
> > rsmith wrote:
> > > You should add support for this function to
> `clang::MultiplexExternalSemaSource` too.
> > Done - though is there any test coverage I should add? Not sure exactly
> when/where/how this is used.
> >
> > Also only made a rough guess at what the right behavior is here. It
> could be a bit more obvious if I made "hasExternalDefinitions" return
> Optional<ExtKind> - then when we find an ExternalASTSource that owns the ID
> (are the IDs unique across the MultiplexExternalSemaSource? I assume they
> have to be, but thought they'd only be valid within a single Module...
> guess I'm confused in a few ways - certainly haven't thought about it in
> great detail) we'd know to stop asking other sources. Probably not worth it
> unless there's a lot of sources?
> You could test this with `-chain-include`. I don't think there's any other
> in-tree way to get multiple external sources.
>
Ah - guess that doesn't make for very interesting testing, then - since I
wouldn't be able to make those external sources with interesting
differences in ExtDefs, I think? Happy to chat more if it's worth
testing/I've not understood how to exercise this.
- Dave
>
>
> https://reviews.llvm.org/D28845
>
>
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
