[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.


================
Comment at: clang/lib/Sema/Sema.cpp:985-986
+
+    // FIXME: Instantiating implicit templates already in the PCH breaks some
+    // OpenMP-specific code paths, see https://reviews.llvm.org/D69585 .
+    if(LangOpts.PCHInstantiateTemplates && !LangOpts.OpenMP) {
----------------
llunak wrote:
> rnk wrote:
> > This really deserves to be debugged before we land it.
> I debugged this more than 2 months ago, that's why the OpenMP code owner is added as a reviewer.  The initial diff (that I have no idea how to display here) included a suggested fix and the description said this was a WIP and included my findings about it. As far as I can tell the only effect that had was that this patch was sitting here all the time without a single reaction, so if nobody OpenMP related cares then I do not either.
> 
> 
So the original comment said "breaks some Open-MP specific code paths" - what kind of breakage occurs? (& I take it with the patch in its current form that breakage is present in the patch (as opposed to the previous version that conditionalized this feature so it didn't happen when OpenMP was enabled)?)


================
Comment at: clang/test/PCH/specialization-after-instantiation.cpp:2
+// Test this without pch.
+// RUN: %clang_cc1 -fsyntax-only -verify -DBODY %s
+
----------------
Does this need testing without PCH here? Presumably that's already been tested elsewhere for some time now?


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

https://reviews.llvm.org/D69585