v.g.vassilev added a comment.

In https://reviews.llvm.org/D34444#818030, @rjmccall wrote:
> In https://reviews.llvm.org/D34444#812836, @v.g.vassilev wrote:
>
> > In https://reviews.llvm.org/D34444#812418, @rjmccall wrote:
> >
> > > In https://reviews.llvm.org/D34444#795175, @v.g.vassilev wrote:
> > >
> > > > @rjmccall, thanks for the prompt and thorough reply.
> > > >
> > > > In https://reviews.llvm.org/D34444#793311, @rjmccall wrote:
> > > >
> > > > > Okay.  In that case, I see two problems, one major and one 
> > > > > potentially major.
> > > >
> > > >
> > > >
> > > >
> > > >   This is a very accurate diagnosis which took us 5 years to discover 
> > > > on an empirical basis ;)
> > >
> > >
> > > You could've asked at any time. :)
> >
> >
> > True. I am not really sure I knew what to ask, though ;)
>
>
> We're open to general "I'm trying to do this and having problems" questions 
> on the mailing lists.  You probably would've needed to know to CC me 
> specifically, though; sadly, I can't keep up with all the lists I need to.


Good to know. Thanks! I will come back to you once I get rid of our O(100) 
clang patches to discuss what would be the best way of supporting incremental 
compilation.

>>> That's quite brittle, because that code is only executed in a code path 
>>> that only you are using, and you're not adding any tests.  I would greatly 
>>> prefer a change to IRGen's core assumptions, as suggested.
>> 
>> I am open to changing this code as well. That should probably be another 
>> review.
> 
> I agree.  Are you comfortable with blocking this review until that lands?  It 
> seems like it would significantly change this.

I am afraid that will slow down (if not suspend) our efforts to upstream our 
local patches. This patch is pretty fundamental for cling and if we change it 
now, I will have to go back and rework our implementation. I'd be much more 
comfortable in reworking it once we run on vanilla clang (then efforts seems 
easier to justify on our end).

It seems to me that despite being suboptimal, it is not very intrusive and it 
would effect only on our use case. I can keep track of such patches and come 
back to you for advice how to do them best. Would that make sense?

> 
> 
>>> I feel it is important that there be a way to inform an ASTConsumer that no 
>>> further requests will be made of it, something other than calling its 
>>> destructor.  I would like you to make sure that the ASTConsumer interface 
>>> supports that and that that call is not made too soon in your alternate 
>>> processing mode.
>> 
>> Do you have a preference of a name of this new interface?
> 
> Maybe just "finish"?
> 
> John.




Repository:
  rL LLVM

https://reviews.llvm.org/D34444



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to