Oh, and putClientData seems like a better name. On Wed, Aug 3, 2011 at 1:02 PM, Ray Ryan <rj...@google.com> wrote:
> I'm still not crazy about having addClientData() and getClientData() on > separate objects. It seems to me that you've violated your own principal > that the GeneratorContext should be the only object that has to get passed > to the generator's helpers. > > Can addClientData() move to the context? That would require getClientData() > to be able to retrieve both cached and freshly added data. Is that > practical? Sounds like it would be hugely convenient. > > > On Wed, Aug 3, 2011 at 12:43 PM, <jbrosenb...@google.com> wrote: > >> Some responses. I did consider most of your suggestions, and in fact >> for a while I did have things as you suggest, so great minds think alike >> :). I'll outline my reasoning: >> >> First, IncrementalGenerators have to live in the same ecosystem as all >> the currently extent non-incremental Generator implementations. Having >> a uniform rebind process whereby the StandardRebindOracle is agnostic to >> this greatly simplified things. If we were starting from scratch, I >> think it would probably look quite a bit different. >> >> Standard Generators only return a type-name (which can possibly be >> null). The RebindResult allows legacy generators to be cleanly wrapped >> and presented to the rebind oracle in a uniform way. >> >> A RebindResult is not the same as a CachedRebindResult. The >> StandardRebindOracle uses the info in the RebindResult to decide how to >> proceed. It may or may not choose to construct a cache entry, and that >> cached entry might contain a partial subset of newly generated types and >> types from a previous cache entry. Since a generator itself shouldn't >> need to know how to integrate cached types and artifacts, this logic is >> better left up to the StandardRebindOracle. The RebindResult also >> contains a target type name, and client data, so it's just a container >> to wrap a generators results. >> >> In the case that an IncrementalGenerator decides it can return quickly >> and request that all previously cached output can be reused, it doesn't >> need to do anything, including not re-committing cached compilation >> units and artifacts to the context, etc. That's all magic handled by >> the rebind oracle. >> >> A CachedRebindResult is specifically input to an IncrementalGenerator, >> but it's cumbersome to pass it in as a separate arg to >> generateIncrementally. Having it live in the GeneratorContext makes one >> less thing a generator implementation needs to pass around to it's >> internal methods (since GeneratorContext at the least is commonly needed >> all over the place for a generator anyway). >> >> Remember too, that much of the time, an IncrementalGenerator will run >> when generatorResultCaching is not enabled, such as currently for all >> web-mode compiles, so it's more of a contextual thing anyway, which >> makes it more intuitive for it to live in the GeneratorContext. >> >> I did have a separate GeneratContextExt interface, which extended the >> base GeneratorContext interface, but ultimately this became unnecessary, >> and it greatly simplified the framework by unifying things. >> >> I think you had a couple other comments offline (but not summarized >> above), I'll wait for your further comments to respond to those. >> >> >> http://gwt-code-reviews.**appspot.com/1468804/<http://gwt-code-reviews.appspot.com/1468804/> >> > > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
