ChuanqiXu9 wrote:

> > And again, also the maintainability is very important too. This is why I 
> > choose the minimal patch, maintainable and understandable. I tried to not 
> > building PCH. But I don't like that patch. It is workaround too and not the 
> > fundamental fix (fundamental fix lives in the serialization part in clang). 
> > Why do we choose a bigger workaround?
> 
> I found that for me clangd still crashes if I import a module in a header 
> file (even with this patch) and this happens at preamble index update. 
> Therefore, I believe we should completely disable preamble optimization if we 
> decide to address stability issue on the clangd side. I had a chance to take 
> a look at the original patch, and it also seemed architecturally incorrect to 
> me, as we reset the preamble size based on what's written in the headers.

I didn't update that patch. I observed a similar crash and I didn't meet it 
after I use this patch. That crash starts with TUScheduler's UpdatePreamble and 
then calls into ParsedAST::build.

I think if you really want that, you can make a patch and let's review it.

https://github.com/llvm/llvm-project/pull/187432
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to