On Wed, Oct 09, 2024 at 07:06:26PM -0400, Patrick Palka wrote:
> On Wed, 9 Oct 2024, Jason Merrill wrote:
>
> > Tested x86_64-pc-linux-gnu, will apply to trunk with the rest of the patch
> > series.
> >
> > -- 8< --
> >
> > At this point there doesn't seem to be much reason not to have modules
> > support enabled by default in C++20, and it's good get more test coverage to
> > find corner case bugs like some I fixed recently.
>
> Not sure how much we care about PCH anymore, but won't this effectively
> disable PCH in C++20 and later due to
>
> /* C++ modules and PCH don't play together. */
> if (flag_modules)
> return 2;
>
> in c_common_valid_pch?
Is it known why those 3 lines were added there?
Is it just somebody who uses modules doesn't need PCH, modules obsolete PCH,
or some code in module.cc lacking GTY(()) markups needed for PCH
save/restore, something else?
If it is just a precaution, perhaps we should just remove it and add a few
tests, if it is known that some cases just don't work with PCH, perhaps
only return 2; if e.g. some module keywords (or anything related to it; or
whatever is known not to work with PCH) are seen in the PCH header rather
than just because -fmodules is on, that option doesn't imply one actually
uses modules, just that one could.
Jakub
