Re: [Chicken-hackers] Regarding the hide declaration, #1376

[megane]

Evan Hanson  writes:

>> Which foo should be hidden?  Both, or none?
>
> Personally, I'd expect the following behaviour, depending on where the
> declaration appears:
>

I did some tests with and without the POC patch I sent yesterday.

Some observations:
(these are the only variables with differing behaviour, see the
attached diff)

- foo1, bar1, foo2, bar2 are now hidden correctly
- foo7, bar7, foo8, bar8 are now hidden incorrectly. This should be easy
to fix by changing the visibility value of the identifier in the export macro.



foo.scm
Description: Binary data


without.out
Description: Binary data


with.out
Description: Binary data


diff
Description: Binary data
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] [PATCH] Moving some things from library.scm and eval.scm to internal.scm

[Evan Hanson]

On 2017-06-05 12:11, Peter Bex wrote:
> On Mon, Jun 05, 2017 at 05:53:32PM +1200, Evan Hanson wrote:
> > On 2017-06-04 13:53, Peter Bex wrote:
> > > Regarding time specifically, there are not many stand-alone programs
> > > that will use this macro, I think.  It's more a thing for benchmarks
> > > and such, so I'm not even sure it must be part of library.  It probably
> > > was in there because the macro was in the "chicken" module.  There's a
> > > lot of stuff that's much less optional than "time" (as in, used by day
> > > to day programs) that lives in other units like file.scm.
> > 
> > True. I do think the support code for `time` would be good to move, if
> > we can sort out the dependency issues.
> 
> What exactly are the dependency issues you're concerned about?  AFAIK
> there's a simple rule: if you use the "time" macro, you'll need to
> include internal.scm into your program.  That's pretty straightforward,
> IMHO.  Besides, you also need to know that you need to include, say,
> extras.scm when using "randomize" from (chicken random).
> 
> The only difference here is that it's the _expansion_ that requires the
> unit rather than the direct call.  Is that the issue?

Basically, yes. In this case the compiler won't take care of loading the
internal unit for you, so you're left with an unmet dependency unless
you know to load that unit explicitly, or happen to do so incidentally.

Currently, things are in a good state because: you can't use `time`
until you import "chicken", importing "chicken" loads the library unit,
the library unit provides the support procedures needed by `time`, and
everything's groovy. By moving the support procedures out of library
this fails to be the case, so from my point of view it's a regression.

Additionally, this is a problem that won't even be caught at compile
time. You'll just end up with an unbound value in the expansion of
`time`, resulting in with segfaults and the like.

> Maybe we can fix the situation by adding something to the manual to
> indicate which units map to which modules.  This would solve this
> problem more generally, including the "randomize" example I gave above.

Yeah, I do think we should include that information in the manual. We
can put it in a little note in the corner of each module page or
something, similarly to how rubydoc.info has a "Defined in: foo.rb"
section.

However, if we end up putting macro support code in a different unit
than that loaded by the module exporting the macro itself, do we note
the runtime or compile-time unit? Both? I really think it's simpler to
just keep these things together for now, until we can add some smarts to
the compiler.

> We still haven't decided on which module "time" should go into, so maybe
> that's part of the problem (it's missing completely from c-l-r).

True... Apparently I put it where it is, though I don't remember doing
that so I probably just did what seemed right at the time (heh). It's
not at all similar to the other procedures in that module, but it is
clearly time-related. Are there any other benchmarking-related things
that could make a happy family together? Doesn't really look like it,
but maybe that's not a problem... Put it in extras? Eggify it? Leave it
where it is? Any other hackers have thoughts about this?

Cheers,

Evan


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Regarding the hide declaration, #1376

[Evan Hanson]

Hey all,

On 2017-06-05 11:43, Peter Bex wrote:
> On Mon, Jun 05, 2017 at 06:09:54PM +1200, Evan Hanson wrote:
> > Is that right? Personally, I'd rather make (declare (hide ...)) simply
> > do the right thing -- the right thing being the behaviour you originally
> > expected when filing #1376 -- than add a new type of declaration or
> > module syntax.
> > 
> > I think this is similar to what Peter has said on that ticket, so a
> > patch of that sort would be very welcome.
> 
> This is slightly more complicated due to declare being module-unaware.
> 
> (declare (hide foo))
> 
> (module a * (import chicken scheme) (define foo 1))
> (module b * (import chicken scheme) (define foo 2))
> 
> Which foo should be hidden?  Both, or none?

