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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to