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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
