Fix some errors and warning when compiling with visual studio.

[smithgeek]

This pull request fixes some issues when compiling with visual studio, including #5.

[phs]

Howdy! Thanks for putting this together, let me take a look. Offhand the only piece I find a little controversial is the third commit, maybe you could explain the problem there in more detail?

I notice also that the infrastructure in this repo is looking a little 2010. I'm going to get it wired up with some virtualization support and build services (including one for windows) before doing anything else. Once that lands (probably later today) hopefully you won't mind rebasing.

[smithgeek]

In that 3rd patch that test was failing because the mock allocator function for Herbie was being called before the chasis or engine functions. My thought was that the order really doesn't matter as long as Herbie ends up with the correct CoupChasis and Hybrid engine which is already verified with the other asserts in that test.

[smithgeek]

Looks like you've been busy and taken care of a couple of these! Do you still want me to rebase any of these patches?

On another topic, have you ever considered trying to add something like the assisted inject feature guice has? It would be nice to have for a project I'm working on and was thinking about trying to add it. I wondered if you had any ideas about it?

Thanks for the great library!

[phs]

Yea, getting modern clang happy for OS X seems to have led me to the same spots.

I believe I also fixed the underlying issue requiring you to change the test in the 3rd commit: see if the current master works cleanly for you with MSVC.

[phs]

You asked about assisted injection; I hadn't really thought about it, but if the feature fits cleanly into a header-only library that deliberately does no code generation, then groovy ^_^.

[smithgeek]

I pulled in your changes and msvc doesn't like them at all. :) I get a bunch of these compile errors

error C2951: template declarations are only permitted at global, namespace, or class scope

It looks like it has something to do with your changes to apply_variadic, don't have time to look at it at the moment though.

[phs]

Ok. I'm also trying to get appveyor happy, which is a build service that offers (my only) access to MSVC. Hopefully whatever you're bumping into will fall out from that.

[phs]

Yea, appveyor is giving me something similar. Are these the errors you see?

https://ci.appveyor.com/project/phs/sauce/build/1.0.64/job/lw97bsblqtq6l844

[smithgeek]

Those errors from appveyor are what I was seeing.

I reverted the changed that caused that to work on the allocate_shared patch I commited and after that patch I'm not seeing the unit test failure any more. I might have misunderstood what the original failure was. Take a look and see what you think.

[phs]

The original failure was the fact that the dependencies were being injected (constructed) after the thing they are injected into. That seems counter-intuitive, but there it is.

The core issue was my choice in apply_variadic to inline the expressions that provide the dependencies into the constructor/function/method calls: arguments can be evaluated in whatever order the compiler wants (and apparently they can be evaluated after the function call itself? That was new for me.)

Anyway, I extracted the sub-expressions to locals to force a "sequence point" in pre-C++11 lingo. Then all the dependent side-effects (including observation by my mocks) would complete before the dependent call was ever made.

On the way I extracted some typedefs to make the template magic easier on the eyes. I don't know yet but I guess this may be where I made MSVC angry (though my typedef is not a template, and indeed the other two compilers vouch that there are no templates within functions.)

[phs]

std::allocate_shared (& alternate implementation friends) are new to me, I'll take a look at them vs. placement new.

[smithgeek]

We've seen the make/allocate _shared functions have some pretty significant performance improvements in certain scenarios. Based on the failing builds it looks like tr1 didn't have allocate_shared though. I'm also not sure if I picked the correct boost header, trying to do it from memory. I can verify that tomorow.

[phs]

Well boo. Pretty sure this is us (and a few spots in gmock as well): https://support.microsoft.com/en-us/kb/241940

EDIT: Hm, maybe. It's not clear if it's a match or not, but there are certainly bugs in modern MSVC that involve spuriously reporting this error.

[phs]

Wait. All the C2951 errors reported by appveyor are in injector.h or the vendored gmock/gmock-matchers.h, not (seemingly) apply_variadic. Could you post a log of the warnings you're seeing (or just the sauce files with line numbers?) Did you say you were able to rejigger something to get no warnings at all?

[smithgeek]

With the 849c9c7 patch included I'm not getting any warnings at all. It looks like all the warnings from that appveyor output is just due to the fact that the /EHsc compiler flag wasn't enabled. I do have that enabled in my project.

[phs]

Interesting. On the linked appveyor build (such as here) I'm seeing lots of error C2951: template declarations are only permitted at global, namespace, or class scope, in addition to the lack of exception semantics controlled by /EHsc.

[phs]

Surely you don't mean the latest appveyor build for this PR, which errored out due to basic config not being set.

[smithgeek]

Maybe I misread your other comment. I'm seeing all of those C2951 errors when building master, essentially the same thing appveyor was reporting. I'm not seeing any compiler warnings though.

To get everything working locally I reverted your last change to apply_variadic and used the 3 patches in this pull request. Then I had no errors or warnings.

[phs]

Ok, that makes more sense. I'll revert the latest master commit & get the appveyor config merged so we can prove that appveyor will do what you're seeing locally.

Then I'd like to try other variations of the reverted commit, since I do believe the current behavior allows the compiler to introduce subtle bugs.

[phs]

Ok, offending commit reverted & appveyor config merged. Try rebasing now.

[phs]

Hello! Since you're still out I'm tempted to pull in just your first two commits in a new PR, in an effort to get MSVC happy (& not cheat you out of the authorship.)

I'm open to working more on integrating allocate_shared but since that's a different topic (not merely compiler warnings) I think it should go in a separate PR from the first two commits.

[smithgeek]

Yup that seems like a smart approach. I can push a new patch here in a minute.

[phs]

Groovy, that's looking pretty good. The linker problem probably just needs me to pass /entry:mainCRTStartup.

Let me see if I can make the appveyor config happy with this and the needed /EHsc flag, then we can rebase & try again. In the mean time I imagine you could keep cooking the allocate_shared support (if you're interested.)

[phs]

The missing flags appear to be in & working now, feel free to rebase.

[phs]

Your commits appear in #20