Re: [PATCH 1/13] v2 [PATCH 1/13] Add support for per-location warning groups (PR 74765)

[Thomas Schwinge <thomas@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 14845 bytes --]

Hi!

On 2021-06-23T13:47:08-0600, Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> On 6/22/21 5:28 PM, David Malcolm wrote:
>> On Tue, 2021-06-22 at 19:18 -0400, David Malcolm wrote:
>>> On Fri, 2021-06-04 at 15:41 -0600, Martin Sebor wrote:
>>>> The attached patch introduces the suppress_warning(),
>>>> warning_suppressed(), and copy_no_warning() APIs [etc.]

Martin, great work on this!

I was a bit surprised to see this key on 'location_t's -- but indeed it
appears to do the right thing.

I now had a bit of a deep dive into some aspects of this, in context of
<https://gcc.gnu.org/PR101574> "gcc/sparseset.h:215:20: error: suggest
parentheses around assignment used as truth value [-Werror=parentheses]"
that I recently filed.  This seems difficult to reproduce, but I'm still
able to reliably reproduce it in one specific build
configuration/directory/machine/whatever.  Initially, we all quickly
assumed that it'd be some GC issue -- but "alas", it's not, at least not
directly.  (But I'll certainly assume that some GC aspects are involved
which make this issue come and go across different GCC sources revisions,
and difficult to reproduce.)

First, two pieces of cleanup:

> --- /dev/null
> +++ b/gcc/warning-control.cc

> +template <class ToType, class FromType>
> +void copy_warning (ToType to, FromType from)
> +{
> +  const key_type_t to_key = convert_to_key (to);
> +
> +  if (nowarn_spec_t *from_map = get_nowarn_spec (from))
> +    {
> +      /* If there's an entry in the map the no-warning bit must be set.  */
> +      gcc_assert (get_no_warning_bit (from));
> +
> +      if (!nowarn_map)
> +     nowarn_map = xint_hash_map_t::create_ggc (32);

OK to push "Simplify 'gcc/diagnostic-spec.h:nowarn_map' setup", see
attached?  If we've just read something from the map, we can be sure that
it exists.  ;-)

> --- /dev/null
> +++ b/gcc/diagnostic-spec.h

> +typedef location_t key_type_t;
> +typedef int_hash <key_type_t, 0, UINT_MAX> xint_hash_t;
> +typedef hash_map<xint_hash_t, nowarn_spec_t> xint_hash_map_t;
> +
> +/* A mapping from the location of an expression to the warning spec
> +   set for it.  */
> +extern GTY(()) xint_hash_map_t *nowarn_map;

More on that data structure setup in a later email; here I'd like to
"Clarify 'key_type_t' to 'location_t' as used for
'gcc/diagnostic-spec.h:nowarn_map'", see attached.  OK to push?  To make
it obvious what exactly the key type is.  No change in behavior.

Why is this relevant?  Via current 'int_hash<key_type_t, 0, UINT_MAX>',
we create a 'int_hash' using "spare" value '0' for 'Empty' marker, and
"spare" value 'UINT_MAX' for 'Deleted' marker.  Now, the latter is
unlikely to ever trigger (but still not correct -- patch in testing), but
the former triggers very much so: value '0' is, per 'gcc/input.h':

    #define UNKNOWN_LOCATION ((location_t) 0)

..., and there are no safe-guards in the code here, so we'll happily put
key 'UNKNOWN_LOCATION' into the 'nowarn_map', and all the
'UNKNOWN_LOCATION' entries share (replace?) one single warning
disposition (problem!), and at the same time that key value is also used
as the 'Empty' marker (problem!).  I have not tried to understand why
this doesn't cause much greater breakage, but propose to fix this as per
the attached "Don't maintain a warning spec for
'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574]".  OK to push?

Leaving aside that for 'UNKNOWN_LOCATION' -- per my understanding, at
least, as per above -- the current implementation isn't doing the right
thing anyway, Richard had in
<http://mid.mail-archive.com/CAFiYyc2am7_pcN4+dvgFSxumnQzYuNRefFWf-zDHiJv-n8EA_w@mail.gmail.com>
toyed with the idea that we for "UNKNOWN_LOCATION create a new location
with the source location being still UNKNOWN but with the appropriate
ad-hoc data to disable the warning".  On the other hand, we have Martin's
initial goal,
<http://mid.mail-archive.com/92db3776-af59-fa20-483b-aa67b17d0751@gmail.com>,
that he'd like to "work toward providing locations for all
expressions/statements".  (I agree -- and get rid of "location wrapper"
nodes at the same time...)  So there certainly is follow-on work to be
done re 'UNKNOWN_LOCATION's, but that's orthogonal to the issue I'm
fixing here.  (Just mentioning all this for context.)

