GNU bug report logs - #75755
feature/igc: Missing IGC_CHECK_RES?

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 75755 in the body.
You can then email your comments to 75755 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 04:27:35 -0600

Gerd, do you remember why you didn't add CHECK_RES here?  Was there a
reason for that or just an oversight?

diff --git a/src/igc.c b/src/igc.c
index 257e7dc53f7..b6d42c9fbe1 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -3129,9 +3129,11 @@ create_thread_aps (struct igc_thread *t)
   res = mps_ap_create_k (&t->immovable_ap, gc->immovable_pool, mps_args_none);
   IGC_CHECK_RES (res);
   res = create_weak_ap (&t->weak_strong_ap, t, false);
+  IGC_CHECK_RES (res);
   res = create_weak_hash_ap (&t->weak_hash_strong_ap, t, false);
   IGC_CHECK_RES (res);
   res = create_weak_ap (&t->weak_weak_ap, t, true);
+  IGC_CHECK_RES (res);
   res = create_weak_hash_ap (&t->weak_hash_weak_ap, t, true);
   IGC_CHECK_RES (res);
 }

Message #8 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Gerd Möllmann <gerd.moellmann <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 75755 <at> debbugs.gnu.org, gerd <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 11:59:08 +0100

Stefan Kangas <stefankangas <at> gmail.com> writes:

> Gerd, do you remember why you didn't add CHECK_RES here?  Was there a
> reason for that or just an oversight?
>
> diff --git a/src/igc.c b/src/igc.c
> index 257e7dc53f7..b6d42c9fbe1 100644
> --- a/src/igc.c
> +++ b/src/igc.c
> @@ -3129,9 +3129,11 @@ create_thread_aps (struct igc_thread *t)
>    res = mps_ap_create_k (&t->immovable_ap, gc->immovable_pool, mps_args_none);
>    IGC_CHECK_RES (res);
>    res = create_weak_ap (&t->weak_strong_ap, t, false);
> +  IGC_CHECK_RES (res);
>    res = create_weak_hash_ap (&t->weak_hash_strong_ap, t, false);
>    IGC_CHECK_RES (res);
>    res = create_weak_ap (&t->weak_weak_ap, t, true);
> +  IGC_CHECK_RES (res);
>    res = create_weak_hash_ap (&t->weak_hash_weak_ap, t, true);
>    IGC_CHECK_RES (res);
>  }

Looks like Pip added new APs in 9c9b7a293f27eab3382c434b29eb9c97cb673432
and forgot the checks.

Message #11 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 75755 <at> debbugs.gnu.org, gerd <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 11:29:29 +0000

"Stefan Kangas" <stefankangas <at> gmail.com> writes:

> Gerd, do you remember why you didn't add CHECK_RES here?  Was there a
> reason for that or just an oversight?

My mistake (c6a3eb01f7acf9ccd00b38e0ef2ab05164e48635).  Please fix, or
I'll do it in a bit.

Thanks for pointing that out!

Pip

Message #14 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 75755 <at> debbugs.gnu.org, Helmut Eller <eller.helmut <at> gmail.com>, gerd <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 12:18:20 +0000

"Pip Cet" <pipcet <at> protonmail.com> writes:

> "Stefan Kangas" <stefankangas <at> gmail.com> writes:
>
>> Gerd, do you remember why you didn't add CHECK_RES here?  Was there a
>> reason for that or just an oversight?
>
> My mistake (c6a3eb01f7acf9ccd00b38e0ef2ab05164e48635).  Please fix, or
> I'll do it in a bit.

Fixed.  Thanks again.  Checked the other mps calls, but what I found is
only this:

static const char *
mps_res_to_string (mps_res_t res)
{
  switch (res)
    {
#define RES_CASE(prefix, id, doc)                                            \
  case prefix##id:                                                           \
    return #prefix #id " " doc;
      _mps_RES_ENUM (RES_CASE, MPS_RES_);
#undef RES_CASE
    default:
      return NULL;
    }
}

It would be nice if we could settle on just one function to convert
mps_t to a string, and use it consistently.

My preference would be to add this code before result_string:

/* Define a named enumeration containing all cases that the integer type
   mps_res_t is known to cover.  */

enum mps_res_enum
{
  _mps_RES_ENUM (RES_CASE, MPS_RES_)
};
#undef RES_CASE

then cast to that type in our switch statement, which would remain
explicit, exhaustive, and default-free.

That way, we get warnings if the enum is extended.

Pip

Message #17 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Gerd Möllmann <gerd.moellmann <at> gmail.com>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: gerd <at> gnu.org, 75755 <at> debbugs.gnu.org, Helmut Eller <eller.helmut <at> gmail.com>,
 Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 14:52:20 +0100

Pip Cet <pipcet <at> protonmail.com> writes:

> "Pip Cet" <pipcet <at> protonmail.com> writes:
>
> My preference would be to add this code before result_string:
>
> /* Define a named enumeration containing all cases that the integer type
>    mps_res_t is known to cover.  */
>
> enum mps_res_enum
> {
>   _mps_RES_ENUM (RES_CASE, MPS_RES_)
> };
> #undef RES_CASE

That would be nice, indeed.

Message #20 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Gerd Möllmann <gerd.moellmann <at> gmail.com>, 
 Pip Cet <pipcet <at> protonmail.com>
