[bug#42146] [PATCH core-updates 1/?] build: substitute: Don't fail silently.

["Ludovic Courtès" <ludo@gnu.org>]

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>>> +;;; Extend regexp objects with a pattern field.
>>> +;;;
>>> +(define-record-type <regexp*>
>>> +  (%make-regexp* pat flag rx)
>>> +  regexp*?
>>> +  (pat regexp*-pattern)                 ;the regexp pattern, a string
>>> +  (flag regexp*-flag)                   ;regexp flags
>>> +  (rx regexp*-rx))                      ;the compiled regexp object
>>> +
>>> +;;; Work around regexp implementation.
>>> +;;; This record allows to track the regexp pattern and then display it.
>>> +(define* (make-regexp* pat #:optional (flag regexp/extended))
>>
>> I’m skeptical about the concrete benefits.  I would not include it in
>> (guix build utils), or at least not in this patch series.

[...]

> The benefit is concrete: it makes it possible to show which regexp
> pattern failed to match (its textual representation), instead of
> something much less useful such as a generic placeholder such as
> "<unknown> (regexp)".

Yes, okay.

Can we keep the <regexp*> interface private, then?  To me it’s an
implementation detail and perhaps a bit of a hack that I’d rather not
commit to maintaining.

The cost might be to duplicate it in teams.scm, but that’s an acceptable
cost IMO.

> It's in actual use if you look at the definition of substitute:
>
>
>   (let ((rx+proc (map (match-lambda
>                         (((or (? regexp? pattern) (? regexp*? pattern)) . proc)
>                          (cons pattern proc))
>                         ((pattern . proc)
>                          (cons (make-regexp* pattern regexp/extended) proc)))
>                       pattern+procs)))
>
> The previous version followed a different approach, annotating the
> rx+proc list with the raw pattern; I think the approach here is a bit
> cleaner, and it should also enable users to pass pre-computed regexp*
> objects to substitute* and have useful error messages produced.

Note that ‘substitute*’ only takes string literals as patterns, not
regexp objects.  The pattern part of clauses has sometimes been abused
to pass code, typically ‘string-append’ calls, but that’s definitely not
the spirit.

Another approach would have been, in ‘substitute*’, to capture the
regexp-as-string as well as other useful debugging info, such as its
source location.

Thanks,
Ludo’.