I'm reasonably confident that my changes are doing the right things in
general, but please carefully review, especially here:

  - 'gcc/warning-control.cc:suppress_warning' functions: is it correct to
    conditionalize on '!RESERVED_LOCATION_P' the 'suppress_warning_at'
    calls and 'supp' update?  Or, should instead 'suppress_warning_at'
    handle the case of '!RESERVED_LOCATION_P'?  (How?)

  - 'gcc/diagnostic-spec.c:copy_warning' and
    'gcc/warning-control.cc:copy_warning': is the rationale correct for
    the 'gcc_checking_assert (!from_spec)': "If we cannot set no-warning
    dispositions for 'to', ascertain that we don't have any for 'from'.
    Otherwise, we'd lose these."?  If the rationale is correct, then
    observing that in 'gcc/warning-control.cc:copy_warning' this
    currently "triggers during GCC build" is something to be looked into,
    later, I suppose, and otherwise, how should I change this code?

Gating on 'RESERVED_LOCATION_P' is what other similar code in GCC is
doing.  Conveniently, that frees up the two values
'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' for use as 'Empty'/'Deleted'
markers -- patch in testing.

Plus some more cleanup that fell out during analysis/development --
patches in testing.

Oh, and one of these actually (unintentially so) happens to resolve
<https://gcc.gnu.org/PR101204> "[12 Regression] infinite recursion in
gtype-desc.c since r12-1801-g7036e9ef462fde8181bece4ac4e03f3aa27204dc",
so unless you've done any work on that, may I take over that PR?

What my patches are not addressing is <https://gcc.gnu.org/PR101292>
"[12 Regression] recent valgrind error in warning-control.cc since
r12-1804-g65870e75616ee4359d1c13b99be794e6a577bc65", and with the
'gcc_assert (*from_map != 0xa5a5a5a5);' I'm actually able to trigger
that during GCC build itself.

Are you aware of any other PRs related to this functionality that I
should re-evaluate?


Grüße
 Thomas


PS.  Relevant code quoted for reference, in case that's useful:

> --- /dev/null
> +++ b/gcc/diagnostic-spec.h

> [...]

> +typedef location_t key_type_t;
> +typedef int_hash <key_type_t, 0, UINT_MAX> xint_hash_t;
> +typedef hash_map<xint_hash_t, nowarn_spec_t> xint_hash_map_t;
> +
> +/* A mapping from the location of an expression to the warning spec
> +   set for it.  */
> +extern GTY(()) xint_hash_map_t *nowarn_map;

> [...]

> --- /dev/null
> +++ b/gcc/diagnostic-spec.c

> [...]