Cc: 75755 <at> debbugs.gnu.org, Helmut Eller <eller.helmut <at> gmail.com>, gerd <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 14:53:07 -0600

Pip Cet <pipcet <at> protonmail.com> writes:

> Fixed.  Thanks again.

Thanks.

Gerd Möllmann <gerd.moellmann <at> gmail.com> writes:

> Pip Cet <pipcet <at> protonmail.com> writes:
>
>> "Pip Cet" <pipcet <at> protonmail.com> writes:
>>
>> My preference would be to add this code before result_string:
>>
>> /* Define a named enumeration containing all cases that the integer type
>>    mps_res_t is known to cover.  */
>>
>> enum mps_res_enum
>> {
>>   _mps_RES_ENUM (RES_CASE, MPS_RES_)
>> };
>> #undef RES_CASE
>
> That would be nice, indeed.

Can we rely on this or is it an internal API?

Message #23 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 75755 <at> debbugs.gnu.org, Helmut Eller <eller.helmut <at> gmail.com>, gerd <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 21:01:59 +0000

"Stefan Kangas" <stefankangas <at> gmail.com> writes:

> Gerd Möllmann <gerd.moellmann <at> gmail.com> writes:
>
>> Pip Cet <pipcet <at> protonmail.com> writes:
>>
>>> "Pip Cet" <pipcet <at> protonmail.com> writes:
>>>
>>> My preference would be to add this code before result_string:
>>>
>>> /* Define a named enumeration containing all cases that the integer type
>>>    mps_res_t is known to cover.  */
>>>
>>> enum mps_res_enum
>>> {
>>>   _mps_RES_ENUM (RES_CASE, MPS_RES_)
>>> };
>>> #undef RES_CASE
>>
>> That would be nice, indeed.
>
> Can we rely on this or is it an internal API?

It's an internal API.  The header has this to say:

 * .naming.internal: Any identifier beginning with an underscore is for
 * internal use within the interface and may change or be withdrawn without
 * warning.

I don't think that it's always horrible to rely on internal APIs,
though.  Maybe we can put it inside "#ifdef _mps_RES_ENUM", with a
comment explaining that if in some distant future there are compilation
errors because _mps_RES_ENUM is defined differently, the #ifdef block
can safely be omitted?

Pip

Message #26 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: gerd <at> gnu.org, Helmut Eller <eller.helmut <at> gmail.com>,
 Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 75755 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Andrea Corallo <acorallo <at> gnu.org>
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 13:52:07 -0800

Pip Cet <pipcet <at> protonmail.com> writes:

> "Stefan Kangas" <stefankangas <at> gmail.com> writes:
>
>> Gerd Möllmann <gerd.moellmann <at> gmail.com> writes:
>>
>>> Pip Cet <pipcet <at> protonmail.com> writes:
>>>
>>>> "Pip Cet" <pipcet <at> protonmail.com> writes:
>>>>
>>>> My preference would be to add this code before result_string:
>>>>
>>>> /* Define a named enumeration containing all cases that the integer type
>>>>    mps_res_t is known to cover.  */
>>>>
>>>> enum mps_res_enum
>>>> {
>>>>   _mps_RES_ENUM (RES_CASE, MPS_RES_)
>>>> };
>>>> #undef RES_CASE
>>>
>>> That would be nice, indeed.
>>
>> Can we rely on this or is it an internal API?
>
> It's an internal API.  The header has this to say:
>
>  * .naming.internal: Any identifier beginning with an underscore is for
>  * internal use within the interface and may change or be withdrawn without
>  * warning.
>
> I don't think that it's always horrible to rely on internal APIs,
> though.  Maybe we can put it inside "#ifdef _mps_RES_ENUM", with a
> comment explaining that if in some distant future there are compilation
> errors because _mps_RES_ENUM is defined differently, the #ifdef block
> can safely be omitted?

AFAICT, being non-exhaustive risks that people will get suboptimal
errors, and being exhaustive means that we can get optimal errors on
master sooner.

OTOH, the situation I'm concerned about is if someone is trying to build
an old Emacs tarball with the latest MPS.  If the internal identifier
disappears, or its interface changes, the build will be broken.  The MPS
developers warn that such a change could happen "without warning".

Are the benefits of using it large enough to be worth the risks?  I'm
currently leaning towards "no", but it's possible that I'm missing
something.

Message #29 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: gerd <at> gnu.org, Helmut Eller <eller.helmut <at> gmail.com>,
 Gerd Möllmann <gerd.moellmann <at> gmail.com>,
 75755 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Andrea Corallo <acorallo <at> gnu.org>
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Wed, 22 Jan 2025 22:42:01 +0000

"Stefan Kangas" <stefankangas <at> gmail.com> writes:

