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

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius 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
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98442