> +/* Map from location to its no-warning disposition.  */
> +
> +GTY(()) xint_hash_map_t *nowarn_map;
> +
> +/* Return the no-warning disposition for location LOC and option OPT
> +   or for all/any otions by default.  */
> +
> +bool
> +warning_suppressed_at (location_t loc, opt_code opt /* = all_warnings */)
> +{
> +  if (!nowarn_map)
> +    return false;
> +
> +  if (const nowarn_spec_t* const pspec = nowarn_map->get (loc))
> +    {
> +      const nowarn_spec_t optspec (opt);
> +      return *pspec & optspec;
> +    }
> +
> +  return false;
> +}
> +
> + /* Change the supression of warnings at location LOC.
> +    OPT controls which warnings are affected.
> +    The wildcard OPT of -1 controls all warnings.
> +    If SUPP is true (the default), enable the suppression of the warnings.
> +    If SUPP is false, disable the suppression of the warnings.  */
> +
> +bool
> +suppress_warning_at (location_t loc, opt_code opt /* = all_warnings */,
> +                  bool supp /* = true */)
> +{
> +  const nowarn_spec_t optspec (supp ? opt : opt_code ());
> +
> +  if (nowarn_spec_t *pspec = nowarn_map ? nowarn_map->get (loc) : NULL)
> +    {
> +      if (supp)
> +     {
> +       *pspec |= optspec;
> +       return true;
> +     }
> +
> +      *pspec &= optspec;
> +      if (*pspec)
> +     return true;
> +
> +      nowarn_map->remove (loc);
> +      return false;
> +    }
> +
> +  if (!supp || opt == no_warning)
> +    return false;
> +
> +  if (!nowarn_map)
> +    nowarn_map = xint_hash_map_t::create_ggc (32);
> +
> +  nowarn_map->put (loc, optspec);
> +  return true;
> +}
> +
> +/* Copy the no-warning disposition from one location to another.  */
> +
> +void
> +copy_warning (location_t to, location_t from)
> +{
> +  if (!nowarn_map)
> +    return;
> +
> +  if (nowarn_spec_t *pspec = nowarn_map->get (from))
> +    nowarn_map->put (to, *pspec);
> +  else
> +    nowarn_map->remove (to);
> +}

> --- /dev/null
> +++ b/gcc/warning-control.cc

> [...]