> Pip Cet <pipcet <at> protonmail.com> writes:
>
>> "Stefan Kangas" <stefankangas <at> gmail.com> writes:
>>
>>> Gerd Möllmann <gerd.moellmann <at> gmail.com> writes:
>>>
>>>> Pip Cet <pipcet <at> protonmail.com> writes:
>>>>
>>>>> "Pip Cet" <pipcet <at> protonmail.com> writes:
>>>>>
>>>>> My preference would be to add this code before result_string:
>>>>>
>>>>> /* Define a named enumeration containing all cases that the integer type
>>>>>    mps_res_t is known to cover.  */
>>>>>
>>>>> enum mps_res_enum
>>>>> {
>>>>>   _mps_RES_ENUM (RES_CASE, MPS_RES_)
>>>>> };
>>>>> #undef RES_CASE
>>>>
>>>> That would be nice, indeed.
>>>
>>> Can we rely on this or is it an internal API?
>>
>> It's an internal API.  The header has this to say:
>>
>>  * .naming.internal: Any identifier beginning with an underscore is for
>>  * internal use within the interface and may change or be withdrawn without
>>  * warning.
>>
>> I don't think that it's always horrible to rely on internal APIs,
>> though.  Maybe we can put it inside "#ifdef _mps_RES_ENUM", with a
>> comment explaining that if in some distant future there are compilation
>> errors because _mps_RES_ENUM is defined differently, the #ifdef block
>> can safely be omitted?
>
> AFAICT, being non-exhaustive risks that people will get suboptimal
> errors, and being exhaustive means that we can get optimal errors on
> master sooner.
>
> OTOH, the situation I'm concerned about is if someone is trying to build
> an old Emacs tarball with the latest MPS.  If the internal identifier
> disappears,

The #ifdef block will simply not be evaluated, and the build will
continue to work.  It'll behave differently in this minor way, but
that's to be expected when a header changes.

> or its interface changes, the build will be broken.  The MPS

That is correct; I hope this is much less likely to happen, but if it
does, we're stuck with unbuildable old Emacs-new MPS combinations.

> developers warn that such a change could happen "without warning".

> Are the benefits of using it large enough to be worth the risks?  I'm
> currently leaning towards "no", but it's possible that I'm missing
> something.

I did not introduce the dependency; I moved it around and had some
preprocessor fun while doing so.  I think it's time to remove it now,
though, but that's not a strong opinion.  Please feel free to do what
you think is best on the branch.  If you have a hard time deciding,
there's always the option of keeping it behind a usually-deactivated
#ifdef, like the BYTE_CODE_SAFE code.

Pip

Message #32 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: pipcet <at> protonmail.com, eller.helmut <at> gmail.com, gerd <at> gnu.org,
 gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 08:47:15 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Wed, 22 Jan 2025 13:52:07 -0800
> Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>, 
> 	75755 <at> debbugs.gnu.org, gerd <at> gnu.org, Helmut Eller <eller.helmut <at> gmail.com>, 
> 	Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>
> 
> Pip Cet <pipcet <at> protonmail.com> writes:
> 
> > "Stefan Kangas" <stefankangas <at> gmail.com> writes:
> >
> >> Gerd Möllmann <gerd.moellmann <at> gmail.com> writes:
> >>
> >>> Pip Cet <pipcet <at> protonmail.com> writes:
> >>>
> >>>> "Pip Cet" <pipcet <at> protonmail.com> writes:
> >>>>
> >>>> My preference would be to add this code before result_string:
> >>>>
> >>>> /* Define a named enumeration containing all cases that the integer type
> >>>>    mps_res_t is known to cover.  */
> >>>>
> >>>> enum mps_res_enum
> >>>> {
> >>>>   _mps_RES_ENUM (RES_CASE, MPS_RES_)
> >>>> };
> >>>> #undef RES_CASE
> >>>
> >>> That would be nice, indeed.
> >>
> >> Can we rely on this or is it an internal API?
> >
> > It's an internal API.  The header has this to say:
> >
> >  * .naming.internal: Any identifier beginning with an underscore is for
> >  * internal use within the interface and may change or be withdrawn without
> >  * warning.
> >
> > I don't think that it's always horrible to rely on internal APIs,
> > though.  Maybe we can put it inside "#ifdef _mps_RES_ENUM", with a
> > comment explaining that if in some distant future there are compilation
> > errors because _mps_RES_ENUM is defined differently, the #ifdef block
> > can safely be omitted?
> 
> AFAICT, being non-exhaustive risks that people will get suboptimal
> errors, and being exhaustive means that we can get optimal errors on
> master sooner.
> 
> OTOH, the situation I'm concerned about is if someone is trying to build
> an old Emacs tarball with the latest MPS.  If the internal identifier
> disappears, or its interface changes, the build will be broken.  The MPS
> developers warn that such a change could happen "without warning".
> 
> Are the benefits of using it large enough to be worth the risks?  I'm
> currently leaning towards "no", but it's possible that I'm missing
> something.

IMO, the single use of this macro is not worth of the complications
due to relying on an internal enumeration macro.  The single function
which uses this is a testing tool, which on top of that warns in its
doc string not to use it.  Introducing significant complexity and
breakage potential into Emacs due to this is IMO a tail wagging the
dog.

So I suggest that we define our own enumeration.  We could copy some
information from the MPS headers if needed (or not: I'm not sure we
should be wedded to their strings in this case).  Yes, that would mean
if they add or remove enumeration values, we will have some
maintenance work on our hands, but it doesn't seem a significant
problem for an internal function that almost no one should be using.

Message #35 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>,
 acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 14:34:56 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Wed, 22 Jan 2025 13:52:07 -0800
