Re: [PATCH] libstdc++: testsuite: test symlnks ifdef _GLIBCXX_HAVE_SYMLINK

[Jonathan Wakely <jwakely@redhat.com>]

On Thu, 23 Jun 2022 at 12:02, Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Jun 22, 2022, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> > OK.
>
> > I'd like to clean this up so the tests don't rely on the "internal"
> > HAVE_SYMLINK macro. We could add something like this to
> > testsuite/util/testsuite_fs.h
>
> > #if defined(__MINGW32__) || defined(__MINGW64__) \
> >   || !defined (_GLIBCXX_HAVE_SYMLINK)
> > # define NO_SYMLINKS
> > #endif
>
> I took your suggestion, and took it a little further.  Here's the
> result.

Nice, thanks!

>
> Regstrapped on x86_64-linux-gnu, also tested with a cross to
> aarch64-rtems6.  Ok to install?

All the changes to the actual tests are fine.

> diff --git a/libstdc++-v3/testsuite/lib/libstdc++.exp b/libstdc++-v3/testsuite/lib/libstdc++.exp
> index 93fdfee687ddb..d71d88fb99837 100644
> --- a/libstdc++-v3/testsuite/lib/libstdc++.exp
> +++ b/libstdc++-v3/testsuite/lib/libstdc++.exp
> @@ -974,13 +974,16 @@ proc v3_try_preprocess { name code flags } {
>  }
>
>  # Return 1 if COND evaluates to true in the preprocessor, 0 otherwise.
> -# The <bits/c++config.h> config header is included.
> -proc v3_check_preprocessor_condition { name cond } {
> +# The <bits/c++config.h> config header is included, and INC, if given,
> +# is pasted between it and the condition evaluation, so it can be used
> +# for additional #include's.
> +proc v3_check_preprocessor_condition { name cond inc } {

Could this new arg be given a default value, so every caller doesn't
have to pass "" to it?

proc v3_check_preprocessor_condition { name cond {inc ""} } {

> diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h
> index 9358a04e56c1f..048f03103e436 100644
> --- a/libstdc++-v3/testsuite/util/testsuite_fs.h
> +++ b/libstdc++-v3/testsuite/util/testsuite_fs.h
> @@ -42,6 +42,11 @@ namespace test_fs = std::experimental::filesystem;
>  #include <random>   // std::random_device
>  #endif
>
> +#if defined(__MINGW32__) || defined(__MINGW64__) \
> +  || !defined (_GLIBCXX_HAVE_SYMLINK)
> +#define NO_SYMLINKS
> +#endif

I think this could be simplified to just
#ifndef _GLIBCXX_HAVE_SYMLINK
because that is false on mingw:
$ grep HAVE_SYMLINK include/x86_64-w64-mingw32/bits/c++config.h
/* #undef _GLIBCXX_HAVE_SYMLINK */

Windows uses CreateSymbolicLinkA to create its symlinks.

And if NO_SYMLINKS just becomes a "public" name for
_GLIBCXX_HAVE_SYMLINK that we can use in tests, then the dg
check_v3_target_fs_symlinks proc could just test that macro too,
without needing to include <testsuite_fs.h> to test NO_SYMLINKS. I
don't feel strongly about that though. The $inc addition to
v3_check_preprocessor_condition seems useful (although it will slow
down evaluation of that condition, because <testsuite_fs.h> includes a
lot of other headers).
And if the definition of NO_SYMLINKS gets more complicated in future
(e.g. we add something like && !defined __foo__) then using
NO_SYMLINKS in the dg proc will keep them in sync.

I'd like the default argument for v3_check_preprocessor_condition so
we don't need to add "" everywhere in that file, but the rest of the
patch is OK as-is or simplified as above, I don't mind. Even keeping
the MINGW tests in the NO_SYMLINKS definition is OK (it certainly
doesn't hurt, it's just a bit redundant).