[PATCH] D81474: Handle delayed-template-parsing functions imported into a non-dtp TU

Fri Jun 12 02:08:06 PDT 2020

sammccall added a comment.

Thanks for reviewing! Going to land this now as the bug is surfaced by other pending changes.

In D81474#2087095 <https://reviews.llvm.org/D81474#2087095>, @kadircet wrote:

> Looking at this some more, it looks like whenever clang is building a TU prefix(preamble), the assumption is that any delayed templates will be parsed at the end of the TU. That is unfortunately not true, if the rest of the TU is built with no-delayed-template-parsing. (as you've already said in the description)
>
> This suggests that DelayedTemplateParsing shouldn't be a benign langopt. But since it is, we should be setting late template parser at the end of TU if there are any delayed templates coming from either the preamble or in the rest of the TU.
>  The latter is already done by checking for langopt, we can check for the former using `Actions.ExternalSemaSource`. But in the absence of delayed templates, setting the parser is (currently) a no-op. So setting it all the time should hopefully be OK.
>  Hence LGTM.

Yeah I think checking ExternalSemaSource is probably correct. Not checking it is simpler though :-)

> Another option would be to change preamble building logic to parse all the late parsed templates at the end, instead of just serializing the tokens. That sounds like a more intrusive change though, so I am more comfortable with current one.

This has different semantics - the point of delayed template parsing is to wait for more symbols to be visible. If we parse at the end of the PCH, then symbols defined in the main file (but before instantiation) are not visible.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81474