Personally, I'd expect the following behaviour, depending on where the
declaration appears:

  [1]
  (module a * (import chicken scheme) [2] (define foo 1) [3])
  [4]
  (module b * (import chicken scheme) [5] (define foo 2) [6])
  [7]
  (import a)
  [8]
  (import b)
  [9]

  [1]: foo (a noop as things work currently, or an error as you propose)
  [2]: a#foo
  [3]: a#foo
  [4]: foo
  [5]: b#foo
  [6]: b#foo
  [7]: foo
  [8]: a#foo
  [9]: b#foo

I suspect this is also the current behaviour, but I'd need to check to
confirm.

> I think the sane thing to do here is to make it error out,
> considering there's no top-level foo to hide

I think the issue of whether or not to make declarations error out when
one of the hidden identifiers doesn't exist is a separate one from
whether it should affect module exports. I slightly prefer the current,
more forgiving behaviour, but don't feel too strongly about it. If we do
make a change, however, I think it should affect all declarations that
take identifier names (where it makes sense), not just `hide'.

> make it work like this:
> 
> (module a *
>(import chicken scheme)
>(define foo 1))
> (module b *
>(declare (hide foo))
>(import chicken scheme) (define foo 2))
> 
> So here a#foo is visible and exported, while b#foo is hidden and
> not exported.

+1, I agree.

Another question is whether this behaviour should apply in eval'd code
as well. I think no, declarations should still be fully ignored in eval.

Cheers,

Evan


signature.asc
Description: PGP signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] [PATCH] Make setting of the file modification time saner

[Kooda]

Both changes signed off and pushed. :)

Sorry for the delay.

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Regarding the hide declaration, #1376

[megane]

Evan Hanson  writes:

> If I understand correctly, this would effectively be an "unexport" of
> sorts, like so:

Yes, that's what I was thinking.
>
> Is that right? Personally, I'd rather make (declare (hide ...)) simply
> do the right thing -- the right thing being the behaviour you originally
> expected when filing #1376 -- than add a new type of declaration or
> module syntax.

Here's a quick POC doing this.

It filters out all identifiers marked 'hidden from the syntax-exports
and variable exports lists in ##sys#finalize-module.

It doesn't touch the indirect exports list. I'm not sure how that should
be handled.

Applies to the 4.12.0 tarball.

If you think this is a valid approach I can clean it up and add some tests.

diff --git a/modules.scm b/modules.scm
index aeeabcf..b5788b1 100644
--- a/modules.scm
+++ b/modules.scm
@@ -447,23 +447,36 @@
 	 (dlist (module-defined-list mod))
 	 (elist (module-exist-list mod))
 	 (missing #f)
+	 ;; from support.scm
+	 (variable-hidden? (lambda (sym)
+ (print "hidden? " sym " "
+	(eq? (##sys#get sym '##compiler#visibility) 'hidden))
+ (eq? (##sys#get sym '##compiler#visibility) 'hidden)))
+	 (id-hidden? (lambda (id)
+			   (variable-hidden? (##sys#module-rename id name
 	 (sdlist (map (lambda (sym) (assq (car sym) (##sys#macro-environment)))
 			  (module-defined-syntax-list mod)))
 	 (sexports
-	  (if (eq? #t explist)
-		  (merge-se (module-sexports mod) sdlist)
-		  (let loop ((me (##sys#macro-environment)))
-		(cond ((null? me) '())
-			  ((##sys#find-export (caar me) mod #f)
-			   (cons (car me) (loop (cdr me
-			  (else (loop (cdr me)))
+	  (let loop [(sxs (if (eq? #t explist)
+  (merge-se (module-sexports mod) sdlist)
+  (let loop ((me (##sys#macro-environment)))
+(cond ((null? me) '())
+	  ((##sys#find-export (caar me) mod #f)
+	   (cons (car me) (loop (cdr me
+	  (else (loop (cdr me)))]
+		(cond
+		 ((null? sxs) '())
+		 ((id-hidden? (caar sxs)) (loop (cdr sxs)))
+		 (else
+		  (cons (car sxs) (loop (cdr sxs)))
 	 (vexports
 	  (let loop ((xl (if (eq? #t explist) elist explist)))
 		(if (null? xl)
 		'()
 		(let* ((h (car xl))
 			   (id (if (symbol? h) h (car h
-		  (if (assq id sexports) 
+		  (if (or (assq id sexports)
+			  (id-hidden? id)) 
 			  (loop (cdr xl))
 			  (cons 
 			   (cons 
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] [PATCH] Moving some things from library.scm and eval.scm to internal.scm

[Peter Bex]

On Mon, Jun 05, 2017 at 05:53:32PM +1200, Evan Hanson wrote:
> Hey Peter,
> 
> On 2017-06-04 13:53, Peter Bex wrote:
> > Regarding time specifically, there are not many stand-alone programs
> > that will use this macro, I think.  It's more a thing for benchmarks
> > and such, so I'm not even sure it must be part of library.  It probably
> > was in there because the macro was in the "chicken" module.  There's a
> > lot of stuff that's much less optional than "time" (as in, used by day
> > to day programs) that lives in other units like file.scm.
> 
> True. I do think the support code for `time` would be good to move, if
> we can sort out the dependency issues.

What exactly are the dependency issues you're concerned about?  AFAIK
there's a simple rule: if you use the "time" macro, you'll need to
include internal.scm into your program.  That's pretty straightforward,
IMHO.  Besides, you also need to know that you need to include, say,
extras.scm when using "randomize" from (chicken random).

The only difference here is that it's the _expansion_ that requires the
unit rather than the direct call.  Is that the issue?

Maybe we can fix the situation by adding something to the manual to
indicate which units map to which modules.  This would solve this
problem more generally, including the "randomize" example I gave above.

We still haven't decided on which module "time" should go into, so maybe
that's part of the problem (it's missing completely from c-l-r).

> It's also probably possible to
> make the compiler generate code to load units as they're required by
> macro expansions, but that's sure to have other implications so I'm not
> super keen to go there right now.

I agree that would be nice but is a bit too complex right now.  Maybe
we'll take a look at this for 5.1 or 5.2.

Cheers,
Peter


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Regarding the hide declaration, #1376

[Peter Bex]

On Mon, Jun 05, 2017 at 06:09:54PM +1200, Evan Hanson wrote:
> Is that right? Personally, I'd rather make (declare (hide ...)) simply
> do the right thing -- the right thing being the behaviour you originally
> expected when filing #1376 -- than add a new type of declaration or
> module syntax.
> 
> I think this is similar to what Peter has said on that ticket, so a
> patch of that sort would be very welcome.

This is slightly more complicated due to declare being module-unaware.

(declare (hide foo))

(module a * (import chicken scheme) (define foo 1))
(module b * (import chicken scheme) (define foo 2))

Which foo should be hidden?  Both, or none?

I think the sane thing to do here is to make it error out,
considering there's no top-level foo to hide, and make it
work like this:

(module a *
   (import chicken scheme)
   (define foo 1))
(module b *
   (declare (hide foo))
   (import chicken scheme) (define foo 2))

So here a#foo is visible and exported, while b#foo is hidden and
not exported.

Cheers,
Peter


signature.asc
Description: Digital signature
___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers

Re: [Chicken-hackers] Regarding the hide declaration, #1376

[Evan Hanson]

Hi megane,

On 2017-05-28 16:12, megane wrote:
> I was thinking maybe we could leave the declarations as they are and add
> an explicit counterpart for export. The implementation may be pretty
> easy to do by just updating the module export and what not lists. Maybe
> call the new syntax hide-export.

If I understand correctly, this would effectively be an "unexport" of
sorts, like so:

(module m1 *
  (import chicken scheme)
  (define (foo) 1)
  (define (bar) 2)
  (unexport foo)) ; or (declare (hide-export ...))

(import m1)
(print (foo)) ; => 1
(print (bar)) ; => error, since bar was "unexported"

Is that right? Personally, I'd rather make (declare (hide ...)) simply
do the right thing -- the right thing being the behaviour you originally
expected when filing #1376 -- than add a new type of declaration or
module syntax.

I think this is similar to what Peter has said on that ticket, so a
patch of that sort would be very welcome.

Cheers,

Evan

___
Chicken-hackers mailing list
Chicken-hackers@nongnu.org
https://lists.nongnu.org/mailman/listinfo/chicken-hackers