>> Cc: Gerd Möllmann <gerd.moellmann <at> gmail.com>,
>> 	75755 <at> debbugs.gnu.org, gerd <at> gnu.org, Helmut Eller <eller.helmut <at> gmail.com>,
>> 	Eli Zaretskii <eliz <at> gnu.org>, Andrea Corallo <acorallo <at> gnu.org>
>>
>> Pip Cet <pipcet <at> protonmail.com> writes:
>>
>> > "Stefan Kangas" <stefankangas <at> gmail.com> writes:
>> >
>> >> Gerd Möllmann <gerd.moellmann <at> gmail.com> writes:
>> >>
>> >>> Pip Cet <pipcet <at> protonmail.com> writes:
>> >>>
>> >>>> "Pip Cet" <pipcet <at> protonmail.com> writes:
>> >>>>
>> >>>> My preference would be to add this code before result_string:
>> >>>>
>> >>>> /* Define a named enumeration containing all cases that the integer type
>> >>>>    mps_res_t is known to cover.  */
>> >>>>
>> >>>> enum mps_res_enum
>> >>>> {
>> >>>>   _mps_RES_ENUM (RES_CASE, MPS_RES_)
>> >>>> };
>> >>>> #undef RES_CASE
>> >>>
>> >>> That would be nice, indeed.
>> >>
>> >> Can we rely on this or is it an internal API?
>> >
>> > It's an internal API.  The header has this to say:
>> >
>> >  * .naming.internal: Any identifier beginning with an underscore is for
>> >  * internal use within the interface and may change or be withdrawn without
>> >  * warning.
>> >
>> > I don't think that it's always horrible to rely on internal APIs,
>> > though.  Maybe we can put it inside "#ifdef _mps_RES_ENUM", with a
>> > comment explaining that if in some distant future there are compilation
>> > errors because _mps_RES_ENUM is defined differently, the #ifdef block
>> > can safely be omitted?
>>
>> AFAICT, being non-exhaustive risks that people will get suboptimal
>> errors, and being exhaustive means that we can get optimal errors on
>> master sooner.
>>
>> OTOH, the situation I'm concerned about is if someone is trying to build
>> an old Emacs tarball with the latest MPS.  If the internal identifier
>> disappears, or its interface changes, the build will be broken.  The MPS
>> developers warn that such a change could happen "without warning".
>>
>> Are the benefits of using it large enough to be worth the risks?  I'm
>> currently leaning towards "no", but it's possible that I'm missing
>> something.
>
> IMO, the single use of this macro is not worth of the complications
> due to relying on an internal enumeration macro.  The single function
> which uses this is a testing tool, which on top of that warns in its
> doc string not to use it.  Introducing significant complexity and
> breakage potential into Emacs due to this is IMO a tail wagging the
> dog.

I agree with this paragraph; it's not worth it, let's drop the code.

> So I suggest that we define our own enumeration.  We could copy some

Why?

mps.h does define an enumeration, it just fails to give it a name (we'd
use it then), or use it in a typedef (we could use it then), and we
can't cast to typeof (MPS_RES_OK) because, well, that's a gcc bug.

(Yes, C defines typeof(ENUM_VALUE) to be equivalent to int; but the C
standard also doesn't prohibit the compiler from remembering where that
"int" came from, that it refers to the underlying type of a specific
enum, and adding extra warnings based on this memory)

So my preference is to remove the code and cast to typeof (MPS_RES_OK),
briefly mentioning in a comment that this approach does not work yet,
but might work in the future.

(I still think the GCC people should warn if a switch statement contains
labels for some but not all elements of an enum, even if the switch
value is cast to int.  It's the labels that matter, not the matched
value.  Works, see patch.  I assume GCC folks will disagree.)

> information from the MPS headers if needed (or not: I'm not sure we
> should be wedded to their strings in this case).  Yes, that would mean

We should use our strings, which Stefan Kangas provided (and which are
in use by the current code).

> if they add or remove enumeration values, we will have some
              ^^^^^^^^^

If they remove enumeration values, that's a breaking change, and the
build will break, as it should. (Warning about non-breaking changes is
better than breaking the build, but breaking the build is still better
than silently accepting a situation which is likely to result in runtime
errors).

> maintenance work on our hands, but it doesn't seem a significant
> problem for an internal function that almost no one should be using.

Just so we don't forget this isn't about this specific case only:

We've yet to add the TSQueryErrorLanguage case to treesit.c because
there was no compile-time warning.  It's a lot more work than it would
have been if the warning had been present.

diff --git a/src/treesit.c b/src/treesit.c
index d4090b949ea..7f34ef085dd 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -1446,9 +1446,13 @@ treesit_query_error_to_string (TSQueryError error)
       return "Capture error at";
     case TSQueryErrorStructure:
       return "Structure error at";
-    default:
-      return "Unknown error";
+      /* FIXME: what's a "language error"?  Do all versions of
+	 treesitter we support define this enumeration value?
+    case TSQueryErrorLanguage:
+      return "Language error at";
+      */
     }
+  return "Unknown error";
 }
 
 static Lisp_Object

In the case of treesit.c, the problem would have been printed by
-Wswitch-enum, or by writing treesit_query_error_to_string more
carefully.

