r246497 - [modules] Rework serialized DeclContext lookup table management. Instead of

[Richard Smith via cfe-commits]

On Tue, Sep 1, 2015 at 6:26 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Tue, Sep 1, 2015 at 4:43 AM, İsmail Dönmez
> <cfe-commits at lists.llvm.org> wrote:
> > Hi,
> >
> > On Tue, Sep 1, 2015 at 1:17 AM, Richard Smith via cfe-commits
> > <cfe-commits at lists.llvm.org> wrote:
> >> Author: rsmith
> >> Date: Mon Aug 31 17:17:11 2015
> >> New Revision: 246497
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=246497&view=rev
> >> Log:
> >> [modules] Rework serialized DeclContext lookup table management.
> Instead of
> >> walking the loaded ModuleFiles looking for lookup tables for the
> context, store
> >> them all in one place, and merge them together if we find we have too
> many
> >> (currently, more than 4). If we do merge, include the merged form in our
> >> serialized lookup table, so that downstream readers never need to look
> at our
> >> imports' tables.
> >>
> >> This gives a huge performance improvement to builds with very large
> numbers of
> >> modules (in some cases, more than a 2x speedup was observed).
> >>
> >> Added:
> >>     cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h
> >
> > This doesn't seem to compile with VS2015:
> >
> > FAILED: C:\PROGRA~2\MICROS~1.0\VC\bin\amd64\cl.exe   /nologo /TP
> > /DWIN32 /D_WINDOWS   -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291
> > -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503
> > -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610
> > -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389
> > -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4324 -w14062 -we4238 /W4
> > /Zc:inline /Zc:sizedDealloc- /MT /O2 /Ob2
> > -Itools\clang\lib\Serialization -I..\tools\clang\lib\Serialization
> > -I..\tools\clang\include -Itools\clang\include -Iinclude -I..\include
> >   -UNDEBUG  /EHs-c- /GR- /showIncludes -DCLANG_ENABLE_ARCMT
> > -DCLANG_ENABLE_OBJC_REWRITER -DCLANG_ENABLE_STATIC_ANALYZER
> > -DGTEST_HAS_RTTI=0 -D_CRT_NONSTDC_NO_DEPRECATE
> > -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE
> > -D_CRT_SECURE_NO_WARNINGS -D_GNU_SOURCE -D_HAS_EXCEPTIONS=0
> > -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS
> > -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
> >
> /Fotools\clang\lib\Serialization\CMakeFiles\clangSerialization.dir\ASTReader.cpp.obj
> > /Fdtools\clang\lib\Serialization\CMakeFiles\clangSerialization.dir\
> > /FS -c ..\tools\clang\lib\Serialization\ASTReader.cpp
> >
> c:\cygwin64\home\ismail\src\llvm\tools\clang\lib\serialization\MultiOnDiskHashTable.h(117):
> > error C2065: 'Files': undeclared identifier
> >
> c:\cygwin64\home\ismail\src\llvm\tools\clang\lib\serialization\MultiOnDiskHashTable.h(111):
> > note: while compiling class template member function 'void
> >
> clang::serialization::MultiOnDiskHashTable<clang::serialization::reader::ASTDeclContextNameLookupTrait>::removeOverriddenTables(void)'
> >
> c:\cygwin64\home\ismail\src\llvm\tools\clang\lib\serialization\MultiOnDiskHashTable.h(243):
> > note: see reference to function template instantiation 'void
> >
> clang::serialization::MultiOnDiskHashTable<clang::serialization::reader::ASTDeclContextNameLookupTrait>::removeOverriddenTables(void)'
> > being compiled
> >
> c:\cygwin64\home\ismail\src\llvm\tools\clang\lib\serialization\ASTReaderInternals.h(114):
> > note: see reference to class template instantiation
> >
> 'clang::serialization::MultiOnDiskHashTable<clang::serialization::reader::ASTDeclContextNameLookupTrait>'
> > being compiled
> >
> c:\cygwin64\home\ismail\src\llvm\tools\clang\lib\serialization\MultiOnDiskHashTable.h(117):
> > error C2228: left of '.count' must have class/struct/union
> >
> c:\cygwin64\home\ismail\src\llvm\tools\clang\lib\serialization\MultiOnDiskHashTable.h(117):
> > note: type is 'unknown-type'
> > ninja: build stopped: subcommand failed.
>
> I have reverted r246497 (which required also reverting r246524 and
> r246521 to avoid merge conflicts) to get back to green. Commit was
> r246546.
>
> I'm not certain why MSVC is falling over on this code, but I suspect
> compiler bug. If anyone has a reduced testcase (which I may spend some
> time on if I have a moment), I would be happy to report it to
> Microsoft.


It's two compiler bugs.

In MSVC 2013, providing a move constructor apparently doesn't suppress the
implicit generation of copy operations in some cases. In particular, given:

  struct X { X(); X(X&&); X &operator=(X&&); ~X(); };
  struct Y { X x; };

... Y is copyable (and in my case, copying it led to a use-after-free,
because X had the equivalent of a pointer member).

In MSVC 2015, it appears that implicit lambda capture doesn't work reliably
inside a member function of a class template. The testcase looks something
like:

template<typename T> struct S {
  typedef T type;
  type f() {
    type n;
    [&] { ++n; }();
    return n;
  }
};
int k = S<int>().f();

... though I don't know if that's enough to reproduce the rejects-valid.

Anyway, these are hopefully both worked around in r246582.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150901/039d3d17/attachment.html>