Re: [PATCH] Fix potential invalid argument error in `##sys#decompose-import'

[Evan Hanson]

On 2020-05-03 11:44, felix.winkelm...@bevuta.com wrote:
> I'd love to apply your patches, but can you remove the "noprefix=true" option
> from your .git/config, please? Apparently git am doesn't recognize this patch.

Yes, apologies, I'll fix it for future patches. I didn't consider that
Git might become confused on the other end, but that makes sense.

Thanks!

Evan

Re: Fwd: Comments on draft #3

[John Cowan]

The behavior of a macro keyword in operand position is not affected by the
patch.

On Sun, May 3, 2020 at 1:04 PM Peter Bex  wrote:

> On Sun, May 03, 2020 at 12:55:40PM -0400, John Cowan wrote:
> > This is the first of two patches that allow macros to be expanded when
> they
> > appear in operand position, so that if pi is a macro for 3.14159, then
> (+ 5
> > pi) will be expanded to (+ 5 3.14159).  This allows macros that work like
> > procedures in the sense that they can be passed as operands.
> >
> > This patch works for er-macros and ir-macros.  Basically if a low-level
> > macro is found in operand position, rather than an a syntax error,
> whatever
> > the macro expands into is substituted.
>
> To me it seems this introduces an ambiguity between (foo) and foo.
> What if foo is a macro that expands to a procedure?  Then
> (apply foo '()) would then be identical to (apply (foo) '()),
> unless I'm misunderstanding something.  I would find that highly
> undesirable.
>
> Cheers,
> Peter
>

Re: Fwd: Comments on draft #3

[Peter Bex]

On Sun, May 03, 2020 at 12:55:40PM -0400, John Cowan wrote:
> This is the first of two patches that allow macros to be expanded when they
> appear in operand position, so that if pi is a macro for 3.14159, then (+ 5
> pi) will be expanded to (+ 5 3.14159).  This allows macros that work like
> procedures in the sense that they can be passed as operands.
> 
> This patch works for er-macros and ir-macros.  Basically if a low-level
> macro is found in operand position, rather than an a syntax error, whatever
> the macro expands into is substituted.

To me it seems this introduces an ambiguity between (foo) and foo.
What if foo is a macro that expands to a procedure?  Then
(apply foo '()) would then be identical to (apply (foo) '()),
unless I'm misunderstanding something.  I would find that highly
undesirable.

Cheers,
Peter


signature.asc
Description: PGP signature

Fwd: Comments on draft #3

[John Cowan]

This is the second of two patches that allow macros to be expanded when
they appear in operand position.  It allows a syntax-rules pattern to be a
single variable rather than a list; this rule will be used when the macro
keyword is in operand position.

-- Forwarded message -
From: Marc Nieper-Wißkirchen 
Date: Sun, May 3, 2020 at 12:10 PM
Subject: Re: Comments on draft #3
To: John Cowan 
Cc: , Lassi Kortela 


With the attached patch, SYNTAX-RULES macros now allow a single identifier
as a top-level pattern, which matches the whole form:

(define-syntax foo
  (syntax-rules ()
(x (begin (display 'x) (newline)

foo ;Expands into code that displays "foo".

Am So., 3. Mai 2020 um 17:51 Uhr schrieb Marc Nieper-Wißkirchen <
m...@nieper-wisskirchen.de>:

> Am So., 3. Mai 2020 um 11:06 Uhr schrieb Marc Nieper-Wißkirchen <
> m...@nieper-wisskirchen.de>:
>
> [...]
>
>
>> I bet that implementing identifier macros in any Scheme system is a
>> work of less than one day each. Already now every Scheme system
>> handles macro keywords, which are not in head position, (usually by
>> throwing an error). To make it usable in systems without
>> ER-MACRO-TRANSFORMER and SYNTAX-CASE, a top-level pattern in
>> SYNTAX-RULES, which just consists of an identifier has to be allowed
>> (as all other patterns won't match).
>>
>
> To prove that my bet is not insubstantial, I have attached a patch for the
> Chicken implementation of Scheme. With the patch (it may need to be
> polished by the maintainers of Chicken), transformers associated with
> keywords are now also invoked when the keyword is not in head position.
>
> The following example will expand `foo' into #f and display "foo" on the
> console.
>
> (define-syntax foo
>   (er-macro-transformer
> (lambda (x r c)
>   (display x)
>   (newline)
>   #f))
>
> foo
>
> Although I have just installed the Chicken source code, which is still
> alien to me, it took me less than two hours to get the thing done.  I
> shouldn't be a hurdle for other implementations as well.
>
> Marc
>
From 73a2cce9168b451e83e92fe5e85380d43e9da657 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc=20Nieper-Wi=C3=9Fkirchen?= 
Date: Sun, 3 May 2020 18:07:32 +0200
Subject: [PATCH 2/2] Add a single identifier top-level pattern to
 syntax-rules.

---
 synrules.scm | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/synrules.scm b/synrules.scm
index d0919862..4a8c46f5 100644
--- a/synrules.scm
+++ b/synrules.scm
@@ -138,7 +138,7 @@
   (define (make-transformer rules)
 `(##sys#er-transformer
   (,%lambda (,%input ,%rename ,%compare)
-	(,%let ((,%tail (,%cdr ,%input)))
+	(,%let ((,%tail (,%cond ((,%pair? ,%input) (,%cdr ,%input)) (,%else ,%input
 	  (,%cond ,@(map process-rule rules)
 		  (,%else (,%syntax-rules-mismatch ,%input)))
 
@@ -146,7 +146,9 @@
 (if (and (pair? rule)
 	 (pair? (cdr rule))
 	 (null? (cddr rule)))
-	(let ((pattern (cdar rule))
+	(let ((pattern (if (pair? (car rule))
+			   (cdar rule)
+			   (car rule)))
 	  (template (cadr rule)))
 	  `((,%and ,@(process-match %tail pattern #f))
 	(,%let* ,(process-pattern pattern
-- 
2.20.1

Fwd: Comments on draft #3

[John Cowan]

This is the first of two patches that allow macros to be expanded when they
appear in operand position, so that if pi is a macro for 3.14159, then (+ 5
pi) will be expanded to (+ 5 3.14159).  This allows macros that work like
procedures in the sense that they can be passed as operands.

This patch works for er-macros and ir-macros.  Basically if a low-level
macro is found in operand position, rather than an a syntax error, whatever
the macro expands into is substituted.

-- Forwarded message -
From: Marc Nieper-Wißkirchen 
Date: Sun, May 3, 2020 at 11:51 AM
Subject: Re: Comments on draft #3
To: John Cowan 
Cc: , Lassi Kortela 


Am So., 3. Mai 2020 um 11:06 Uhr schrieb Marc Nieper-Wißkirchen <
m...@nieper-wisskirchen.de>:

[...]


> I bet that implementing identifier macros in any Scheme system is a
> work of less than one day each. Already now every Scheme system
> handles macro keywords, which are not in head position, (usually by
> throwing an error). To make it usable in systems without
> ER-MACRO-TRANSFORMER and SYNTAX-CASE, a top-level pattern in
> SYNTAX-RULES, which just consists of an identifier has to be allowed
> (as all other patterns won't match).
>

To prove that my bet is not insubstantial, I have attached a patch for the
Chicken implementation of Scheme. With the patch (it may need to be
polished by the maintainers of Chicken), transformers associated with
keywords are now also invoked when the keyword is not in head position.

The following example will expand `foo' into #f and display "foo" on the
console.

(define-syntax foo
  (er-macro-transformer
(lambda (x r c)
  (display x)
  (newline)
  #f))

foo

Although I have just installed the Chicken source code, which is still
alien to me, it took me less than two hours to get the thing done.  I
shouldn't be a hurdle for other implementations as well.

Marc
From 32416f1e97f6ccdecf858ff8cbc41a55a4099afb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc=20Nieper-Wi=C3=9Fkirchen?= 
Date: Sun, 3 May 2020 17:43:21 +0200
Subject: [PATCH] Implement identifier macros for er/ir-macro-transformer.

---
 core.scm   |  20 +++---
 eval.scm   | 204 +++--
 expand.scm |  99 ++
 3 files changed, 169 insertions(+), 154 deletions(-)

diff --git a/core.scm b/core.scm
index 584c3453..f84fd619 100644
--- a/core.scm
+++ b/core.scm
@@ -637,7 +637,11 @@
 
   (define (walk x e dest ldest h outer-ln tl?)
 (cond ((keyword? x) `(quote ,x))
-	  ((symbol? x) (resolve-variable x e dest ldest h))
+	  ((symbol? x)
+	   (emit-syntax-trace-info x #f)
+	   (let ((xexpanded (expand x (##sys#current-environment) #f)))
+	 (if (not (eq? x xexpanded)) (walk xexpanded e dest ldest h outer-ln tl?)
+		 (resolve-variable x e dest ldest h
 	  ((not (pair? x))
 	   (if (constant? x)
 	   `(quote ,x)
@@ -793,15 +797,15 @@
   vars tmps)
 			   (##core#let () ,@body) ) )
 			e dest ldest h ln #f)))
-  
+
 ((##core#with-forbidden-refs)
  (let* ((loc (caddr x))
 (vars (map (lambda (v)
  (cons (resolve-variable v e dest
- ldest h) 
+ ldest h)
loc))
 (cadr x
-   (fluid-let ((forbidden-refs 
+   (fluid-let ((forbidden-refs
  (append vars forbidden-refs)))
  (walk (cadddr x) e dest ldest h ln #f
 
@@ -2129,10 +2133,10 @@
 (not (db-get db name 'global))
 (not (db-get db name 'unknown))
 (eq? '##core#lambda (node-class val))
-(not (llist-match? (third (node-parameters val)) 
+(not (llist-match? (third (node-parameters val))
(cdr subs
 (quit-compiling
-		  "known procedure called with wrong number of arguments: `~A'" 
+		  "known procedure called with wrong number of arguments: `~A'"
 	  (real-name name)))
 		 (collect! db name 'call-sites (cons here n
 	 (walk (first subs) env localenv fullenv here)
@@ -2773,8 +2777,8 @@
 		  boxedaliases) ))
    (if (null? aliases)
    body
-   (make-node 'let (list (car aliases)) 
-		  (list (car values) 
+   (make-node 'let (list (car aliases))
+		  (list (car values)
 			(loop (cdr aliases) (cdr values))
  body) ) ) )
 		(let ((cvars (map (lambda (v) (ref-var (varnode v) here closure))
diff --git a/eval.scm b/eval.scm
index 7c9da9a0..ca76c6dc 100644
--- a/eval.scm
+++ 

Re: [PATCH] force finalizers only if finalizers exist

[megane]

felix.winkelm...@bevuta.com writes:

>> On 2020-04-07 14:51, felix.winkelm...@bevuta.com wrote:
>> > > > This patch disables the final GC for finalizer forcing at normal 
>> > > > program termination
>> > > > if no finalizers are live as the GC is unnecessary in such cases.
>> > >
>> > > How about possible pending finalizers?
>> >
>> > You mean pending from a previous GC? I'm not sure - shouldn't they already
>> > be executed at this stage?
>>
>> Hey megane, could you go into a bit more detail about this?
>>
>> If I understand correctly, you mean that some finalizers that have been
>> moved onto the pending_finalizers_symbol may not have been invoked by
>> this point, but will also not included in the C_i_live_finalizers_count,
>> is that right? That seems right to me, but it's not clear to me what the
>> check should rather be.
>>
>
> live_finalizer_count gets decreased when a pending finalizer is inserted
> into the pending finalizers lists, so I understand that the live count does 
> not
> include the pending ones. But the ones pending are not automatically run,
> so it may be that before exiting we may have to do a final 
> ##sys#run-pending-finalizers.
> I will submit a modified patch.

Yeah that sounds about right.

There might be a situation where a thread is already executing
##sys#run-pending-finalizers, but has run out of its time slice, or gc
interrupted or something. In this situation running
##sys#run-pending-finalizers only once might not be enough.


>
>
> felix

Re: More millisecond procedures

[Peter Bex]

On Sun, May 03, 2020 at 03:48:46PM +0300, Lassi Kortela wrote:
> > Attached is a set of two patches.  The first one simply adds a
> > deprecation notice to current-milliseconds and adds the new procedure
> > current-process-milliseconds.
> 
> Racket also has `current-inexact-milliseconds`: "Returns the current time in
> milliseconds since midnight UTC, January 1, 1970. The result may contain
> fractions of a millisecond."
> 

That sounds wrong in many ways.  As you get further from 1970, you lose
more and more precision in the sub-millisecond range.

> In general, millisecond precision is probably attainable on most/all
> operating systems without too much fuss.

Yeah, and that's why we came to the conclusion that in CHICKEN core,
it's probably enough to just change the name of current-milliseconds
to reduce confusion and call it a day.

> It's the sub-millisecond range where the problems begin.
> 
> Would it make sense to write a really simple SRFI about these
> millisecond-precision procedures?

I think SRFI-174 should suffice.  Also, I don't have the energy to
follow all the new SRFIs being pumped out as part of R7RS anyway.

Cheers,
Peter


signature.asc
Description: PGP signature

More millisecond procedures

[Lassi Kortela]

Attached is a set of two patches.  The first one simply adds a
deprecation notice to current-milliseconds and adds the new procedure
current-process-milliseconds.


Racket also has `current-inexact-milliseconds`: "Returns the current 
time in milliseconds since midnight UTC, January 1, 1970. The result may 
contain fractions of a millisecond." 



In general, millisecond precision is probably attainable on most/all 
operating systems without too much fuss. It's the sub-millisecond range 
where the problems begin.


Would it make sense to write a really simple SRFI about these 
millisecond-precision procedures?

[PATCH] Deprecate current-milliseconds in favor of current-process-milliseconds and improve behaviour a bit [was: Re: Exposing subsecond precision in current-seconds]

[Peter Bex]

On Thu, Apr 30, 2020 at 12:11:31PM +0200, felix.winkelm...@bevuta.com wrote:
> > Yeah, like I said in my other message, we could use the name
> > current-process-milliseconds, like Racket;
> > https://docs.racket-lang.org/reference/time.html
> >
> > I like the name, because it's pretty clear what it does; it yields the
> > number of milliseconds the process has been running for.
> >
> > We can make it a procedure without arguments, because that part of the
> > Racket interface looks messy and confusing to me.
> 
> Yes, makes sense.

Attached is a set of two patches.  The first one simply adds a
deprecation notice to current-milliseconds and adds the new procedure
current-process-milliseconds.

While I was looking into this, I've noticed a few strange things.
First off, on Unix, C_startup_time_seconds uses just the second since
startup, ignoring the microseconds.  This leads to erratic behaviour:

$ csi -e -R chicken.time -p '(current-process-milliseconds)'
301
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
910
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
82
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
269
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
905
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
15

With the patch, we get sane behaviour:
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
23
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
24
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
26
$ csi -e -R chicken.time -p '(current-process-milliseconds)'
24

On Windows, we used to use clock(), but according to
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/clock?view=vs-2019
that function stops behaving sanely after 24.8 days of uptime,
at which point it starts returning -1.  This shouldn't usually
be a problem, but on a server running some long-running CHICKEN
process this could be an issue.

Besides, that also says CLOCKS_PER_SEC is defined as 1000 by
Microsoft.  So the logic in there doesn't make much sense since
it's defined as such, and C_NONUNIX is only true on MINGW32.

And if that code were triggered on non-Windows non-UNIX systems,
that would result in this undefined issue where the time since
system startup might be used.  To me it makes a lot more sense
to define current-process-milliseconds as time since process
startup, and drop the wishy-washy documentation that it's
time "since process or system startup".

Cheers,
Peter
From a53e6a707e34fa26f42a110b8948ef9710c0d86e Mon Sep 17 00:00:00 2001
From: Peter Bex 
Date: Sun, 3 May 2020 12:15:08 +0200
Subject: [PATCH 1/2] Deprecate current-milliseconds in favor of
 current-process-milliseconds

For consistency, a similar deprecation is also made for the underlying
C API.
---
 DEPRECATED   |  6 ++
 NEWS | 11 +--
 batch-driver.scm |  2 +-
 chicken.h|  5 -
 chicken.time.import.scm  |  1 +
 library.scm  |  6 +-
 manual/Module (chicken time) |  4 ++--
 runtime.c|  6 ++
 scheduler.scm|  6 +++---
 tcp.scm  | 10 +-
 tests/loopy-test.scm |  6 +++---
 tests/test.scm   | 10 +-
 types.db |  3 ++-
 13 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/DEPRECATED b/DEPRECATED
index 40756ce2..8f25faa8 100644
--- a/DEPRECATED
+++ b/DEPRECATED
@@ -1,6 +1,12 @@
 Deprecated functions and variables
 ==
 
+5.2.1
+- current-milliseconds and its C implementations C_milliseconds and
+  C_a_i_current_milliseconds have been deprecated in favor of
+  current-process_milliseconds, C_current_process_milliseconds and
+  C_a_i_current_process_milliseconds
+
 5.1.1
 
 - ##sys#check-exact and its C implementations C_i_check_exact and
diff --git a/NEWS b/NEWS
index 825fa8ab..67cc9f55 100644
--- a/NEWS
+++ b/NEWS
@@ -4,13 +4,20 @@
   - Fixed a bug where optimisations for `irregex-match?` would cause
 runtime errors due to the inlined specialisations not being
 fully-expanded (see #1690).
+  - current-milliseconds has been deprecated in favor of the name
+current-process-milliseconds, to avoid confusion due to naming
+of current-milliseconds versus current-seconds, which do something
+quite different.
 
 - Runtime system
   - Sleeping primordial thread doesn't forget mutations made to
 parameters in interrupt handlers anymore. (See #1638. Fix
 contributed by Sebastien Marie)
-- A feature corresponding to the word size is available
-   regardless of the word size (#1693)
+  - A feature corresponding to the word size is available
+regardless of the word size (#1693)
+  - Deprecated C_(a_i_current_)milliseconds in favor of
+C_(a_i_)current_process_milliseconds to match the Scheme-level
+deprecation of current-milliseconds.
 
 - Build system
   - 

Re: [PATCH] Mark identifiers used to collect profiling info as `bound-to-procedure'

[felix . winkelmann]

> Calls to `##sys#register-profile-info' and `set-profile-info-vector!`
> are inserted into the program when profiling is enabled, so they should
> be marked as procedures so the resulting call nodes are marked as safe,
> just like we do for the `##sys#profile-entry' and `exit' procedures.

Pushed as well.


felix

Re: [PATCH] Fix potential invalid argument error in `##sys#decompose-import'

[felix . winkelmann]

> This fixes a small bug in `##sys#decompose-import' where "spec" (which
> is a list) is passed to the `warn' procedure rather than "name" (which
> is a symbol, as expected). This leads to an invalid argument error
> arising from `symbol->string', e.g.
>

Pushed.


felix

Re: [PATCH] Fix potential invalid argument error in `##sys#decompose-import'

[felix . winkelmann]

> On Sun, May 03, 2020 at 11:44:00AM +0200, felix.winkelm...@bevuta.com wrote:
> > I'd love to apply your patches, but can you remove the "noprefix=true" 
> > option
> > from your .git/config, please? Apparently git am doesn't recognize this 
> > patch.
>
> git am -p0
>
> should do the trick. It told to `git apply' that there is no prefix.

Ah, thanks. Git never stops to find ways to annoy me...


felix

Re: [PATCH] Fix potential invalid argument error in `##sys#decompose-import'

[Sebastien Marie]

On Sun, May 03, 2020 at 11:44:00AM +0200, felix.winkelm...@bevuta.com wrote:
> I'd love to apply your patches, but can you remove the "noprefix=true" option
> from your .git/config, please? Apparently git am doesn't recognize this patch.

git am -p0

should do the trick. It told to `git apply' that there is no prefix.

-- 
Sebastien Marie

Re: [PATCH] Fix potential invalid argument error in `##sys#decompose-import'

[felix . winkelmann]

I'd love to apply your patches, but can you remove the "noprefix=true" option
from your .git/config, please? Apparently git am doesn't recognize this patch.


felix