Re: Preview: portable dumper

[Robert Pluim]

Daniel Colascione <address@hidden> writes:

> On 02/20/2018 09:28 AM, Paul Eggert wrote:
>> On 02/20/2018 09:19 AM, Daniel Colascione wrote:
>>> Do you get an assertion failure if you enable assertion checking?
>>> If we're seeing this kind of problem from the compiler expectation
>>> alone, I'll definitely revert that change.
>>
>> In my experience, eassume (and 'assume') shouldn't be used for
>> complicated expressions, as GCC is too-easily confused by
>> them. eassume should be used only for expressions where the
>> assumption really does help the compiler, either by generating
>> significantly-better code or by pacifying a false alarm.
>
> I'd hoped things had gotten better, and some local testing seemed to
> suggest it was safe to use these days. Apparently it's not, which is a
> shame. It'd be nice, but not urgent, to track down which specific
> assumptions caused what I think must be miscompilations so we could
> report them upstream.

The guilty change is:

--- a/src/pdumper.c
+++ b/src/pdumper.c
@@ -607,7 +607,7 @@ dump_set_have_current_referrer (struct dump_context *ctx, 
bool have)
 static void
 DUMP_CLEAR_REFERRER (struct dump_context *ctx)
 {
-  eassert (ctx->have_current_referrer);
+  eassume (ctx->have_current_referrer);
   dump_set_have_current_referrer (ctx, false);
   if (dump_tracking_referrers_p (ctx))
     ctx->current_referrer = Qnil;

If I revert just that hunk, the build succeeds.

Robert