Switching to -Wswitch-enum would be good, but cause a number of warnings
for code which is perfectly acceptable because it uses a local or
logically complete enum instead of one which is subject to unexpected
changes.

And, yes, two or three places would require #pragma's to avoid the
warning because an external enum we cannot control or establish the
potential values of in a preprocessor macro might change under us and
otherwise cause noise.

I'm volunteering to do this, except for bidi.c; I'll put a #pragma in to
avoid the warning, and then someone who understands the code better can
decide whether to make the code comply to -Wswitch-enum or not.  Let me
know if this is desirable and I'll send a patch (which will be large
because things like the switch (XTYPE (elt)) in xdisp.c change
indentation levels).

Pip

Message #38 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755 <at> debbugs.gnu.org, stefankangas <at> gmail.com, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 17:12:38 +0200

> Date: Thu, 23 Jan 2025 14:34:56 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> > IMO, the single use of this macro is not worth of the complications
> > due to relying on an internal enumeration macro.  The single function
> > which uses this is a testing tool, which on top of that warns in its
> > doc string not to use it.  Introducing significant complexity and
> > breakage potential into Emacs due to this is IMO a tail wagging the
> > dog.
> 
> I agree with this paragraph; it's not worth it, let's drop the code.
> 
> > So I suggest that we define our own enumeration.  We could copy some
> 
> Why?

Because it's easy, and will still give us human-readable error
messages.

> So my preference is to remove the code and cast to typeof (MPS_RES_OK),
> briefly mentioning in a comment that this approach does not work yet,
> but might work in the future.

I'm fine with that as well, this being an obscure internal function.

> > maintenance work on our hands, but it doesn't seem a significant
> > problem for an internal function that almost no one should be using.
> 
> Just so we don't forget this isn't about this specific case only:

No, what I wrote was _only_ about this one case.  The other cases
should be considered separately, and the conclusion might well be
different.

Message #41 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755 <at> debbugs.gnu.org, stefankangas <at> gmail.com, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 17:17:54 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Thu, 23 Jan 2025 14:34:56 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: Stefan Kangas <stefankangas <at> gmail.com>, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> > IMO, the single use of this macro is not worth of the complications
>> > due to relying on an internal enumeration macro.  The single function
>> > which uses this is a testing tool, which on top of that warns in its
>> > doc string not to use it.  Introducing significant complexity and
>> > breakage potential into Emacs due to this is IMO a tail wagging the
>> > dog.
>>
>> I agree with this paragraph; it's not worth it, let's drop the code.
>>
>> > So I suggest that we define our own enumeration.  We could copy some
>>
>> Why?
>
> Because it's easy, and will still give us human-readable error
> messages.

How does defining an enum give us that?  Did you mean "our own switch
statement"?  If so, what's wrong with the one on the current branch?

Pip

Message #44 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755 <at> debbugs.gnu.org, stefankangas <at> gmail.com, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 19:47:55 +0200

> Date: Thu, 23 Jan 2025 17:17:54 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: stefankangas <at> gmail.com, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> Date: Thu, 23 Jan 2025 14:34:56 +0000
> >> From: Pip Cet <pipcet <at> protonmail.com>
> >> Cc: Stefan Kangas <stefankangas <at> gmail.com>, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
> >>
> >> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> >>
> >> > IMO, the single use of this macro is not worth of the complications
> >> > due to relying on an internal enumeration macro.  The single function
> >> > which uses this is a testing tool, which on top of that warns in its
> >> > doc string not to use it.  Introducing significant complexity and
> >> > breakage potential into Emacs due to this is IMO a tail wagging the
> >> > dog.
> >>
> >> I agree with this paragraph; it's not worth it, let's drop the code.
> >>
> >> > So I suggest that we define our own enumeration.  We could copy some
> >>
> >> Why?
> >
> > Because it's easy, and will still give us human-readable error
> > messages.
> 
> How does defining an enum give us that?  Did you mean "our own switch
> statement"?  If so, what's wrong with the one on the current branch?

We are talking past each other.  I meant the readable text that comes
with each enum value:

  #define _mps_RES_ENUM(R, X) \
    R(X, OK,            "success (always zero)") \
    R(X, FAIL,          "unspecified failure") \
    R(X, RESOURCE,      "unable to obtain resources") \
    R(X, MEMORY,        "unable to obtain memory") \
    R(X, LIMIT,         "limitation reached") \
    R(X, UNIMPL,        "unimplemented facility") \
    R(X, IO,            "system I/O error") \
    R(X, COMMIT_LIMIT,  "arena commit limit exceeded") \
    R(X, PARAM,         "illegal user parameter value")

which mps_res_to_string uses to produce a string for each value of
mps_res_t.

IOW, I was not talking about defining an enum, I was talking about
defining a text string for each value of enum.

Message #47 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Pip Cet <pipcet <at> protonmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, acorallo <at> gnu.org,
 eller.helmut <at> gmail.com, gerd <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 11:51:50 -0600

Pip Cet <pipcet <at> protonmail.com> writes:

> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>
>>> Date: Thu, 23 Jan 2025 14:34:56 +0000
>>> From: Pip Cet <pipcet <at> protonmail.com>
>>> Cc: Stefan Kangas <stefankangas <at> gmail.com>, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
>>>
>>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>>
>>> > IMO, the single use of this macro is not worth of the complications
>>> > due to relying on an internal enumeration macro.  The single function
>>> > which uses this is a testing tool, which on top of that warns in its
>>> > doc string not to use it.  Introducing significant complexity and
>>> > breakage potential into Emacs due to this is IMO a tail wagging the
>>> > dog.
>>>
>>> I agree with this paragraph; it's not worth it, let's drop the code.
>>>
>>> > So I suggest that we define our own enumeration.  We could copy some
>>>
>>> Why?
>>
>> Because it's easy, and will still give us human-readable error
>> messages.
>
> How does defining an enum give us that?  Did you mean "our own switch
> statement"?  If so, what's wrong with the one on the current branch?

FWIW, I'd keep it simple here and just install this.  At the end of the
day, while suboptimal, it's also not the end of the world if a new error
returns the "unknown error" string.  We do get the numerical error code,
so we can always decipher it.

diff --git a/src/igc.c b/src/igc.c
index 57c13614f73..b02d29ca4da 100644
--- a/src/igc.c
+++ b/src/igc.c
@@ -252,18 +252,6 @@
 static enum igc_state igc_state = IGC_STATE_INITIAL;
 static void set_state (enum igc_state state);

-/* Define a named enumeration containing all cases that the integer type
-   mps_res_t is known to cover.  */
-
-#define RES_CASE(prefix, id, doc)                                            \
-  id,
-
-enum mps_res_enum
-{
-  _mps_RES_ENUM (RES_CASE, MPS_RES_)
-};
-#undef RES_CASE
-
 /* Convert an mps result code into a result string.  This shouldn't
    allocate memory because it's called when a fatal memory management
    error occurs. */
