On 5/27/25 5:12 PM, Jason Merrill wrote:
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?

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?

Thoughts? The functions could take the _classifier, or even just the m_classification_history, but that just moves the access problem into their callers, who would still need some way to get there from the diagnostic_context. Do you have another idea for that?

Jason

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

Reply via email to