public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [pushed 1/4] diagnostics: eliminate diagnostic_kind_count
@ 2023-11-06 19:49 David Malcolm
  2023-11-06 19:49 ` [pushed 2/4] diagnostics: make diagnostic_context::m_urlifier private David Malcolm
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: David Malcolm @ 2023-11-06 19:49 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

No functional change intended.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-5166-g579bb65cdd35a4.

gcc/ChangeLog:
	* diagnostic.cc (diagnostic_context::check_max_errors): Replace
	uses of diagnostic_kind_count with simple field acesss.
	(diagnostic_context::report_diagnostic): Likewise.
	(diagnostic_text_output_format::~diagnostic_text_output_format):
	Replace use of diagnostic_kind_count with
	diagnostic_context::diagnostic_count.
	* diagnostic.h (diagnostic_kind_count): Delete.
	(errorcount): Replace use of diagnostic_kind_count with
	diagnostic_context::diagnostic_count.
	(warningcount): Likewise.
	(werrorcount): Likewise.
	(sorrycount): Likewise.
---
 gcc/diagnostic.cc | 16 ++++++++--------
 gcc/diagnostic.h  | 17 ++++-------------
 2 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index e917e6ce4ac..90103e150f7 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -651,9 +651,9 @@ diagnostic_context::check_max_errors (bool flush)
   if (!m_max_errors)
     return;
 
-  int count = (diagnostic_kind_count (this, DK_ERROR)
-	       + diagnostic_kind_count (this, DK_SORRY)
-	       + diagnostic_kind_count (this, DK_WERROR));
+  int count = (m_diagnostic_count[DK_ERROR]
+	       + m_diagnostic_count[DK_SORRY]
+	       + m_diagnostic_count[DK_WERROR]);
 
   if (count >= m_max_errors)
     {
@@ -1547,8 +1547,8 @@ diagnostic_context::report_diagnostic (diagnostic_info *diagnostic)
 	 error has already occurred.  This is counteracted by
 	 abort_on_error.  */
       if (!CHECKING_P
-	  && (diagnostic_kind_count (this, DK_ERROR) > 0
-	      || diagnostic_kind_count (this, DK_SORRY) > 0)
+	  && (m_diagnostic_count[DK_ERROR] > 0
+	      || m_diagnostic_count[DK_SORRY] > 0)
 	  && !m_abort_on_error)
 	{
 	  expanded_location s 
@@ -1563,9 +1563,9 @@ diagnostic_context::report_diagnostic (diagnostic_info *diagnostic)
 			     diagnostic->message.m_args_ptr);
     }
   if (diagnostic->kind == DK_ERROR && orig_diag_kind == DK_WARNING)
-    ++diagnostic_kind_count (this, DK_WERROR);
+    ++m_diagnostic_count[DK_WERROR];
   else
-    ++diagnostic_kind_count (this, diagnostic->kind);
+    ++m_diagnostic_count[diagnostic->kind];
 
   /* Is this the initial diagnostic within the stack of groups?  */
   if (m_diagnostic_groups.m_emission_count == 0)
@@ -2336,7 +2336,7 @@ auto_diagnostic_group::~auto_diagnostic_group ()
 diagnostic_text_output_format::~diagnostic_text_output_format ()
 {
   /* Some of the errors may actually have been warnings.  */
-  if (diagnostic_kind_count (&m_context, DK_WERROR))
+  if (m_context.diagnostic_count (DK_WERROR))
     {
       /* -Werror was given.  */
       if (m_context.warning_as_error_requested_p ())
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index cf21558c7b2..4ef031b5d1c 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -701,24 +701,15 @@ extern diagnostic_context *global_dc;
    ready for use.  */
 #define diagnostic_ready_p() (global_dc->printer != NULL)
 
-/* The total count of a KIND of diagnostics emitted so far.  */
-
-inline int &
-diagnostic_kind_count (diagnostic_context *context,
-		       diagnostic_t kind)
-{
-  return context->diagnostic_count (kind);
-}
-
 /* The number of errors that have been issued so far.  Ideally, these
    would take a diagnostic_context as an argument.  */
-#define errorcount diagnostic_kind_count (global_dc, DK_ERROR)
+#define errorcount global_dc->diagnostic_count (DK_ERROR)
 /* Similarly, but for warnings.  */
-#define warningcount diagnostic_kind_count (global_dc, DK_WARNING)
+#define warningcount global_dc->diagnostic_count (DK_WARNING)
 /* Similarly, but for warnings promoted to errors.  */
-#define werrorcount diagnostic_kind_count (global_dc, DK_WERROR)
+#define werrorcount global_dc->diagnostic_count (DK_WERROR)
 /* Similarly, but for sorrys.  */
-#define sorrycount diagnostic_kind_count (global_dc, DK_SORRY)
+#define sorrycount global_dc->diagnostic_count (DK_SORRY)
 
 /* Returns nonzero if warnings should be emitted.  */
 #define diagnostic_report_warnings_p(DC, LOC)				\
-- 
2.26.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pushed 2/4] diagnostics: make diagnostic_context::m_urlifier private
  2023-11-06 19:49 [pushed 1/4] diagnostics: eliminate diagnostic_kind_count David Malcolm
@ 2023-11-06 19:49 ` David Malcolm
  2023-11-06 19:49 ` [pushed 3/4] diagnostics: introduce class diagnostic_option_classifier David Malcolm
  2023-11-06 19:49 ` [pushed 4/4] diagnostics: split out struct diagnostic_source_printing_options David Malcolm
  2 siblings, 0 replies; 4+ messages in thread
From: David Malcolm @ 2023-11-06 19:49 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

No functional change intended.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-5167-ga526cc6ff32e22.

gcc/ChangeLog:
	* diagnostic.cc (diagnostic_context::set_urlifier): New.
	* diagnostic.h (diagnostic_context::set_urlifier): New decl.
	(diagnostic_context::m_urlifier): Make private.
	* gcc.cc (driver::global_initializations): Use set_urlifier rather
	than directly setting field.
	* toplev.cc (general_init): Likewise.
