FTR, here's what a cl-built binary shows, with the same build flags other than is_clang=true.

Deleted: vs-build-base-members.png 199 KB

	vs-build-base-members.png 199 KB View Download

That's neat. This is definitely supposed to work and I've dumped fields in base classes using windbg, but clearly we need to test with VS too.

Adrian, do you want to take a look? This is probably more important than the virtual base type info we were talking about.

I'm investigating.

Scott, you're using a component build, right? The leading theory is that the type info for the base class (content::NavigationThrottle) is emitted in content.dll.pdb, and VS is not able to match up a forward decl in one PDB with a type definition in another PDB.

We have a debug info size optimization similar to key function optimization in the Itanium ABI, which only emits type info for classes with vtables in TUs where the vtable is emitted. The idea is that your binary won't link without a vtable, so you can always find the debug info there. This is a *very* important size optimization though, so I'm hesitant to turn it off...

Yes, component.

Maybe at least report a bug against VS then if it seems like it could resolve it? I dunno.

It could still be a problem with the type info we're generating.  I'm working on a simple test case so I can verify the hypothesis and see if there's anything we can do on our end.

Fixed in upstream Clang in r278861 by ensuring that we emit the type info for dllimported types.

amccarth: Did you check how much that slows down linking? Right now, linking in effectively /Z7 mode with clang-cl is pretty fast, while that isn't the case with cl.exe (see numbers on eg https://codereview.chromium.org/2228633002/). We need to be very careful that we don't accidentally ruin our link time.

I have not measured it.  This affects only dynamic classes that are explicitly imported from a DLL (e.g., with __declspec(dllimport)), which should be only a small fraction of the external classes and thus only a small bump in the amount of debug info.

Huh, every FOO_EXPORT macro expands to declspec(dllimport) -- in other words, every class used by a component that's not defined in the component is imported. Why is this only a small fraction?

I haven't looked at Chrome components specifically.  It seems odd that a DLL would export nearly every class it defines--that would be a huge API surface--or that another component would consume virtually all classes from a DLL.  That would imply a very high degree of coupling and seemingly defeat some of the reasons for breaking things into separate components.

But I'll do some measurements on Chrome binaries and report back.

The alternative is that clang-cl-built binaries get a worse debugging experience than cl-built ones.

Yes, chromium components are very coupled. It's a forced split into many binaries for build time iteration performance, but the code was never intended to be in separate pieces, or define any sort an API.

https://cs.chromium.org/search/?q=%5BA-Z%5D%2B_EXPORT&sq=package:chromium&type=cs

This is a good overview of the component build, both motivation and implementation: https://chromium.googlesource.com/chromium/src/+/master/docs/component_build.md

I compared the link time of all the Chrome DLLs with and without the change, and found that it's marginally faster with the change than without.

Methodology:

To get ninja to use my local Release build of clang-cl.exe, I copied it over the one in:

   D:\src\chromium\src\third_party\llvm-build\Release+Asserts\bin

In order to isolate the link times, I built Chrome Debug locally, then deleted all the resulting DLLs, and timed how long to rebuild them.

  D:\src\chromium\src>ninja -C out\Debug -j 40 chrome
  [ builds everything ]
  D:\src\chromium\src>del out\Debug\*.dll
  D:\src\chromium\src>timecmd ninja -C out\Debug -j 40 chrome
  [ re-links the DLLs and reports elapsed time ]

I did two timings each, with and without this patch.

Results:

With the change (extra debug info), this took 17:29.
Without the change, it took 18:38.

I re-ran each one (in the opposite order to rule out cache effects) and got similar numbers:  17:51, 18:32.

I think it's safe to say that this fix did not hurt link time and may have actually helped it.  As for why it would actually be faster with the extra debug info for the exported base classes: I don't have an explanation.  I suppose it's possible that it's faster to find the debug info for the base class than to determine that it doesn't exist.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a033f395bf2547cf4764f77cc9c86d08f3e22c23

commit a033f395bf2547cf4764f77cc9c86d08f3e22c23
Author: thakis <thakis@chromium.org>
Date: Wed Aug 31 05:16:14 2016

Roll clang 278861:280106.

* win: Members of base classes now should show up in debugger.
* win: Debugger shouldn't show funny highlights anymore due to
  debug info no longer including column information.  (we still
  force this on if sanitizers are used, mostly for clusterfuzz.
  maybe we want to make this toggleable independent of sanitizers
  at some point)
* win: -Wextern-initializer no longer warns on midl-generated code
* win: clang-cl now accepts /source-encoding:utf-8 and friends
  (utf-8 was the source enconding in clang-cl before already, but
  now we don't warn on an explicit flag requesting this)
* all platforms: Three plugin checks are now on-by-default,
  remove flags for these (see
    https://codereview.chromium.org/2267713003
    https://codereview.chromium.org/2268203002
    https://codereview.chromium.org/2265093002
  )
* win: clang-cl's /Brepro now does what it's supposed to do
* win: clang-cl now emits absolute paths in diagnostics, by
  popular request.

Ran `tools/clang/scripts/upload_revision.py 280106`.

BUG= 640254 , 637456 , 636109 , 636091 , 636099 

Review-Url: https://codereview.chromium.org/2292173002
Cr-Commit-Position: refs/heads/master@{#415563}

[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/build/config/clang/BUILD.gn
[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/build/config/compiler/BUILD.gn
[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/build/config/sanitizers/BUILD.gn
[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/build/config/win/BUILD.gn
[modify] https://crrev.com/a033f395bf2547cf4764f77cc9c86d08f3e22c23/tools/clang/scripts/update.py

This should work now, please shout if it doesn't.