ArcsinX 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) 
https://github.com/llvm/llvm-project/commit/596b63ad4019e61030803789a1844a0f1aeb34db
 (related issue 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.
- keep this as is

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)

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