The glog outputs recently introduced in hdf5.cpp break compatibility …

[MalteOeljeklaus]

The glog outputs recently introduced in hdf5.cpp break compatibility for some compilers (eg. MSVC) due to declaring a variable after a case label without its own scope. This commit fixes this problem by adding brackets to define a new variable scope after each case label. Further reading: https://stackoverflow.com/questions/92396/why-cant-variables-be-declared-in-a-switch-statement

[lukeyeager]

That's coming from #2978 - my bad.

Can we catch this sort of thing with the TravisCI build matrix somehow? Or with lint?

[bhack]

@lukeyeager There was an old effort to add other compilers (clang) to the build matrix but I have not collected enough interest. See #1218

[MalteOeljeklaus]

For MSVC, most necessary changes are single lines that can easily be put into #ifdef blocks so they will not introduce new problems with gcc. I created the PR because in my opinion, that is a feature worth preserving.

[ronghanghu]

I think we should merge this PR, since it does not affect Linux or OSX users, while makes things easier for community windows ports.

[MalteOeljeklaus]

I'm actually interested in what changes are allowed to improve compiler compatibility. I've made a MSVC port in [1] and it can be seen, that most changes to the original code are not that big, although sometimes it's required to include custom compatibility headers. Negative effects for gcc users can be avoided completely by using #ifdef _MSC_VER .

[1] master...MalteOeljeklaus:master

[willyd]

@MalteOeljeklaus I made a pull request #2816 to bring MSVC support in Caffe and core devs said that they would merge the PR provided that:

• Changes should be the least invasive possible. Concretely, we would rather rely on a cross-platform library than ifdefs where possible -- is the functionality ifdef'd presently available in boost for instance?
• Continuous integration is needed, since we do not have the know-how or equipment to check Windows. Could you set up appveyor as you sugested?
• Documentation should continue to make clear that Windows support is a community effort.

Although I'd rather have Caffe avoid non cross-platform code, I think your idea of libwincaffe is nice and in line with @shelhamer's comment above.

At the time I submitted the PR I did not take special care to avoid unnecessary ifdefs. I think your work brings us closer to a least invasive possible solution. However, I feel that we should probably try to have a single (or a few) platform specific header where we would put the necessary ifdefs and include this file wherever we need it to avoid spreading #ifdef _MSC_VER all over the place.

The CMake based build could be useful here too to automatically pull and build libwincaffe using ExternalProject like is done with glog and gflags.

I believe the most sensible approach to add support for other compilers such as MSVC, is to submit PRs that have the smallest number of changes possible to make review easier for the core devs like you did.

As a side note you can have a look at caffe-builder a CMake based project I put together to build caffe and its dependencies easily.

[longjon]

See #3365 (comment) which applies here as well.

[shelhamer]

Closing in favor of the minimal change in #3365, but thanks for the fix!