[PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

Tue Jul 4 01:00:33 PDT 2023

ChuanqiXu added a comment.

In D126694#4470358 <https://reviews.llvm.org/D126694#4470358>, @iains wrote:

> In D126694#4470297 <https://reviews.llvm.org/D126694#4470297>, @ChuanqiXu wrote:
>
>>> Yes, that was the decision at the last time we looked - because removing decls would degrade this - if we have new information that changes our preferred design, then fine.
>>
>> I remember the major reason for the last time to not remove the decls are that the design of AST doesn't support to remove decls. And my current idea is, we can refuse to write the discardable Decls into the BMIs.
>
> @rsmith pointed out that the API was not intended for that purpose - so, yes, you are correct that the next place to look was in the serialisation [but ISTR that this also has some challenges because of the way in which the structures are interconnected].  It might be necessary to do a pass before the serialisation and actually prune the AST there. (in a similar manner we'd need to prune it to remove non-inline function bodies in some future version)

Yeah, I just feel it is implementable. But I need to give it a try.

>>> One solution is to place the elision behind a flag so that the user can choose slower compilation with better diagnostics or faster compilation but maybe harder-to-find errors?
>>
>> I proposed to add a flag. But that was a helper for developers to find if we did anything wrong. I don't want to provide such a flag since on the one hand, there are already many flags  and on the other hand, form my user experience, it is not so hard to find the unexported decls. It is really much much more easier than template programming.
>
> Yeah, (as you know) I definitely prefer not to add more flags - there are too many - it was only an option in case there were many people against degrading diagnostic output.

Got it.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126694/new/

https://reviews.llvm.org/D126694