[PATCH] D69585: Add option to instantiate templates already in the PCH

Sat May 23 08:59:47 PDT 2020

llunak marked an inline comment as done.
llunak added inline comments.

================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5610
+  if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
+                   options::OPT_fno_pch_instantiate_templates, false))
+    CmdArgs.push_back("-fpch-instantiate-templates");
----------------
rnk wrote:
> Does MSVC default to this behavior? Should this default to true with clang-cl /Yu / /Yc? This can be future work and does not need to be part of this patch.
Since MSVC is noticeably faster for an equivalent PCH compile than current Clang, presumably it instantiates templates already in the PCH. But that doesn't really matter for this patch, if it were ok to enable this by default for clang-cl, than it would be ok also for clang itself. That cannot be done now though, https://reviews.llvm.org/D69585#1946765 points out a corner case where this change makes a valid compilation error out, and that's the reason for having this behind a flag. I expect Clang could possibly be adjusted to bail out and delay template instantiantion in such a case until it can be performed successfully, but given the response rate to my PCH patches I first wanted to get the feature in somehow, and I can try to make the flag default/irrelevant later.

Repository:
  rC Clang

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

https://reviews.llvm.org/D69585