> +/* Return the no-warning bit for EXPR.  */
> +
> +static inline bool
> +get_no_warning_bit (const_tree expr)
> +{
> +  return expr->base.nowarning_flag;
> +}
> +
> +/* Return the no-warning bit for statement STMT.  */
> +
> +static inline bool
> +get_no_warning_bit (const gimple *stmt)
> +{
> +  return stmt->no_warning;
> +}
> +
> +/* Set the no-warning bit for EXPR to VALUE.  */
> +
> +static inline void
> +set_no_warning_bit (tree expr, bool value)
> +{
> +  expr->base.nowarning_flag = value;
> +}
> +
> +/* Set the no-warning bit for statement STMT to VALUE.  */
> +
> +static inline void
> +set_no_warning_bit (gimple *stmt, bool value)
> +{
> +  stmt->no_warning = value;
> +}
> +
> +/* Return EXPR location or zero.  */
> +
> +static inline key_type_t
> +convert_to_key (const_tree expr)
> +{
> +  if (DECL_P (expr))
> +    return DECL_SOURCE_LOCATION (expr);
> +  if (EXPR_P (expr))
> +    return EXPR_LOCATION (expr);
> +  return 0;
> +}
> +
> +/* Return STMT location (may be zero).  */
> +
> +static inline key_type_t
> +convert_to_key (const gimple *stmt)
> +{
> +  return gimple_location (stmt);
> +}
> +
> +/* Return the no-warning bitmap for decl/expression EXPR.  */
> +
> +static nowarn_spec_t *
> +get_nowarn_spec (const_tree expr)
> +{
> +  const key_type_t key = convert_to_key (expr);
> +
> +  if (!get_no_warning_bit (expr) || !key)
> +    return NULL;
> +
> +  return nowarn_map ? nowarn_map->get (key) : NULL;
> +}
> +
> +/* Return the no-warning bitmap for stateemt STMT.  */
> +
> +static nowarn_spec_t *
> +get_nowarn_spec (const gimple *stmt)
> +{
> +  const key_type_t key = convert_to_key (stmt);
> +
> +  if (!get_no_warning_bit (stmt))
> +    return NULL;
> +
> +  return nowarn_map ? nowarn_map->get (key) : NULL;
> +}
> +
> +/* Return true if warning OPT is suppressed for decl/expression EXPR.
> +   By default tests the disposition for any warning.  */
> +
> +bool
> +warning_suppressed_p (const_tree expr, opt_code opt /* = all_warnings */)
> +{
> +  const nowarn_spec_t *spec = get_nowarn_spec (expr);
> +
> +  if (!spec)
> +    return get_no_warning_bit (expr);
> +
> +  const nowarn_spec_t optspec (opt);
> +  bool dis = *spec & optspec;
> +  gcc_assert (get_no_warning_bit (expr) || !dis);
> +  return dis;
> +}
> +
> +/* Return true if warning OPT is suppressed for statement STMT.
> +   By default tests the disposition for any warning.  */
> +
> +bool
> +warning_suppressed_p (const gimple *stmt, opt_code opt /* = all_warnings */)
> +{
> +  const nowarn_spec_t *spec = get_nowarn_spec (stmt);
> +
> +  if (!spec)
> +    /* Fall back on the single no-warning bit.  */
> +    return get_no_warning_bit (stmt);
> +
> +  const nowarn_spec_t optspec (opt);
> +  bool dis = *spec & optspec;
> +  gcc_assert (get_no_warning_bit (stmt) || !dis);
> +  return dis;
> +}
> +
> +/* Enable, or by default disable, a warning for the expression.
> +   The wildcard OPT of -1 controls all warnings.  */
> +
> +void
> +suppress_warning (tree expr, opt_code opt /* = all_warnings */,
> +               bool supp /* = true */)
> +{
> +  if (opt == no_warning)
> +    return;
> +
> +  const key_type_t key = convert_to_key (expr);
> +
> +  supp = suppress_warning_at (key, opt, supp) || supp;
> +  set_no_warning_bit (expr, supp);
> +}
> +
> +/* Enable, or by default disable, a warning for the statement STMT.
> +   The wildcard OPT of -1 controls all warnings.  */
> +
> +void
> +suppress_warning (gimple *stmt, opt_code opt /* = all_warnings */,
> +               bool supp /* = true */)
> +{
> +  if (opt == no_warning)
> +    return;
> +
> +  const key_type_t key = convert_to_key (stmt);
> +
> +  supp = suppress_warning_at (key, opt, supp) || supp;
> +  set_no_warning_bit (stmt, supp);
> +}
> +
> +/* Copy the warning disposition mapping between an expression and/or
> +   a statement.  */
> +
> +template <class ToType, class FromType>
> +void copy_warning (ToType to, FromType from)
> +{
> +  const key_type_t to_key = convert_to_key (to);
> +
> +  if (nowarn_spec_t *from_map = get_nowarn_spec (from))
> +    {
> +      /* If there's an entry in the map the no-warning bit must be set.  */
> +      gcc_assert (get_no_warning_bit (from));
> +
> +      if (!nowarn_map)
> +     nowarn_map = xint_hash_map_t::create_ggc (32);
> +
> +      nowarn_map->put (to_key, *from_map);
> +      set_no_warning_bit (to, true);
> +    }
> +  else
> +    {
> +      if (nowarn_map)
> +     nowarn_map->remove (to_key);
> +
> +      /* The no-warning bit might be set even if there's no entry
> +      in the map.  */
> +      set_no_warning_bit (to, get_no_warning_bit (from));
> +    }
> +}
> +
> +/* Copy the warning disposition mapping from one expression to another.  */
> +
> +void
> +copy_warning (tree to, const_tree from)
> +{
> +  copy_warning<tree, const_tree>(to, from);
> +}
> +
> +/* Copy the warning disposition mapping from a statement to an expression.  */
> +
> +void
> +copy_warning (tree to, const gimple *from)
> +{
> +  copy_warning<tree, const gimple *>(to, from);
> +}
> +
> +/* Copy the warning disposition mapping from an expression to a statement.  */
> +
> +void
> +copy_warning (gimple *to, const_tree from)
> +{
> +  copy_warning<gimple *, const_tree>(to, from);
> +}
> +
> +/* Copy the warning disposition mapping from one statement to another.  */
> +
> +void
> +copy_warning (gimple *to, const gimple *from)
> +{
> +  copy_warning<gimple *, const gimple *>(to, from);
> +}


