ChuanqiXu9 wrote:

> > > I think clangd should reparse opened files, when dependent file changes 
> > > (independent of is this include happens in preamble section or not). In 
> > > this case clangd doesn't do this if include is not in the preamble 
> > > section, so this looks like another problem.
> > > This patch introduces ability to avoid preamble build and I think this 
> > > works as expected. But yes, we found some problem cases but these 
> > > problems have different origin. Anyway, this is just an option, which can 
> > > be disabled/enabled.
> > 
> > 
> > If you think this is a missing feature, and the current patch introduces 
> > some problems but acceptable as modules support are experimental and we 
> > will add the feature back in the feature. Let's add them in document. And I 
> > think we can land this.
> 
> Returning to your concern. I found that reparsing of opened files is based on 
> preamble and this was on purpose because it was more simple to implement 
> (just check if preamble is compatible with previous one or not). That's why 
> this doesn't work for include directives in the middle of the file (and for 
> our case when we don't build preamble) 
> [596b63a](https://github.com/llvm/llvm-project/commit/596b63ad4019e61030803789a1844a0f1aeb34db)
>  (related issue 
> [clangd/clangd#107](https://github.com/clangd/clangd/issues/107) )
> 
> So, if we decide to go with this patch, I think we have the following options:
> 
> * we can always report that preamble is incompatible if we skip preamble 
> build (just add a check into isPreambleCompatible() and return false). This 
> will lead to all opened files reparsing at any file save.

This seems unacceptable. 

> * keep this as is

At least for modules, it is not so bad. At least support for modules in clangd 
is still experimental.

> 
> In any case I think we need to file an issue about this problem (with a 
> header inclusion which is outside of the preamble bounds). And this can be 
> fixed with a check at a file save: if this file is in include structure of an 
> opened file or not (needs additional investigation, but I think this should 
> not be very hard)

Yes, it is an existing issue that the files not belong to the preamble are not 
watched. And we need another fix for it.



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

Reply via email to