---
 gcc/diagnostic.cc | 8 ++++++++
 gcc/diagnostic.h  | 3 +++
 gcc/gcc.cc        | 2 +-
 gcc/toplev.cc     | 2 +-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index 90103e150f7..c617b34f02b 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -373,6 +373,14 @@ diagnostic_context::set_client_data_hooks (diagnostic_client_data_hooks *hooks)
   m_client_data_hooks = hooks;
 }
 
+void
+diagnostic_context::set_urlifier (urlifier *urlifier)
+{
+  /* Ideally we'd use a std::unique_ptr here.  */
+  delete m_urlifier;
+  m_urlifier = urlifier;
+}
+
 void
 diagnostic_context::create_edit_context ()
 {
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 4ef031b5d1c..f9950ec2cf8 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -297,6 +297,7 @@ public:
   void set_output_format (diagnostic_output_format *output_format);
   void set_text_art_charset (enum diagnostic_text_art_charset charset);
   void set_client_data_hooks (diagnostic_client_data_hooks *hooks);
+  void set_urlifier (urlifier *);
   void create_edit_context ();
   void set_warning_as_error_requested (bool val)
   {
@@ -518,10 +519,12 @@ public:
      particular option.  */
   char *(*m_get_option_url) (diagnostic_context *, int);
 
+private:
   /* An optional hook for adding URLs to quoted text strings in
      diagnostics.  Only used for the main diagnostic message.  */
   urlifier *m_urlifier;
 
+public:
   void (*m_print_path) (diagnostic_context *, const diagnostic_path *);
   json::value *(*m_make_json_for_path) (diagnostic_context *,
 					const diagnostic_path *);
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index 02464958f36..51120c1489e 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -8292,7 +8292,7 @@ driver::global_initializations ()
   diagnostic_initialize (global_dc, 0);
   diagnostic_color_init (global_dc);
   diagnostic_urls_init (global_dc);
-  global_dc->m_urlifier = make_gcc_urlifier ();
+  global_dc->set_urlifier (make_gcc_urlifier ());
 
 #ifdef GCC_DRIVER_HOST_INITIALIZATION
   /* Perform host dependent initialization when needed.  */
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index e39162a3e49..d8e8978dd55 100644
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -1049,7 +1049,7 @@ general_init (const char *argv0, bool init_signals)
   global_dc->m_option_state = &global_options;
   global_dc->m_option_name = option_name;
   global_dc->m_get_option_url = get_option_url;
-  global_dc->m_urlifier = make_gcc_urlifier ();
+  global_dc->set_urlifier (make_gcc_urlifier ());
 
   if (init_signals)
     {
-- 
2.26.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pushed 3/4] diagnostics: introduce class diagnostic_option_classifier
  2023-11-06 19:49 [pushed 1/4] diagnostics: eliminate diagnostic_kind_count David Malcolm
  2023-11-06 19:49 ` [pushed 2/4] diagnostics: make diagnostic_context::m_urlifier private David Malcolm
@ 2023-11-06 19:49 ` David Malcolm
  2023-11-06 19:49 ` [pushed 4/4] diagnostics: split out struct diagnostic_source_printing_options David Malcolm
  2 siblings, 0 replies; 4+ messages in thread
From: David Malcolm @ 2023-11-06 19:49 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

This patch gathers the 6 fields in diagnostic_context relating to
keeping track of overriding the severity of warnings, and
pushing/popping those severities, moving them all into a new class
diagnostic_option_classifier.

No functional change intended.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-5168-g38763e2c188fa9.

gcc/ChangeLog:
	* diagnostic.cc (diagnostic_context::push_diagnostics): Convert
	to...
	(diagnostic_option_classifier::push): ...this.
	(diagnostic_context::pop_diagnostics): Convert to...
	(diagnostic_option_classifier::pop): ...this.
	(diagnostic_context::initialize): Move code to...
	(diagnostic_option_classifier::init): ...this new function.
	(diagnostic_context::finish): Move code to...
	(diagnostic_option_classifier::fini): ...this new function.
	(diagnostic_context::classify_diagnostic): Convert to...
	(diagnostic_option_classifier::classify_diagnostic): ...this.
	(diagnostic_context::update_effective_level_from_pragmas): Convert
	to...
	(diagnostic_option_classifier::update_effective_level_from_pragmas):
	...this.
	(diagnostic_context::diagnostic_enabled): Update for refactoring.
	* diagnostic.h (struct diagnostic_classification_change_t): Move into...
	(class diagnostic_option_classifier): ...this new class.
	(diagnostic_context::option_unspecified_p): Update for move of
	fields into m_option_classifier.
	(diagnostic_context::classify_diagnostic): Likewise.
	(diagnostic_context::push_diagnostics): Likewise.
	(diagnostic_context::pop_diagnostics): Likewise.
	(diagnostic_context::update_effective_level_from_pragmas): Delete.
	(diagnostic_context::m_classify_diagnostic): Move into class
	diagnostic_option_classifier.
	(diagnostic_context::m_option_classifier): Likewise.
	(diagnostic_context::m_classification_history): Likewise.
	(diagnostic_context::m_n_classification_history): Likewise.
	(diagnostic_context::m_push_list): Likewise.
	(diagnostic_context::m_n_push): Likewise.
	(diagnostic_context::m_option_classifier): New.
---
 gcc/diagnostic.cc | 124 ++++++++++++++++++++++++-------------------
 gcc/diagnostic.h  | 131 ++++++++++++++++++++++++++++++++--------------
 2 files changed, 163 insertions(+), 92 deletions(-)

diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index c617b34f02b..addd6606eaa 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -149,13 +149,63 @@ diagnostic_set_caret_max_width (diagnostic_context *context, int value)
   context->m_source_printing.max_width = value;
 }
 
+void
+diagnostic_option_classifier::init (int n_opts)
+{
+  m_n_opts = n_opts;
+  m_classify_diagnostic = XNEWVEC (diagnostic_t, n_opts);
+  for (int i = 0; i < n_opts; i++)
+    m_classify_diagnostic[i] = DK_UNSPECIFIED;
+  m_push_list = nullptr;
+  m_n_push = 0;
+}
+
+void
+diagnostic_option_classifier::fini ()
+{
+  XDELETEVEC (m_classify_diagnostic);
+  m_classify_diagnostic = nullptr;
+  free (m_push_list);
+  m_n_push = 0;
+}
+
+/* Save all diagnostic classifications in a stack.  */
+
+void
+diagnostic_option_classifier::push ()
+{
+  m_push_list = (int *) xrealloc (m_push_list, (m_n_push + 1) * sizeof (int));
+  m_push_list[m_n_push ++] = m_n_classification_history;
+}
+
+/* Restore the topmost classification set off the stack.  If the stack
+   is empty, revert to the state based on command line parameters.  */
+
+void
+diagnostic_option_classifier::pop (location_t where)
+{
+  int jump_to;
+
+  if (m_n_push)
+    jump_to = m_push_list [-- m_n_push];
+  else
+    jump_to = 0;
+
+  const int i = m_n_classification_history;
+  m_classification_history =
+    (diagnostic_classification_change_t *) xrealloc (m_classification_history, (i + 1)
+						     * sizeof (diagnostic_classification_change_t));
+  m_classification_history[i].location = where;
+  m_classification_history[i].option = jump_to;
+  m_classification_history[i].kind = DK_POP;
+  m_n_classification_history ++;
+}
+
 /* Initialize the diagnostic message outputting machinery.  */
 
 void
 diagnostic_context::initialize (int n_opts)
 {
-  int i;
-
   /* Allocate a basic pretty-printer.  Clients will replace this a
      much more elaborated pretty-printer if they wish.  */
   this->printer = XNEW (pretty_printer);
@@ -165,12 +215,10 @@ diagnostic_context::initialize (int n_opts)
   memset (m_diagnostic_count, 0, sizeof m_diagnostic_count);
   m_warning_as_error_requested = false;
   m_n_opts = n_opts;
-  m_classify_diagnostic = XNEWVEC (diagnostic_t, n_opts);
-  for (i = 0; i < n_opts; i++)
-    m_classify_diagnostic[i] = DK_UNSPECIFIED;
+  m_option_classifier.init (n_opts);
   m_source_printing.enabled = false;
   diagnostic_set_caret_max_width (this, pp_line_cutoff (this->printer));
-  for (i = 0; i < rich_location::STATICALLY_ALLOCATED_RANGES; i++)
+  for (int i = 0; i < rich_location::STATICALLY_ALLOCATED_RANGES; i++)
     m_source_printing.caret_chars[i] = '^';
   m_show_cwe = false;
   m_show_rules = false;
@@ -326,8 +374,7 @@ diagnostic_context::finish ()
   delete m_file_cache;
   m_file_cache = nullptr;
 
-  XDELETEVEC (m_classify_diagnostic);
-  m_classify_diagnostic = nullptr;
+  m_option_classifier.fini ();
 
   /* diagnostic_context::initialize allocates this->printer using XNEW
      and placement-new.  */
@@ -1052,9 +1099,11 @@ default_diagnostic_finalizer (diagnostic_context *context,
    range.  If OPTION_INDEX is zero, the new setting is for all the
    diagnostics.  */
 diagnostic_t
-diagnostic_context::classify_diagnostic (int option_index,
-					 diagnostic_t new_kind,
-					 location_t where)
+diagnostic_option_classifier::
+classify_diagnostic (const diagnostic_context *context,
+		     int option_index,
+		     diagnostic_t new_kind,
+		     location_t where)
 {
   diagnostic_t old_kind;
 
@@ -1074,10 +1123,10 @@ diagnostic_context::classify_diagnostic (int option_index,
       /* Record the command-line status, so we can reset it back on DK_POP. */
       if (old_kind == DK_UNSPECIFIED)
 	{
-	  old_kind = !m_option_enabled (option_index,
-					m_lang_mask,
-					m_option_state)
-	    ? DK_IGNORED : (m_warning_as_error_requested
+	  old_kind = !context->m_option_enabled (option_index,
+						 context->m_lang_mask,
+						 context->m_option_state)
+	    ? DK_IGNORED : (context->warning_as_error_requested_p ()
 			    ? DK_ERROR : DK_WARNING);
 	  m_classify_diagnostic[option_index] = old_kind;
 	}
@@ -1104,37 +1153,6 @@ diagnostic_context::classify_diagnostic (int option_index,
   return old_kind;
 }
 
-/* Save all diagnostic classifications in a stack.  */
-void
-diagnostic_context::push_diagnostics (location_t where ATTRIBUTE_UNUSED)
-{
-  m_push_list = (int *) xrealloc (m_push_list, (m_n_push + 1) * sizeof (int));
-  m_push_list[m_n_push ++] = m_n_classification_history;
-}
-
-/* Restore the topmost classification set off the stack.  If the stack
-   is empty, revert to the state based on command line parameters.  */
-void
-diagnostic_context::pop_diagnostics (location_t where)
-{
-  int jump_to;
-  int i;
-
-  if (m_n_push)
-    jump_to = m_push_list [-- m_n_push];
-  else
-    jump_to = 0;
-
-  i = m_n_classification_history;
-  m_classification_history =
-    (diagnostic_classification_change_t *) xrealloc (m_classification_history, (i + 1)
-						     * sizeof (diagnostic_classification_change_t));
-  m_classification_history[i].location = where;
-  m_classification_history[i].option = jump_to;
-  m_classification_history[i].kind = DK_POP;
-  m_n_classification_history ++;
-}
-
 /* Helper function for print_parseable_fixits.  Print TEXT to PP, obeying the
    escaping rules for -fdiagnostics-parseable-fixits.  */
 
@@ -1246,14 +1264,14 @@ diagnostic_context::get_any_inlining_info (diagnostic_info *diagnostic)
 /* Update the kind of DIAGNOSTIC based on its location(s), including
    any of those in its inlining stack, relative to any
      #pragma GCC diagnostic
-   directives recorded within CONTEXT.
+   directives recorded within this object.
 
    Return the new kind of DIAGNOSTIC if it was updated, or DK_UNSPECIFIED
    otherwise.  */
 
 diagnostic_t
-diagnostic_context::
-update_effective_level_from_pragmas (diagnostic_info *diagnostic)
+diagnostic_option_classifier::
+update_effective_level_from_pragmas (diagnostic_info *diagnostic) const
 {
   if (m_n_classification_history <= 0)
     return DK_UNSPECIFIED;
@@ -1444,14 +1462,14 @@ diagnostic_context::diagnostic_enabled (diagnostic_info *diagnostic)
     return false;
 
   /* This tests for #pragma diagnostic changes.  */
-  diagnostic_t diag_class = update_effective_level_from_pragmas (diagnostic);
+  diagnostic_t diag_class
+    = m_option_classifier.update_effective_level_from_pragmas (diagnostic);
 
   /* This tests if the user provided the appropriate -Werror=foo
      option.  */
   if (diag_class == DK_UNSPECIFIED
-      && (m_classify_diagnostic[diagnostic->option_index]
-	  != DK_UNSPECIFIED))
-    diagnostic->kind = m_classify_diagnostic[diagnostic->option_index];
+      && !option_unspecified_p (diagnostic->option_index))
+    diagnostic->kind = m_option_classifier.get_current_override (diagnostic->option_index);
 
   /* This allows for future extensions, like temporarily disabling
      warnings for ranges of source code.  */
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index f9950ec2cf8..b83cdeb35c1 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -168,16 +168,6 @@ struct diagnostic_info
   } m_iinfo;
 };
 
-/* Each time a diagnostic's classification is changed with a pragma,
-   we record the change and the location of the change in an array of
-   these structs.  */
-struct diagnostic_classification_change_t
-{
-  location_t location;
-  int option;
-  diagnostic_t kind;
-};
-
 /*  Forward declarations.  */
 typedef void (*diagnostic_starter_fn) (diagnostic_context *,
 				       diagnostic_info *);
@@ -240,6 +230,79 @@ public:
   void on_diagram (const diagnostic_diagram &diagram) override;
 };
 
+/* A stack of sets of classifications: each entry in the stack is
+   a mapping from option index to diagnostic severity that can be changed
+   via pragmas.  The stack can be pushed and popped.  */
+
+class diagnostic_option_classifier
+{
+public:
+  void init (int n_opts);
+  void fini ();
+
+  /* Save all diagnostic classifications in a stack.  */
+  void push ();
+
+  /* Restore the topmost classification set off the stack.  If the stack
+     is empty, revert to the state based on command line parameters.  */
+  void pop (location_t where);
+
+  bool option_unspecified_p (int opt) const
+  {
+    return get_current_override (opt) == DK_UNSPECIFIED;
+  }
+
+  diagnostic_t get_current_override (int opt) const
+  {
+    gcc_assert (opt < m_n_opts);
+    return m_classify_diagnostic[opt];
+  }
+
+  diagnostic_t
+  classify_diagnostic (const diagnostic_context *context,
+		       int option_index,
+		       diagnostic_t new_kind,
+		       location_t where);
+
+  diagnostic_t
+  update_effective_level_from_pragmas (diagnostic_info *diagnostic) const;
+
+private:
+  /* Each time a diagnostic's classification is changed with a pragma,
+     we record the change and the location of the change in an array of
+     these structs.  */
+  struct diagnostic_classification_change_t
+  {
+    location_t location;
+    int option;
+    diagnostic_t kind;
+  };
+
+  int m_n_opts;
+
+  /* For each option index that can be passed to warning() et al
+     (OPT_* from options.h when using this code with the core GCC
+     options), this array may contain a new kind that the diagnostic
+     should be changed to before reporting, or DK_UNSPECIFIED to leave
+     it as the reported kind, or DK_IGNORED to not report it at
+     all.  */
+  diagnostic_t *m_classify_diagnostic;
+
+  /* History of all changes to the classifications above.  This list
+     is stored in location-order, so we can search it, either
+     binary-wise or end-to-front, to find the most recent
+     classification for a given diagnostic, given the location of the
+     diagnostic.  */
+  diagnostic_classification_change_t *m_classification_history;
+
+  /* The size of the above array.  */
+  int m_n_classification_history;
+
+  /* For pragma push/pop.  */
+  int *m_push_list;
+  int m_n_push;
+};
+
 /* This data structure bundles altogether any information relevant to
    the context of a diagnostic message.  */
 class diagnostic_context
@@ -273,8 +336,7 @@ public:
 
   bool option_unspecified_p (int opt) const
   {
-    gcc_assert (opt < m_n_opts);
-    return m_classify_diagnostic[opt] == DK_UNSPECIFIED;
+    return m_option_classifier.option_unspecified_p (opt);
   }
 
   bool report_diagnostic (diagnostic_info *);
@@ -287,9 +349,22 @@ public:
   diagnostic_t
   classify_diagnostic (int option_index,
 		       diagnostic_t new_kind,
-		       location_t where);
-  void push_diagnostics (location_t where ATTRIBUTE_UNUSED);
-  void pop_diagnostics (location_t where);
+		       location_t where)
+  {
+    return m_option_classifier.classify_diagnostic (this,
+						    option_index,
+						    new_kind,
+						    where);
+  }
+
+  void push_diagnostics (location_t where ATTRIBUTE_UNUSED)
+  {
+    m_option_classifier.push ();
+  }
+  void pop_diagnostics (location_t where)
+  {
+    m_option_classifier.pop (where);
+  }
 
   void emit_diagram (const diagnostic_diagram &diagram);
 
@@ -376,9 +451,6 @@ private:
   bool diagnostic_enabled (diagnostic_info *diagnostic);
 
   void get_any_inlining_info (diagnostic_info *diagnostic);
-  diagnostic_t
-  update_effective_level_from_pragmas (diagnostic_info *diagnostic);
-
 
   /* Data members.
      Ideally, all of these would be private and have "m_" prefixes.  */
@@ -401,27 +473,8 @@ private:
      al.  */
   int m_n_opts;
 
-  /* For each option index that can be passed to warning() et al
-     (OPT_* from options.h when using this code with the core GCC
-     options), this array may contain a new kind that the diagnostic
-     should be changed to before reporting, or DK_UNSPECIFIED to leave
-     it as the reported kind, or DK_IGNORED to not report it at
-     all.  */
-  diagnostic_t *m_classify_diagnostic;
-
-  /* History of all changes to the classifications above.  This list
-     is stored in location-order, so we can search it, either
-     binary-wise or end-to-front, to find the most recent
-     classification for a given diagnostic, given the location of the
-     diagnostic.  */
-  diagnostic_classification_change_t *m_classification_history;
-
-  /* The size of the above array.  */
-  int m_n_classification_history;
-
-  /* For pragma push/pop.  */
-  int *m_push_list;
-  int m_n_push;
+  /* The stack of sets of overridden diagnostic option severities.  */
+  diagnostic_option_classifier m_option_classifier;
 
   /* True if we should print any CWE identifiers associated with
      diagnostics.  */
-- 
2.26.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pushed 4/4] diagnostics: split out struct diagnostic_source_printing_options
  2023-11-06 19:49 [pushed 1/4] diagnostics: eliminate diagnostic_kind_count David Malcolm
  2023-11-06 19:49 ` [pushed 2/4] diagnostics: make diagnostic_context::m_urlifier private David Malcolm
  2023-11-06 19:49 ` [pushed 3/4] diagnostics: introduce class diagnostic_option_classifier David Malcolm
@ 2023-11-06 19:49 ` David Malcolm
  2 siblings, 0 replies; 4+ messages in thread
From: David Malcolm @ 2023-11-06 19:49 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

This patch removes almost all use of diagnostic_context from the
source-printing code.

No functional change intended.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-5169-g54da47f9459890.

gcc/ChangeLog:
	* diagnostic-show-locus.cc (class colorizer): Take just a
	pretty_printer rather than a diagnostic_context.
	(layout::layout): Make context param a const reference,
	and pretty_printer param non-optional.
	(layout::m_context): Drop field.
	(layout::m_options): New field.
	(layout::m_colorize_source_p): Drop field.
	(layout::m_show_labels_p): Drop field.
	(layout::m_show_line_numbers_p): Drop field.
	(layout::print_gap_in_line_numbering): Use m_options.
	(layout::calculate_line_spans): Likewise.
	(layout::calculate_linenum_width): Likewise.
	(layout::calculate_x_offset_display): Likewise.
	(layout::print_source_line): Likewise.
	(layout::start_annotation_line): Likewise.
	(layout::print_annotation_line): Likewise.
	(layout::print_line): Likewise.
	(gcc_rich_location::add_location_if_nearby): Update for changes to
	layout ctor.
	(diagnostic_show_locus): Likewise.
	(selftest::test_offset_impl): Likewise.
	(selftest::test_layout_x_offset_display_utf8): Likewise.
	(selftest::test_layout_x_offset_display_tab): Likewise.
	(selftest::test_tab_expansion): Likewise.
	* diagnostic.h (diagnostic_context::m_source_printing): Move
	declaration of struct outside diagnostic_context as...
	(struct diagnostic_source_printing_options)... this.
---
 gcc/diagnostic-show-locus.cc | 94 +++++++++++++++++-------------------
 gcc/diagnostic.h             | 88 ++++++++++++++++-----------------
 2 files changed, 88 insertions(+), 94 deletions(-)

diff --git a/gcc/diagnostic-show-locus.cc b/gcc/diagnostic-show-locus.cc
index d7a471426b9..43523572fe5 100644
--- a/gcc/diagnostic-show-locus.cc
+++ b/gcc/diagnostic-show-locus.cc
@@ -83,7 +83,7 @@ struct point_state
 class colorizer
 {
  public:
-  colorizer (diagnostic_context *context,
+  colorizer (pretty_printer *pp,
 	     diagnostic_t diagnostic_kind);
   ~colorizer ();
 
@@ -113,7 +113,7 @@ class colorizer
   static const int STATE_FIXIT_INSERT  = -2;
   static const int STATE_FIXIT_DELETE  = -3;
 
-  diagnostic_context *m_context;
+  pretty_printer *m_pp;
   diagnostic_t m_diagnostic_kind;
   int m_current_state;
   const char *m_range1;
@@ -365,10 +365,10 @@ struct char_display_policy : public cpp_char_column_policy
 class layout
 {
  public:
-  layout (diagnostic_context *context,
+  layout (const diagnostic_context &context,
 	  rich_location *richloc,
 	  diagnostic_t diagnostic_kind,
-	  pretty_printer *pp = nullptr);
+	  pretty_printer *pp);
 
   bool maybe_add_location_range (const location_range *loc_range,
 				 unsigned original_idx,
@@ -428,15 +428,12 @@ class layout
   move_to_column (int *column, int dest_column, bool add_left_margin);
 
  private:
-  diagnostic_context *m_context;
+  const diagnostic_source_printing_options &m_options;
   pretty_printer *m_pp;
   char_display_policy m_policy;
   location_t m_primary_loc;
   exploc_with_display_col m_exploc;
   colorizer m_colorizer;
-  bool m_colorize_source_p;
-  bool m_show_labels_p;
-  bool m_show_line_numbers_p;
   bool m_diagnostic_path_p;
   auto_vec <layout_range> m_layout_ranges;
   auto_vec <const fixit_hint *> m_fixit_hints;
@@ -451,9 +448,9 @@ class layout
 /* The constructor for "colorizer".  Lookup and store color codes for the
    different kinds of things we might need to print.  */
 
-colorizer::colorizer (diagnostic_context *context,
+colorizer::colorizer (pretty_printer *pp,
 		      diagnostic_t diagnostic_kind) :
-  m_context (context),
+  m_pp (pp),
   m_diagnostic_kind (diagnostic_kind),
   m_current_state (STATE_NORMAL_TEXT)
 {
@@ -461,7 +458,7 @@ colorizer::colorizer (diagnostic_context *context,
   m_range2 = get_color_by_name ("range2");
   m_fixit_insert = get_color_by_name ("fixit-insert");
   m_fixit_delete = get_color_by_name ("fixit-delete");
-  m_stop_color = colorize_stop (pp_show_color (context->printer));
+  m_stop_color = colorize_stop (pp_show_color (m_pp));
 }
 
 /* The destructor for "colorize".  If colorization is on, print a code to
@@ -497,35 +494,35 @@ colorizer::begin_state (int state)
       break;
 
     case STATE_FIXIT_INSERT:
-      pp_string (m_context->printer, m_fixit_insert);
+      pp_string (m_pp, m_fixit_insert);
       break;
 
     case STATE_FIXIT_DELETE:
-      pp_string (m_context->printer, m_fixit_delete);
+      pp_string (m_pp, m_fixit_delete);
       break;
 
     case 0:
       /* Make range 0 be the same color as the "kind" text
 	 (error vs warning vs note).  */
       pp_string
-	(m_context->printer,
-	 colorize_start (pp_show_color (m_context->printer),
+	(m_pp,
+	 colorize_start (pp_show_color (m_pp),
 			 diagnostic_get_color_for_kind (m_diagnostic_kind)));
       break;
 
     case 1:
-      pp_string (m_context->printer, m_range1);
+      pp_string (m_pp, m_range1);
       break;
 
     case 2:
-      pp_string (m_context->printer, m_range2);
+      pp_string (m_pp, m_range2);
       break;
 
     default:
       /* For ranges beyond 2, alternate between color 1 and color 2.  */
       {
 	gcc_assert (state > 2);
-	pp_string (m_context->printer,
+	pp_string (m_pp,
 		   state % 2 ? m_range1 : m_range2);
       }
       break;
@@ -538,7 +535,7 @@ void
 colorizer::finish_state (int state)
 {
   if (state != STATE_NORMAL_TEXT)
-    pp_string (m_context->printer, m_stop_color);
+    pp_string (m_pp, m_stop_color);
 }
 
 /* Get the color code for NAME (or the empty string if
@@ -547,7 +544,7 @@ colorizer::finish_state (int state)
 const char *
 colorizer::get_color_by_name (const char *name)
 {
-  return colorize_start (pp_show_color (m_context->printer), name);
+  return colorize_start (pp_show_color (m_pp), name);
 }
 
 /* Implementation of class layout_range.  */
@@ -1182,20 +1179,17 @@ make_policy (const diagnostic_context &dc,
    Determine m_x_offset_display, to ensure that the primary caret
    will fit within the max_width provided by the diagnostic_context.  */
 
-layout::layout (diagnostic_context * context,
+layout::layout (const diagnostic_context &context,
 		rich_location *richloc,
 		diagnostic_t diagnostic_kind,
 		pretty_printer *pp)
-: m_context (context),
-  m_pp (pp ? pp : context->printer),
-  m_policy (make_policy (*context, *richloc)),
+: m_options (context.m_source_printing),
+  m_pp (pp ? pp : context.printer),
+  m_policy (make_policy (context, *richloc)),
   m_primary_loc (richloc->get_range (0)->m_loc),
   m_exploc (richloc->get_expanded_location (0), m_policy,
 	    LOCATION_ASPECT_CARET),
-  m_colorizer (context, diagnostic_kind),
-  m_colorize_source_p (context->m_source_printing.colorize_source_p),
-  m_show_labels_p (context->m_source_printing.show_labels_p),
-  m_show_line_numbers_p (context->m_source_printing.show_line_numbers_p),
+  m_colorizer (m_pp, diagnostic_kind),
   m_diagnostic_path_p (diagnostic_kind == DK_DIAGNOSTIC_PATH),
   m_layout_ranges (richloc->get_num_locations ()),
   m_fixit_hints (richloc->get_num_fixit_hints ()),
@@ -1229,8 +1223,8 @@ layout::layout (diagnostic_context * context,
   calculate_linenum_width ();
   calculate_x_offset_display ();
 
-  if (context->m_source_printing.show_ruler_p)
-    show_ruler (m_x_offset_display + m_context->m_source_printing.max_width);
+  if (m_options.show_ruler_p)
+    show_ruler (m_x_offset_display + m_options.max_width);
 }
 
 
@@ -1363,7 +1357,7 @@ layout::will_show_line_p (linenum_type row) const
 void
 layout::print_gap_in_line_numbering ()
 {
-  gcc_assert (m_show_line_numbers_p);
+  gcc_assert (m_options.show_line_numbers_p);
 
   pp_emit_prefix (m_pp);
 
@@ -1546,7 +1540,7 @@ layout::calculate_line_spans ()
       line_span *current = &m_line_spans[m_line_spans.length () - 1];
       const line_span *next = &tmp_spans[i];
       gcc_assert (next->m_first_line >= current->m_first_line);
-      const int merger_distance = m_show_line_numbers_p ? 1 : 0;
+      const int merger_distance = m_options.show_line_numbers_p ? 1 : 0;
       if ((linenum_arith_t)next->m_first_line
 	  <= (linenum_arith_t)current->m_last_line + 1 + merger_distance)
 	{
@@ -1595,8 +1589,7 @@ layout::calculate_linenum_width ()
     m_linenum_width = MAX (m_linenum_width, 3);
   /* If there's a minimum margin width, apply it (subtracting 1 for the space
      after the line number.  */
-  m_linenum_width = MAX (m_linenum_width,
-			 m_context->m_source_printing.min_margin_width - 1);
+  m_linenum_width = MAX (m_linenum_width, m_options.min_margin_width - 1);
 }
 
 /* Calculate m_x_offset_display, which improves readability in case the source
@@ -1610,7 +1603,7 @@ layout::calculate_x_offset_display ()
 {
   m_x_offset_display = 0;
 
-  const int max_width = m_context->m_source_printing.max_width;
+  const int max_width = m_options.max_width;
   if (!max_width)
     {
       /* Nothing to do, the width is not capped.  */
@@ -1644,7 +1637,7 @@ layout::calculate_x_offset_display ()
      with a space.  */
   const int source_display_cols = eol_display_column;
   int left_margin_size = 1;
-  if (m_show_line_numbers_p)
+  if (m_options.show_line_numbers_p)
       left_margin_size = m_linenum_width + 3;
   caret_display_column += left_margin_size;
   eol_display_column += left_margin_size;
@@ -1691,7 +1684,7 @@ layout::print_source_line (linenum_type row, const char *line, int line_bytes)
   m_colorizer.set_normal_text ();
 
   pp_emit_prefix (m_pp);
-  if (m_show_line_numbers_p)
+  if (m_options.show_line_numbers_p)
     {
       int width = num_digits (row);
       for (int i = 0; i < m_linenum_width - width; i++)
@@ -1737,7 +1730,7 @@ layout::print_source_line (linenum_type row, const char *line, int line_bytes)
 	 For frontends that only generate carets, we don't colorize the
 	 characters above them, since this would look strange (e.g.
 	 colorizing just the first character in a token).  */
-      if (m_colorize_source_p)
+      if (m_options.colorize_source_p)
 	{
 	  bool in_range_p;
 	  point_state state;
@@ -1809,7 +1802,7 @@ void
 layout::start_annotation_line (char margin_char) const
 {
   pp_emit_prefix (m_pp);
-  if (m_show_line_numbers_p)
+  if (m_options.show_line_numbers_p)
     {
       /* Print the margin.  If MARGIN_CHAR != ' ', then print up to 3
 	 of it, right-aligned, padded with spaces.  */
@@ -1852,8 +1845,7 @@ layout::print_annotation_line (linenum_type row, const line_bounds lbounds)
 	      /* Draw the caret.  */
 	      char caret_char;
 	      if (state.range_idx < rich_location::STATICALLY_ALLOCATED_RANGES)
-		caret_char
-		  = m_context->m_source_printing.caret_chars[state.range_idx];
+		caret_char = m_options.caret_chars[state.range_idx];
 	      else
 		caret_char = '^';
 	      pp_character (m_pp, caret_char);
@@ -2796,7 +2788,7 @@ layout::print_line (linenum_type row)
     = print_source_line (row, line.get_buffer (), line.length ());
   if (should_print_annotation_line_p (row))
     print_annotation_line (row, lbounds);
-  if (m_show_labels_p)
+  if (m_options.show_labels_p)
     print_any_labels (row);
   print_trailing_fixits (row);
 }
@@ -2816,7 +2808,7 @@ gcc_rich_location::add_location_if_nearby (location_t loc,
   /* Use the layout location-handling logic to sanitize LOC,
      filtering it to the current line spans within a temporary
      layout instance.  */
-  layout layout (global_dc, this, DK_ERROR);
+  layout layout (*global_dc, this, DK_ERROR, nullptr);
   location_range loc_range;
   loc_range.m_loc = loc;
   loc_range.m_range_display_kind = SHOW_RANGE_WITHOUT_CARET;
@@ -2857,7 +2849,7 @@ diagnostic_show_locus (diagnostic_context * context,
 
   context->m_last_location = loc;
 
-  layout layout (context, richloc, diagnostic_kind, pp);
+  layout layout (*context, richloc, diagnostic_kind, pp);
   for (int line_span_idx = 0; line_span_idx < layout.get_num_line_spans ();
        line_span_idx++)
     {
@@ -2969,7 +2961,7 @@ test_offset_impl (int caret_byte_col, int max_width,
   rich_location richloc (line_table,
 			 linemap_position_for_column (line_table,
 						      caret_byte_col));
-  layout test_layout (&dc, &richloc, DK_ERROR);
+  layout test_layout (dc, &richloc, DK_ERROR, nullptr);
   ASSERT_EQ (left_margin - test_linenum_sep,
 	     test_layout.get_linenum_width ());
   ASSERT_EQ (expected_x_offset_display,
@@ -3084,7 +3076,7 @@ test_layout_x_offset_display_utf8 (const line_table_case &case_)
     rich_location richloc (line_table,
 			   linemap_position_for_column (line_table,
 							emoji_col));
-    layout test_layout (&dc, &richloc, DK_ERROR);
+    layout test_layout (dc, &richloc, DK_ERROR, nullptr);
     test_layout.print_line (1);
     ASSERT_STREQ ("     |         1         \n"
 		  "     |         1         \n"
@@ -3109,7 +3101,7 @@ test_layout_x_offset_display_utf8 (const line_table_case &case_)
     rich_location richloc (line_table,
 			   linemap_position_for_column (line_table,
 							emoji_col + 2));
-    layout test_layout (&dc, &richloc, DK_ERROR);
+    layout test_layout (dc, &richloc, DK_ERROR, nullptr);
     test_layout.print_line (1);
     ASSERT_STREQ ("     |        1         1 \n"
 		  "     |        1         2 \n"
@@ -3186,7 +3178,7 @@ test_layout_x_offset_display_tab (const line_table_case &case_)
     {
       test_diagnostic_context dc;
       dc.m_tabstop = tabstop;
-      layout test_layout (&dc, &richloc, DK_ERROR);
+      layout test_layout (dc, &richloc, DK_ERROR, nullptr);
       test_layout.print_line (1);
       const char *out = pp_formatted_text (dc.printer);
       ASSERT_EQ (NULL, strchr (out, '\t'));
@@ -3209,7 +3201,7 @@ test_layout_x_offset_display_tab (const line_table_case &case_)
       dc.m_source_printing.min_margin_width
 	= test_left_margin - test_linenum_sep + 1;
       dc.m_source_printing.show_line_numbers_p = true;
-      layout test_layout (&dc, &richloc, DK_ERROR);
+      layout test_layout (dc, &richloc, DK_ERROR, nullptr);
       test_layout.print_line (1);
 
       /* We have arranged things so that two columns will be printed before
@@ -5521,7 +5513,7 @@ test_tab_expansion (const line_table_case &case_)
     rich_location richloc (line_table,
 			   linemap_position_for_column (line_table,
 							first_non_ws_byte_col));
-    layout test_layout (&dc, &richloc, DK_ERROR);
+    layout test_layout (dc, &richloc, DK_ERROR, nullptr);
     test_layout.print_line (1);
     ASSERT_STREQ ("            This: `      ' is a tab.\n"
 		  "            ^\n",
@@ -5536,7 +5528,7 @@ test_tab_expansion (const line_table_case &case_)
     rich_location richloc (line_table,
 			   linemap_position_for_column (line_table,
 							right_quote_byte_col));
-    layout test_layout (&dc, &richloc, DK_ERROR);
+    layout test_layout (dc, &richloc, DK_ERROR, nullptr);
     test_layout.print_line (1);
     ASSERT_STREQ ("            This: `      ' is a tab.\n"
 		  "                         ^\n",
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index b83cdeb35c1..58341cec308 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -303,6 +303,50 @@ private:
   int m_n_push;
 };
 
+/* A bundle of options relating to printing the user's source code
+   (potentially with a margin, underlining, labels, etc).  */
+
+struct diagnostic_source_printing_options
+{
+  /* True if we should print the source line with a caret indicating
+     the location.
+     Corresponds to -fdiagnostics-show-caret.  */
+  bool enabled;
+
+  /* Maximum width of the source line printed.  */
+  int max_width;
+
+  /* Character used at the caret when printing source locations.  */
+  char caret_chars[rich_location::STATICALLY_ALLOCATED_RANGES];
+
+  /* When printing source code, should the characters at carets and ranges
+     be colorized? (assuming colorization is on at all).
+     This should be true for frontends that generate range information
+     (so that the ranges of code are colorized),
+     and false for frontends that merely specify points within the
+     source code (to avoid e.g. colorizing just the first character in
+     a token, which would look strange).  */
+  bool colorize_source_p;
+
+  /* When printing source code, should labelled ranges be printed?
+     Corresponds to -fdiagnostics-show-labels.  */
+  bool show_labels_p;
+
+  /* When printing source code, should there be a left-hand margin
+     showing line numbers?
+     Corresponds to -fdiagnostics-show-line-numbers.  */
+  bool show_line_numbers_p;
+
+  /* If printing source code, what should the minimum width of the margin
+     be?  Line numbers will be right-aligned, and padded to this width.
+     Corresponds to -fdiagnostics-minimum-margin-width=VALUE.  */
+  int min_margin_width;
+
+  /* Usable by plugins; if true, print a debugging ruler above the
+     source output.  */
+  bool show_ruler_p;
+};
+
 /* This data structure bundles altogether any information relevant to
    the context of a diagnostic message.  */
 class diagnostic_context
@@ -601,49 +645,7 @@ public:
 
   bool m_inhibit_notes_p;
 
-  /* Fields relating to printing the user's source code (potentially with
-     a margin, underlining, labels, etc).  */
-  struct {
-
-    /* True if we should print the source line with a caret indicating
-       the location.
-       Corresponds to -fdiagnostics-show-caret.  */
-    bool enabled;
-
-    /* Maximum width of the source line printed.  */
-    int max_width;
-
-    /* Character used at the caret when printing source locations.  */
-    char caret_chars[rich_location::STATICALLY_ALLOCATED_RANGES];
-
-    /* When printing source code, should the characters at carets and ranges
-       be colorized? (assuming colorization is on at all).
-       This should be true for frontends that generate range information
-       (so that the ranges of code are colorized),
-       and false for frontends that merely specify points within the
-       source code (to avoid e.g. colorizing just the first character in
-       a token, which would look strange).  */
-    bool colorize_source_p;
-
-    /* When printing source code, should labelled ranges be printed?
-       Corresponds to -fdiagnostics-show-labels.  */
-    bool show_labels_p;
-
-    /* When printing source code, should there be a left-hand margin
-       showing line numbers?
-       Corresponds to -fdiagnostics-show-line-numbers.  */
-    bool show_line_numbers_p;
-
-    /* If printing source code, what should the minimum width of the margin
-       be?  Line numbers will be right-aligned, and padded to this width.
-       Corresponds to -fdiagnostics-minimum-margin-width=VALUE.  */
-    int min_margin_width;
-
-    /* Usable by plugins; if true, print a debugging ruler above the
-       source output.  */
-    bool show_ruler_p;
-
-  } m_source_printing;
+  diagnostic_source_printing_options m_source_printing;
 
 private:
   /* True if -freport-bug option is used.  */
-- 
2.26.3


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-11-06 19:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-06 19:49 [pushed 1/4] diagnostics: eliminate diagnostic_kind_count David Malcolm
2023-11-06 19:49 ` [pushed 2/4] diagnostics: make diagnostic_context::m_urlifier private David Malcolm
2023-11-06 19:49 ` [pushed 3/4] diagnostics: introduce class diagnostic_option_classifier David Malcolm
2023-11-06 19:49 ` [pushed 4/4] diagnostics: split out struct diagnostic_source_printing_options David Malcolm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).