Grüße
 Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Simplify-gcc-diagnostic-spec.h-nowarn_map-setup.patch --]
[-- Type: text/x-diff, Size: 1025 bytes --]

From 095c16ead5d432726f2b6de5ce12fd367600076d Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 1 Sep 2021 16:48:55 +0200
Subject: [PATCH 1/3] Simplify 'gcc/diagnostic-spec.h:nowarn_map' setup

If we've just read something from the map, we can be sure that it exists.

	gcc/
	* warning-control.cc (copy_warning): Remove 'nowarn_map' setup.
---
 gcc/warning-control.cc | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/gcc/warning-control.cc b/gcc/warning-control.cc
index ec8ed232763..9c506e163d6 100644
--- a/gcc/warning-control.cc
+++ b/gcc/warning-control.cc
@@ -188,9 +188,7 @@ void copy_warning (ToType to, FromType from)
       /* If there's an entry in the map the no-warning bit must be set.  */
       gcc_assert (get_no_warning_bit (from));
 
-      if (!nowarn_map)
-	nowarn_map = xint_hash_map_t::create_ggc (32);
-
+      gcc_checking_assert (nowarn_map);
       nowarn_map->put (to_key, *from_map);
       set_no_warning_bit (to, true);
     }
-- 
2.30.2


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Clarify-key_type_t-to-location_t-as-used-for-gcc-dia.patch --]
[-- Type: text/x-diff, Size: 5439 bytes --]

From 23d9b93401349fca03efaef0fef0960933f4c316 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 31 Aug 2021 22:01:23 +0200
Subject: [PATCH 2/3] Clarify 'key_type_t' to 'location_t' as used for
 'gcc/diagnostic-spec.h:nowarn_map'

To make it obvious what exactly the key type is.  No change in behavior.

	gcc/
	* diagnostic-spec.h (typedef xint_hash_t): Use 'location_t' instead of...
	(typedef key_type_t): ... this.  Remove.
	(nowarn_map): Document.
	* diagnostic-spec.c (nowarn_map): Likewise.
	* warning-control.cc (convert_to_key): Evolve functions into...
	(get_location): ... these.  Adjust all users.
---
 gcc/diagnostic-spec.c  |  2 +-
 gcc/diagnostic-spec.h  |  6 ++----
 gcc/warning-control.cc | 41 ++++++++++++++++++++++-------------------
 3 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/gcc/diagnostic-spec.c b/gcc/diagnostic-spec.c
index 5e961e1b9f9..eac5a3317c8 100644
--- a/gcc/diagnostic-spec.c
+++ b/gcc/diagnostic-spec.c
@@ -105,7 +105,7 @@ nowarn_spec_t::nowarn_spec_t (opt_code opt)
     }
 }
 
-/* Map from location to its no-warning disposition.  */
+/* A mapping from a 'location_t' to the warning spec set for it.  */
 
 GTY(()) xint_hash_map_t *nowarn_map;
 
diff --git a/gcc/diagnostic-spec.h b/gcc/diagnostic-spec.h
index 4e4d260f74a..9b3aaaa3ce6 100644
--- a/gcc/diagnostic-spec.h
+++ b/gcc/diagnostic-spec.h
@@ -130,12 +130,10 @@ operator!= (const nowarn_spec_t &lhs, const nowarn_spec_t &rhs)
   return !(lhs == rhs);
 }
 
-typedef location_t key_type_t;
-typedef int_hash <key_type_t, 0, UINT_MAX> xint_hash_t;
+typedef int_hash <location_t, 0, UINT_MAX> xint_hash_t;
 typedef hash_map<xint_hash_t, nowarn_spec_t> xint_hash_map_t;
 
