[libcxx-commits] [PATCH] D98442: [libcxx] [test] Use GetWindowsInaccessibleDir() instead of dirs with perms::none in fs.op.is_*

Martin Storsjö via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 8 12:25:59 PDT 2021


mstorsjo added inline comments.


================
Comment at: libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.is_block_file/is_block_file.pass.cpp:84
     permissions(dir, perms::none);
+#endif
 
----------------
curdeius wrote:
> mstorsjo wrote:
> > Quuxplusone wrote:
> > > The general approach looks great to me, but I do think it'd make more sense to pull `scoped_test_env env;` up to the top of main regardless of `_WIN32`. Even if that "scope guard" is redundant on Windows, it feels sketchy to declare a "scope guard" variable inside an ifdef when its lifetime extends outside the ifdef.
> > > (If you do this, obviously, do it consistently everywhere.)
> > Hmm, I can see that it stylistically is weird in one way to have it outlive the ifdef, but I still think I prefer it this way - the ifdef branches set up different variables that then exist for the duration of the surrounding scope... If there's more opinions in this direction I can change it of course.
> I agree with Arthur, it is less surprising to have the guard in all branches of #if. Even if it's not necessary on Windows now, it may become necessary one day, this can avoid stupid mistakes then.
Ok, thanks for chiming in. Updating then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98442



More information about the libcxx-commits mailing list