On Thu, 6 Mar 2025 at 13:54, Thomas Schwinge <tschwi...@baylibre.com> wrote: > > Hi! > > On 2025-02-26T23:08:29+0000, Jonathan Wakely <jwak...@redhat.com> wrote: > > On Wed, 26 Feb 2025 at 20:53, Thomas Schwinge <tschwi...@baylibre.com> > > wrote: > >> On 2025-02-26T10:50:11+0000, Jonathan Wakely <jwak...@redhat.com> wrote: > >> > On Wed, 26 Feb 2025 at 10:47, Jonathan Wakely <jwak...@redhat.com> wrote: > >> >> On Wed, 26 Feb 2025 at 10:19, Thomas Schwinge wrote: > >> >> > In particular, 'GLIBCXX_ENABLE_CXX_FLAGS' shouldn't overwrite > >> >> > 'EXTRA_CXX_FLAGS' > >> >> > (and prepend any additional '--enable-cxx-flags=[...]'). > >> >> > >> >> This seems good, but why prepend instead of append here? > >> >> If there are important flags passed down from top-level configure that > >> >> shouldn't be replaced by the libstdc++ --enable-cxx-flags option, can > >> >> we mention that in the new comment in acinclude.m4? > >> > > >> > Oh sorry, they're more likely to be from configure.host not from the > >> > top-level, right? > >> > >> That's right: for GCN, nvptx configurations inject '-fno-exceptions' etc. > >> via 'libstdc++-v3/configure.host'. > >> > >> > But if we prepend, then when users put bad options in > >> > --enable-cxx-flags we silently override that with good target-specific > >> > ones from configure.host > >> > >> That was my intention indeed -- but not a strong one. ;-) (I've myself > >> never used '--enable-cxx-flags=[...]'.) > > > > I have used it occasionally, but long ago. > > > >> > Maybe if users explicitly give bad options, they should get an error, > >> > or a bad result? > >> > >> That's fine for me. So if you think that makes more sense, then I'll be > >> happy to swap it around. I agree that it would be more standard to allow > >> user-specified flags to override the default ones. > > > > Yeah, I think I'd prefer that. OK with that change, and maybe change > > the comment in acinclude.m4 from "Prepend the additional flags." to > > something like: > > > > # Append the additional flags to any that came from configure.host > > Like in the attached > "libstdc++: Allow 'configure.host' to pre-set 'EXTRA_CFLAGS', > 'EXTRA_CXX_FLAGS'"?
Looks good. OK for trunk, thanks.