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

[Reid "Away June-Sep" Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm

IMO there is a pretty clear performance use case for this mode of operation, and it seems to me that you have addressed @rsmith's feedback. Please wait a few days to see if he has more to add, but otherwise, feel free to land this after Friday.



================
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");
----------------
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.


================
Comment at: clang/test/PCH/crash-12631281.cpp:4
+
+// RUN: %clang_cc1 -std=c++11 %s -emit-pch -fpch-instantiate-templates -o %t.pch
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -include-pch %t.pch -verify
----------------
I wish we had a better solution for parameterizing tests like this, but thank you for making sure this new mode is covered by the existing tests. It looks like you found some interesting issues.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69585