llunak added inline comments.

================
Comment at: clang/lib/Sema/Sema.cpp:984
+
+    PerformPendingInstantiations();
   }
----------------
aganea wrote:
> All tests pass if you add `if (LangOpts.BuildingPCHWithObjectFile)` here. But 
> if a specialization occurs inside a .CPP which includes the .PCH, not sure 
> what would/should happen, and if that'S handled. Definitely needs a test for 
> that. @rsmith WDYT?
> All tests pass if you add if (LangOpts.BuildingPCHWithObjectFile) here.

That is because BuildingPCHWithObjectFile is almost unused by tests, so this 
just disables the change. But the change should be enabled for everything, it 
doesn't depend on whether PCH's object file is used.

> But if a specialization occurs inside a .CPP which includes the .PCH, not 
> sure what would/should happen, and if that'S handled. 

You mean this?

```
a.h:
#pragma once
template< typename T >
struct A { int foo() const; };
int bar(A<double>* a) { return a->foo(); }
a.cpp:
#include "a.h"
template<> int A<double>::foo() const { return 10; }

```
That errors out with "explicit specialization of 'foo' after instantiation" 
with or without PCH used, with or without my patch. I don't know about tests 
for it though. I can add this as one.



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