-/* A mapping from the location of an expression to the warning spec
-   set for it.  */
+/* A mapping from a 'location_t' to the warning spec set for it.  */
 extern GTY(()) xint_hash_map_t *nowarn_map;
 
 #endif // DIAGNOSTIC_SPEC_H_INCLUDED
diff --git a/gcc/warning-control.cc b/gcc/warning-control.cc
index 9c506e163d6..8d6c0828445 100644
--- a/gcc/warning-control.cc
+++ b/gcc/warning-control.cc
@@ -62,22 +62,22 @@ set_no_warning_bit (gimple *stmt, bool value)
   stmt->no_warning = value;
 }
 
-/* Return EXPR location or zero.  */
+/* Return EXPR location or 'UNKNOWN_LOCATION'.  */
 
-static inline key_type_t
-convert_to_key (const_tree expr)
+static inline location_t
+get_location (const_tree expr)
 {
   if (DECL_P (expr))
     return DECL_SOURCE_LOCATION (expr);
   if (EXPR_P (expr))
     return EXPR_LOCATION (expr);
-  return 0;
+  return UNKNOWN_LOCATION;
 }
 
-/* Return STMT location (may be zero).  */
+/* Return STMT location (may be 'UNKNOWN_LOCATION').  */
 
-static inline key_type_t
-convert_to_key (const gimple *stmt)
+static inline location_t
+get_location (const gimple *stmt)
 {
   return gimple_location (stmt);
 }
@@ -87,12 +87,15 @@ convert_to_key (const gimple *stmt)
 static nowarn_spec_t *
 get_nowarn_spec (const_tree expr)
 {
-  const key_type_t key = convert_to_key (expr);
+  const location_t loc = get_location (expr);
 
-  if (!get_no_warning_bit (expr) || !key)
+  if (loc == UNKNOWN_LOCATION)
     return NULL;
 
-  return nowarn_map ? nowarn_map->get (key) : NULL;
+  if (!get_no_warning_bit (expr))
+    return NULL;
+
+  return nowarn_map ? nowarn_map->get (loc) : NULL;
 }
 
 /* Return the no-warning bitmap for stateemt STMT.  */
@@ -100,12 +103,12 @@ get_nowarn_spec (const_tree expr)
 static nowarn_spec_t *
 get_nowarn_spec (const gimple *stmt)
 {
-  const key_type_t key = convert_to_key (stmt);
+  const location_t loc = get_location (stmt);
 
   if (!get_no_warning_bit (stmt))
     return NULL;
 
-  return nowarn_map ? nowarn_map->get (key) : NULL;
+  return nowarn_map ? nowarn_map->get (loc) : NULL;
 }
 
 /* Return true if warning OPT is suppressed for decl/expression EXPR.
@@ -153,9 +156,9 @@ suppress_warning (tree expr, opt_code opt /* = all_warnings */,
   if (opt == no_warning)
     return;
 
-  const key_type_t key = convert_to_key (expr);
+  const location_t loc = get_location (expr);
 
-  supp = suppress_warning_at (key, opt, supp) || supp;
+  supp = suppress_warning_at (loc, opt, supp) || supp;
   set_no_warning_bit (expr, supp);
 }
 
@@ -169,9 +172,9 @@ suppress_warning (gimple *stmt, opt_code opt /* = all_warnings */,
   if (opt == no_warning)
     return;
 
-  const key_type_t key = convert_to_key (stmt);
+  const location_t loc = get_location (stmt);
 
-  supp = suppress_warning_at (key, opt, supp) || supp;
+  supp = suppress_warning_at (loc, opt, supp) || supp;
   set_no_warning_bit (stmt, supp);
 }
 
