[libcxx-commits] [PATCH] D89530: [libcxx] [test] Fix filesystem_test_helper.h to compile for windows

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/support/filesystem_test_helper.h:31
 
+#ifdef _WIN32
+// MSVC headers warn if using the versions without a leading underscore.
----------------
ldionne wrote:
> mstorsjo wrote:
> > ldionne wrote:
> > > Instead, I would create a layer of indirection for the utilities we're using here like this:
> > > 
> > > ```
> > > namespace utils {
> > > 
> > > #if defined(_WIN32)
> > >   inline std::string getcwd() { /* windows impl */ }
> > >   inline int mkdir(char const*, int) { /* windows impl */ }
> > >   // etc
> > > #else
> > >   inline std::string getcwd() { /* posix impl */ }
> > >   inline int mkdir(char const*, int) { /* posix impl */ }
> > >   // etc
> > > #endif
> > > 
> > >   inline bool exists(std::string const& path) { /* common impl */ }
> > > } // end namespace utils
> > > ```
> > > 
> > > It's more verbose, but easier to follow and the code below can make unconditional calls to these functions without being cluttered by `#if`s.
> > Hmm, I can agree with the clarity, but for these functions, there's no `#if`s anywhere in the calling code. Especially the duplication for getcwd feels a bit harsh, when the difference is just between `::getcwd` and `::_getcwd`.
> > 
> > I thought we could get rid of a few of these (that just add the underscore prefix) by defining `_CRT_NONSTDC_NO_WARNINGS` though, leaving just mkdir and ftruncate. But that ends up requiring manually linking against `oldnames.lib` (which normally is linked implicitly, but not in the setup things are done in the libc++ test suite)...
> I think we should link against it manually if that's what's done automatically normally. That makes us closer to running the compiler the way it's run by default.
Ok, will have a poke into doing that. The thing is that normally, when using the compiler normally, we'd be calling `clang-cl`, which automatically adds both the C runtime and `oldnames.lib`, but when invoking plain `clang++.exe`, it sidesteps all of that, but the clang driver only adds the C runtime. I guess the proper fix there would be to make the clang driver add `oldnames.lib` correspondingly there.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89530/new/

https://reviews.llvm.org/D89530