[PATCH] Local classes in templates (PR9685 fix)

[Erik Olofsson]

Hi doug.gregor,

This patch is an update to the patch I made last year: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120820/063039.html

Sorry for not getting to this earlier. I now have proper system setup for handling changes to the multiple patches we have against llvm, so I will be able to address review comments in a timely manner.

I have addressed all the minor things in the linked email.

> I don't quite understand this. Local classes cannot have member templates, so what exactly are we adjusting for here?

Clang was missing an error message when you have a template function inside a local class. I incorrectly thought this was allowed by the standard. I have now fixed clang to emit an error for this case. I also had to fix several tests that assume the same thing. 

I have also included the original tests for PR9685 in c++1y mode which allows this.

> My only other general comment is that this is a lot of information to save for template instantiations, but it's only used in the (relatively uncommon) case where there is a local class within a function template. Can we only save this information when the pattern from which the function is instantiated actually has a local class somewhere in it?

I don’t have time to do this optimization right now. I tried to optimize this last year when I first implemented this, but removed it because this was hard enough to get right without it. Could this be committed without it and be revisited?

This is some statistics for a function/template heavy compilation (which does not require this patch). It's generating grammar with boost::spirit:

clang compiled in release without asserts.

Number of saved scopes: 61336
Number of restored scopes: 10323

User time averaged over 5 invocations without patch: 11.381s
User time averaged over 5 invocations with patch: 11.375s

I could not find any significant difference in time, as the fluctuations were bigger that the changes, so I used Instruments to profile the compilation (had to sample at 40 µs to see the functions):

Running Time	Self		Symbol Name
17303.2ms   22.0%	0.0	 	clang-3 (46986)
23.4ms    0.1%	23.4	 	clang::LocalInstantiationScope::Exit()
18.6ms    0.1%	18.6	 	clang::FunctionInInstantiationScope::functionInScope(clang::LocalInstantiationScope*, clang::FunctionDecl*)
2.8ms    0.0%	2.8	 	clang::Sema::setupLocalScope(clang::RestoreLocalInstantiationScope&, clang::Decl*, clang::Decl*)

44.8ms total = 0.25 %

http://llvm-reviews.chandlerc.com/D1358

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  include/clang/Sema/Template.h
  lib/AST/ItaniumMangle.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  test/CodeGenCXX/mangle-local-class-names.cpp
  test/PCH/cxx-local-templates.cpp
  test/SemaTemplate/class-template-decl.cpp
  test/SemaTemplate/instantiate-exception-spec-cxx11.cpp
  test/SemaCXX/PR9685-cxx1y.cpp
  test/SemaCXX/PR9685.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1358.1.patch
Type: text/x-patch
Size: 55933 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130811/3dd4ccb3/attachment.bin>