@@ -181,7 +184,7 @@ suppress_warning (gimple *stmt, opt_code opt /* = all_warnings */,
 template <class ToType, class FromType>
 void copy_warning (ToType to, FromType from)
 {
-  const key_type_t to_key = convert_to_key (to);
+  const location_t to_loc = get_location (to);
 
   if (nowarn_spec_t *from_map = get_nowarn_spec (from))
     {
@@ -189,13 +192,13 @@ void copy_warning (ToType to, FromType from)
       gcc_assert (get_no_warning_bit (from));
 
       gcc_checking_assert (nowarn_map);
-      nowarn_map->put (to_key, *from_map);
+      nowarn_map->put (to_loc, *from_map);
       set_no_warning_bit (to, true);
     }
   else
     {
       if (nowarn_map)
-	nowarn_map->remove (to_key);
+	nowarn_map->remove (to_loc);
 
       /* The no-warning bit might be set even if there's no entry
 	 in the map.  */
-- 
2.30.2


[-- Attachment #4: 0003-Don-t-maintain-a-warning-spec-for-UNKNOWN_LOCATION-B.patch --]
[-- Type: text/x-diff, Size: 6034 bytes --]

From 79d636fadf43a1ae62e30013f868521a8a0529f5 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Mon, 30 Aug 2021 22:36:47 +0200
Subject: [PATCH 3/3] Don't maintain a warning spec for
 'UNKNOWN_LOCATION'/'BUILTINS_LOCATION' [PR101574]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This resolves PR101574 "gcc/sparseset.h:215:20: error: suggest parentheses
around assignment used as truth value [-Werror=parentheses]", as (bogusly)
reported at commit a61f6afbee370785cf091fe46e2e022748528307:

    In file included from [...]/source-gcc/gcc/lra-lives.c:43:
    [...]/source-gcc/gcc/lra-lives.c: In function ‘void make_hard_regno_dead(int)’:
    [...]/source-gcc/gcc/sparseset.h:215:20: error: suggest parentheses around assignment used as truth value [-Werror=parentheses]
      215 |        && (((ITER) = sparseset_iter_elm (SPARSESET)) || 1);             \
          |            ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    [...]/source-gcc/gcc/lra-lives.c:304:3: note: in expansion of macro ‘EXECUTE_IF_SET_IN_SPARSESET’
      304 |   EXECUTE_IF_SET_IN_SPARSESET (pseudos_live, i)
          |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~

	gcc/
	PR bootstrap/101574
	* diagnostic-spec.c (warning_suppressed_at, copy_warning): Handle
	'RESERVED_LOCATION_P' locations.
	* warning-control.cc (get_nowarn_spec, suppress_warning)
	(copy_warning): Likewise.
---
 gcc/diagnostic-spec.c  | 24 +++++++++++++++++---
 gcc/warning-control.cc | 50 ++++++++++++++++++++++++++++--------------
 2 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/gcc/diagnostic-spec.c b/gcc/diagnostic-spec.c
index eac5a3317c8..76b5d5e57f5 100644
--- a/gcc/diagnostic-spec.c
+++ b/gcc/diagnostic-spec.c
@@ -115,6 +115,8 @@ GTY(()) xint_hash_map_t *nowarn_map;
 bool
 warning_suppressed_at (location_t loc, opt_code opt /* = all_warnings */)
 {
+  gcc_checking_assert (!RESERVED_LOCATION_P (loc));
+
   if (!nowarn_map)
     return false;
 
@@ -137,6 +139,8 @@ bool
 suppress_warning_at (location_t loc, opt_code opt /* = all_warnings */,
 		     bool supp /* = true */)
 {
+  gcc_checking_assert (!RESERVED_LOCATION_P (loc));
+
   const nowarn_spec_t optspec (supp ? opt : opt_code ());
 
   if (nowarn_spec_t *pspec = nowarn_map ? nowarn_map->get (loc) : NULL)
@@ -173,8 +177,22 @@ copy_warning (location_t to, location_t from)
   if (!nowarn_map)
     return;
 
-  if (nowarn_spec_t *pspec = nowarn_map->get (from))
-    nowarn_map->put (to, *pspec);
+  nowarn_spec_t *from_spec;
+  if (RESERVED_LOCATION_P (from))
+    from_spec = NULL;
   else
-    nowarn_map->remove (to);
+    from_spec = nowarn_map->get (from);
+  if (RESERVED_LOCATION_P (to))
+    {
+      /* If we cannot set no-warning dispositions for 'to', ascertain that we
+	 don't have any for 'from'.  Otherwise, we'd lose these.  */
+      gcc_checking_assert (!from_spec);
+    }
+  else
+    {
+      if (from_spec)
+	nowarn_map->put (to, *from_spec);
+      else
+	nowarn_map->remove (to);
+    }
 }
diff --git a/gcc/warning-control.cc b/gcc/warning-control.cc
index 8d6c0828445..cca4b3bf8e1 100644
--- a/gcc/warning-control.cc
+++ b/gcc/warning-control.cc
@@ -89,7 +89,7 @@ get_nowarn_spec (const_tree expr)
 {
   const location_t loc = get_location (expr);
 
-  if (loc == UNKNOWN_LOCATION)
+  if (RESERVED_LOCATION_P (loc))
     return NULL;
 
   if (!get_no_warning_bit (expr))
@@ -105,6 +105,9 @@ get_nowarn_spec (const gimple *stmt)
 {
   const location_t loc = get_location (stmt);
 
+  if (RESERVED_LOCATION_P (loc))
+    return NULL;
+
   if (!get_no_warning_bit (stmt))
     return NULL;
 
@@ -158,7 +161,8 @@ suppress_warning (tree expr, opt_code opt /* = all_warnings */,
 
   const location_t loc = get_location (expr);
 
-  supp = suppress_warning_at (loc, opt, supp) || supp;
+  if (!RESERVED_LOCATION_P (loc))
+    supp = suppress_warning_at (loc, opt, supp) || supp;
   set_no_warning_bit (expr, supp);
 }
 
@@ -174,7 +178,8 @@ suppress_warning (gimple *stmt, opt_code opt /* = all_warnings */,
 
   const location_t loc = get_location (stmt);
 
-  supp = suppress_warning_at (loc, opt, supp) || supp;
+  if (!RESERVED_LOCATION_P (loc))
+    supp = suppress_warning_at (loc, opt, supp) || supp;
   set_no_warning_bit (stmt, supp);
 }
 
@@ -186,24 +191,37 @@ void copy_warning (ToType to, FromType from)
 {
   const location_t to_loc = get_location (to);
 
-  if (nowarn_spec_t *from_map = get_nowarn_spec (from))
-    {
-      /* If there's an entry in the map the no-warning bit must be set.  */
-      gcc_assert (get_no_warning_bit (from));
+  bool supp = get_no_warning_bit (from);
 
-      gcc_checking_assert (nowarn_map);
-      nowarn_map->put (to_loc, *from_map);
-      set_no_warning_bit (to, true);
+  nowarn_spec_t *from_spec = get_nowarn_spec (from);
+  if (RESERVED_LOCATION_P (to_loc))
+    {
+#if 0 //TODO triggers during GCC build
+      /* If we cannot set no-warning dispositions for 'to', ascertain that we
+	 don't have any for 'from'.  Otherwise, we'd lose these.  */
+      gcc_checking_assert (!from_spec);
+#endif
     }
   else
     {
-      if (nowarn_map)
-	nowarn_map->remove (to_loc);
-
-      /* The no-warning bit might be set even if there's no entry
-	 in the map.  */
-      set_no_warning_bit (to, get_no_warning_bit (from));
+      if (from_spec)
+	{
+	  /* If there's an entry in the map the no-warning bit must be set.  */
+	  gcc_assert (supp);
+
+	  gcc_checking_assert (nowarn_map);
+	  nowarn_map->put (to_loc, *from_spec);
+	}
+      else
+	{
+	  if (nowarn_map)
+	    nowarn_map->remove (to_loc);
+	}
     }
+
+  /* The no-warning bit might be set even if the map has not been consulted, or
+     otherwise if there's no entry in the map.  */
+  set_no_warning_bit (to, supp);
 }
 
 /* Copy the warning disposition mapping from one expression to another.  */
-- 
2.30.2