@@ -271,9 +259,7 @@ #define RES_CASE(prefix, id, doc)
                          \
 static const char *
 mps_res_to_string (mps_res_t res)
 {
-  /* mps_res_t is typedef'd to int, we want an enum so GCC warns about
-     new cases.  */
-  switch ((enum mps_res_enum) res)
+  switch (res)
     {
     case MPS_RES_OK:
       return "operation succeeded";

Message #50 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755 <at> debbugs.gnu.org, stefankangas <at> gmail.com, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 17:53:01 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Thu, 23 Jan 2025 17:17:54 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: stefankangas <at> gmail.com, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
>>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>> >> Date: Thu, 23 Jan 2025 14:34:56 +0000
>> >> From: Pip Cet <pipcet <at> protonmail.com>
>> >> Cc: Stefan Kangas <stefankangas <at> gmail.com>, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
>> >>
>> >> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>> >>
>> >> > IMO, the single use of this macro is not worth of the complications
>> >> > due to relying on an internal enumeration macro.  The single function
>> >> > which uses this is a testing tool, which on top of that warns in its
>> >> > doc string not to use it.  Introducing significant complexity and
>> >> > breakage potential into Emacs due to this is IMO a tail wagging the
>> >> > dog.
>> >>
>> >> I agree with this paragraph; it's not worth it, let's drop the code.
>> >>
>> >> > So I suggest that we define our own enumeration.  We could copy some
>> >>
>> >> Why?
>> >
>> > Because it's easy, and will still give us human-readable error
>> > messages.
>>
>> How does defining an enum give us that?  Did you mean "our own switch
>> statement"?  If so, what's wrong with the one on the current branch?
>
> We are talking past each other.  I meant the readable text that comes
> with each enum value:
>
>   #define _mps_RES_ENUM(R, X) \
>     R(X, OK,            "success (always zero)") \
>     R(X, FAIL,          "unspecified failure") \
>     R(X, RESOURCE,      "unable to obtain resources") \
>     R(X, MEMORY,        "unable to obtain memory") \
>     R(X, LIMIT,         "limitation reached") \
>     R(X, UNIMPL,        "unimplemented facility") \
>     R(X, IO,            "system I/O error") \
>     R(X, COMMIT_LIMIT,  "arena commit limit exceeded") \
>     R(X, PARAM,         "illegal user parameter value")
>
> which mps_res_to_string uses to produce a string for each value of
> mps_res_t.
>
> IOW, I was not talking about defining an enum, I was talking about
> defining a text string for each value of enum.

So what are you suggesting be changed?  We do that with a switch
statement, in mps_res_to_string.  Are you proposing to do it another
way?

Pip

Message #53 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 17:56:31 +0000

"Stefan Kangas" <stefankangas <at> gmail.com> writes:

> Pip Cet <pipcet <at> protonmail.com> writes:
>
>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>
>>>> Date: Thu, 23 Jan 2025 14:34:56 +0000
>>>> From: Pip Cet <pipcet <at> protonmail.com>
>>>> Cc: Stefan Kangas <stefankangas <at> gmail.com>, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
>>>>
>>>> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>>>>
>>>> > IMO, the single use of this macro is not worth of the complications
>>>> > due to relying on an internal enumeration macro.  The single function
>>>> > which uses this is a testing tool, which on top of that warns in its
>>>> > doc string not to use it.  Introducing significant complexity and
>>>> > breakage potential into Emacs due to this is IMO a tail wagging the
>>>> > dog.
>>>>
>>>> I agree with this paragraph; it's not worth it, let's drop the code.
>>>>
>>>> > So I suggest that we define our own enumeration.  We could copy some
>>>>
>>>> Why?
>>>
>>> Because it's easy, and will still give us human-readable error
>>> messages.
>>
>> How does defining an enum give us that?  Did you mean "our own switch
>> statement"?  If so, what's wrong with the one on the current branch?
>
> FWIW, I'd keep it simple here and just install this.  At the end of the

Please do.  I confess I tried to find a solution for the general case
here; unnamed enums are rare so worrying about that was inappropriate.

Pip

Message #56 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755 <at> debbugs.gnu.org, stefankangas <at> gmail.com, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 20:27:00 +0200

> Date: Thu, 23 Jan 2025 17:53:01 +0000
> From: Pip Cet <pipcet <at> protonmail.com>
> Cc: stefankangas <at> gmail.com, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
> 
> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> 
> >> Date: Thu, 23 Jan 2025 17:17:54 +0000
> >> From: Pip Cet <pipcet <at> protonmail.com>
> >> Cc: stefankangas <at> gmail.com, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
> >>
> >> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> >>
> >> >> Date: Thu, 23 Jan 2025 14:34:56 +0000
> >> >> From: Pip Cet <pipcet <at> protonmail.com>
> >> >> Cc: Stefan Kangas <stefankangas <at> gmail.com>, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
> >> >>
> >> >> "Eli Zaretskii" <eliz <at> gnu.org> writes:
> >> >>
> >> >> > IMO, the single use of this macro is not worth of the complications
> >> >> > due to relying on an internal enumeration macro.  The single function
> >> >> > which uses this is a testing tool, which on top of that warns in its
> >> >> > doc string not to use it.  Introducing significant complexity and
> >> >> > breakage potential into Emacs due to this is IMO a tail wagging the
> >> >> > dog.
> >> >>
> >> >> I agree with this paragraph; it's not worth it, let's drop the code.
> >> >>
> >> >> > So I suggest that we define our own enumeration.  We could copy some
> >> >>
> >> >> Why?
> >> >
> >> > Because it's easy, and will still give us human-readable error
> >> > messages.
> >>
> >> How does defining an enum give us that?  Did you mean "our own switch
> >> statement"?  If so, what's wrong with the one on the current branch?
> >
> > We are talking past each other.  I meant the readable text that comes
> > with each enum value:
> >
> >   #define _mps_RES_ENUM(R, X) \
> >     R(X, OK,            "success (always zero)") \
> >     R(X, FAIL,          "unspecified failure") \
> >     R(X, RESOURCE,      "unable to obtain resources") \
> >     R(X, MEMORY,        "unable to obtain memory") \
> >     R(X, LIMIT,         "limitation reached") \
> >     R(X, UNIMPL,        "unimplemented facility") \
> >     R(X, IO,            "system I/O error") \
> >     R(X, COMMIT_LIMIT,  "arena commit limit exceeded") \
> >     R(X, PARAM,         "illegal user parameter value")
> >
> > which mps_res_to_string uses to produce a string for each value of
> > mps_res_t.
> >
> > IOW, I was not talking about defining an enum, I was talking about
> > defining a text string for each value of enum.
> 
> So what are you suggesting be changed?  We do that with a switch
> statement, in mps_res_to_string.  Are you proposing to do it another
> way?

I'm proposing to define, in igc.c, a set of strings, one each for
every value of the enumeration.  And then to use that in
mps_res_to_string to produce a human-readable error message describing
each mps_res_t value.

Message #59 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Pip Cet <pipcet <at> protonmail.com>
Cc: gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, acorallo <at> gnu.org,
 eller.helmut <at> gmail.com, gerd <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 13:28:18 -0600

Eli Zaretskii <eliz <at> gnu.org> writes:

> I'm proposing to define, in igc.c, a set of strings, one each for
> every value of the enumeration.  And then to use that in
> mps_res_to_string to produce a human-readable error message describing
> each mps_res_t value.

Please let me know what you think of my most recent patch.  If you want
something different, may I ask that you send a patch?  I don't think
I understand if your idea is different from what I posted.  Thanks.

Message #62 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 13:32:19 -0600

Pip Cet <pipcet <at> protonmail.com> writes:

> Please do.

Thanks, I'm waiting to hear back from Eli as well.

> I confess I tried to find a solution for the general case
> here; unnamed enums are rare so worrying about that was inappropriate.

No problem at all.  It can be useful to take a broad look at things.

My conclusion from our discussions about this is that I'd be happy to
see improvements to GCC in this area.  :-)

Message #65 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: pipcet <at> protonmail.com, eller.helmut <at> gmail.com, gerd <at> gnu.org,
 gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 21:58:40 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Thu, 23 Jan 2025 13:28:18 -0600
> Cc: gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, gerd <at> gnu.org, 
> 	eller.helmut <at> gmail.com, acorallo <at> gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I'm proposing to define, in igc.c, a set of strings, one each for
> > every value of the enumeration.  And then to use that in
> > mps_res_to_string to produce a human-readable error message describing
> > each mps_res_t value.
> 
> Please let me know what you think of my most recent patch.  If you want
> something different, may I ask that you send a patch?  I don't think
> I understand if your idea is different from what I posted.  Thanks.

Your patch still uses _mps_RES_ENUM, which is a macro that's not
supposed to be used outside of MPS.  Isn't that so?  I thought all
this discussion was how to avoid using that macro, wasn't it?

Message #68 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: pipcet <at> protonmail.com, eller.helmut <at> gmail.com, gerd <at> gnu.org,
 gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 23 Jan 2025 15:32:16 -0600

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Please let me know what you think of my most recent patch.  If you want
>> something different, may I ask that you send a patch?  I don't think
>> I understand if your idea is different from what I posted.  Thanks.
>
> Your patch still uses _mps_RES_ENUM, which is a macro that's not
> supposed to be used outside of MPS.  Isn't that so?  I thought all
> this discussion was how to avoid using that macro, wasn't it?

AFAIU, the patch I proposed removes that macro.  I just now grepped for
_mps_RES_ENUM after applying it, and couldn't find any matches in my
tree.  Are you seeing something different?

To simplify the review, I've attached the patch with a proper commit
message below.  Thanks.

[0001-Don-t-use-MPS-internal-macro-in-mps_res_to_string.patch (text/x-patch, attachment)]

Message #71 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: pipcet <at> protonmail.com, eller.helmut <at> gmail.com, gerd <at> gnu.org,
 gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Fri, 24 Jan 2025 09:29:54 +0200

> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Thu, 23 Jan 2025 15:32:16 -0600
> Cc: pipcet <at> protonmail.com, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, 
> 	gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
> 
> >> Please let me know what you think of my most recent patch.  If you want
> >> something different, may I ask that you send a patch?  I don't think
> >> I understand if your idea is different from what I posted.  Thanks.
> >
> > Your patch still uses _mps_RES_ENUM, which is a macro that's not
> > supposed to be used outside of MPS.  Isn't that so?  I thought all
> > this discussion was how to avoid using that macro, wasn't it?
> 
> AFAIU, the patch I proposed removes that macro.  I just now grepped for
> _mps_RES_ENUM after applying it, and couldn't find any matches in my
> tree.  Are you seeing something different?

Yes, I was looking at a stale igc.c, so I didn't see that
mps_res_to_string was modified exactly as I suggested to do.

Sorry.

Message #74 received at 75755 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: pipcet <at> protonmail.com, eller.helmut <at> gmail.com, gerd <at> gnu.org,
 gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Fri, 24 Jan 2025 18:02:11 -0600

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Thu, 23 Jan 2025 15:32:16 -0600
>> Cc: pipcet <at> protonmail.com, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org,
>> 	gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
>>
>> >> Please let me know what you think of my most recent patch.  If you want
>> >> something different, may I ask that you send a patch?  I don't think
>> >> I understand if your idea is different from what I posted.  Thanks.
>> >
>> > Your patch still uses _mps_RES_ENUM, which is a macro that's not
>> > supposed to be used outside of MPS.  Isn't that so?  I thought all
>> > this discussion was how to avoid using that macro, wasn't it?
>>
>> AFAIU, the patch I proposed removes that macro.  I just now grepped for
>> _mps_RES_ENUM after applying it, and couldn't find any matches in my
>> tree.  Are you seeing something different?
>
> Yes, I was looking at a stale igc.c, so I didn't see that
> mps_res_to_string was modified exactly as I suggested to do.

No problem, and thanks.  Installed on feature/igc as commit 2ce718a5274.

Message #79 received at 75755-done <at> debbugs.gnu.org (full text, mbox):

From: Pip Cet <pipcet <at> protonmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: gerd <at> gnu.org, eller.helmut <at> gmail.com, gerd.moellmann <at> gmail.com,
 75755-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, acorallo <at> gnu.org
Subject: Re: bug#75755: feature/igc: Missing IGC_CHECK_RES?
Date: Thu, 06 Feb 2025 00:25:47 +0000

"Stefan Kangas" <stefankangas <at> gmail.com> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Stefan Kangas <stefankangas <at> gmail.com>
>>> Date: Thu, 23 Jan 2025 15:32:16 -0600
>>> Cc: pipcet <at> protonmail.com, gerd.moellmann <at> gmail.com, 75755 <at> debbugs.gnu.org,
>>> 	gerd <at> gnu.org, eller.helmut <at> gmail.com, acorallo <at> gnu.org
>>>
>>> >> Please let me know what you think of my most recent patch.  If you want
>>> >> something different, may I ask that you send a patch?  I don't think
>>> >> I understand if your idea is different from what I posted.  Thanks.
>>> >
>>> > Your patch still uses _mps_RES_ENUM, which is a macro that's not
>>> > supposed to be used outside of MPS.  Isn't that so?  I thought all
>>> > this discussion was how to avoid using that macro, wasn't it?
>>>
>>> AFAIU, the patch I proposed removes that macro.  I just now grepped for
>>> _mps_RES_ENUM after applying it, and couldn't find any matches in my
>>> tree.  Are you seeing something different?
>>
>> Yes, I was looking at a stale igc.c, so I didn't see that
>> mps_res_to_string was modified exactly as I suggested to do.
>
> No problem, and thanks.  Installed on feature/igc as commit 2ce718a5274.

I'm closing this, because I think the original issue, and the enum
followup discussions, have both been resolved :-)

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.