On 5/27/25 4:47 PM, Jason Merrill wrote:
On 5/27/25 1:33 PM, David Malcolm wrote:
On Fri, 2025-05-23 at 16:58 -0400, Jason Merrill wrote:
On 4/14/25 9:57 AM, Jason Merrill wrote:
On 1/9/25 10:00 PM, Jason Merrill wrote:
Tested x86_64-pc-linux-gnu. Is the diagnostic.h change OK for
trunk?
Ping?
Ping.
Sorry for the delay in responding; comments below...
-- 8< --
To respect the #pragma diagnostic lines in libstdc++ headers when
compiling
with module std, we need to represent them in the module.
I think it's reasonable to make module_state a friend of
diagnostic_option_classifier to allow direct access to the data.
This
is a
different approach from how Jakub made PCH streaming members of
diagnostic_option_classifier, but it seems to me that modules
handling
belongs in module.cc.
Putting it in module.cc looks good to me, though perhaps it should be
just a friend of diagnostic_option_classifier but not of
diagnostic_context? Could the functions take a
diagnostic_option_classifier rather than a diagnostic_context?
diagnostic_context is something of a "big blob" of a class.
The friend in diagnostic_context is to be able to name
m_option_classifier. We could instead make that member public?
[...snip...]
+ bytes_out sec (to);
+ if (sec.streaming_p ())
+ sec.begin ();
I confess I don't fully understand the module code yet - in particular
the streaming vs non-streaming distinction. What are the "if
(sec.streaming_p ())" guards doing here? It looks it can be false if
the param "elf_out *to" is null (can that happen?), and if it's false,
then this function essentially becomes a no-op. Is that what we want?
Hmm, perhaps an early if (!sec.streaming_p ()) return would be simpler,
I'll try that.
That breaks, apparently because we need the early calls to
write_location to record that we need to represent these locations.
Jason
