vgvassilev wrote:

> > I'd feel uncomfortable exposing a `noload_redecls` interface because it can 
> > potentially break the AST invariants. I think allowing non-modules 
> > developers to choose when to complete a redeclaration chain will introduce 
> > more bugs than it can fix ;)
> > I'd propose to move forward with `getMostRecentExistingDecl` if it does the 
> > job. We can have a separate PR discussing the ergonomics if there is a 
> > strong feeling about going this way.
> 
> I'd like it as: (1) We already have `noload_lookup` and other similar 
> "unpublic" API (e.g., `getOwningModuleID` `invalidateCachedLinkage`) in 
> DeclBase.h.

  Well, sure, `noload_lookup` is done to check what's the current state before 
completing something externally. In fact, I think we should keep these 
interfaces at minimum and only use them where we really have no other way out.

> (2) Merging Redecls is a major performance hurting point of using modules. 

  Only when the modules approach is not bottom up. The non-bottom up approach 
has bigger problems than performance at the moment and frankly we cannot 
reasonably support it right now...

> (3) In the past, there are many "fix" which just tried to mitigate the bug by 
> loading all redecls without understanding the real problem.

  Does that need to happen in this PR? I believe it brings down the peak memory 
significantly for most of the module users and we should not wait too long to 
get this merged.


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

Reply via email to