* Patch for wrong no declaration seen for command-line-arguments
@ 2013-09-12 3:42 Matthieu Vachon
2013-09-12 20:09 ` Charles Turner
2013-09-16 5:49 ` Per Bothner
0 siblings, 2 replies; 7+ messages in thread
From: Matthieu Vachon @ 2013-09-12 3:42 UTC (permalink / raw)
To: kawa
[-- Attachment #1: Type: text/plain, Size: 491 bytes --]
Hi Per,
Here my try at fixing the incorrect warning that occurs when
generating a main method and using the `command-line-arguments`. It's
probably not the optimal way to fix the issue but I hope it is good
enough to be included in the trunk.
I added two test cases but I'm unsure if the syntax I used is right.
Talking about that, what is the procedure to run the test suite? I
would like to know so I can test my patches when developing them prior
to sending them to you.
Regards,
Matt
[-- Attachment #2: fix-wrong-command-line-arguments-warning.patch --]
[-- Type: application/octet-stream, Size: 3437 bytes --]
Index: gnu/expr/ChangeLog
===================================================================
--- gnu/expr/ChangeLog (revision 7584)
+++ gnu/expr/ChangeLog (working copy)
@@ -1,3 +1,10 @@
+2013-09-11 Matthieu Vachon <matthieu.o.vachon@gmail.com>
+
+ * FindCapturedVars.java (visitReferenceExp): Added a check
+ to see if unknown declaration will be bound at runtime
+ which is the case for command-line-arguments when a main
+ method needs to be generated.
+
2013-09-07 Per Bothner <per@bothner.com>
* PrimProcedure.java (mostSpecific): Moved from MethodProc.
Index: gnu/expr/FindCapturedVars.java
===================================================================
--- gnu/expr/FindCapturedVars.java (revision 7584)
+++ gnu/expr/FindCapturedVars.java (working copy)
@@ -476,9 +476,10 @@ public class FindCapturedVars extends ExpExpVisitor<Void>
exp.isProcedureName());
exp.setBinding(decl);
}
- if (decl.getFlag(Declaration.IS_UNKNOWN))
+ if (decl.getFlag(Declaration.IS_UNKNOWN) && !isBoundAtRuntime(decl, comp)) {
maybeWarnNoDeclarationSeen(exp.getSymbol(), exp.isProcedureName(),
comp, exp);
+ }
capture(exp.contextDecl(), decl);
return exp;
@@ -540,4 +541,20 @@ public class FindCapturedVars extends ExpExpVisitor<Void>
return super.visitSetExp(exp, ignored);
}
+ /**
+ * Returns a boolean determining if a declaration is to be bound
+ * into the environment at runtime only. This is the case for the
+ * special command-line-arguments symbol which is bound to arguments
+ * only at runtime.
+ *
+ * @param declaration The declaration to check for runtime injection.
+ * @param compilation The compilation unit associated to the declaration.
+ *
+ * @return true if declaration will be bound at runtime, false otherwise.
+ */
+ boolean isBoundAtRuntime(Declaration declaration, Compilation compilation)
+ {
+ return compilation.generateMainMethod() &&
+ "command-line-arguments".equals(declaration.getName());
+ }
}
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 7584)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,13 @@
+2013-09-13 Matthieu Vachon <matthieu.o.vachon@gmail.com>
+
+ * unknown1.scm: New test to check that no warning
+ is issued when generating a main method and using
+ command-line-arguments.
+
+ * unknown2.scm: New test to check that a warning
+ is issued when not generating a main method and using
+ command-line-arguments.
+
2013-09-10 Matthieu Vachon <matthieu.o.vachon@gmail.com>
Per Bothner <per@bothner.com>
Index: testsuite/unknown1.scm
===================================================================
--- testsuite/unknown1.scm (revision 0)
+++ testsuite/unknown1.scm (working copy)
@@ -0,0 +1,5 @@
+(module-compile-options main: #t)
+
+;; No warning should be generated
+(format #t "Argument count: ~a~%" (vector-length command-line-arguments))
+;; Output: Argument count: 0
Index: testsuite/unknown2.scm
===================================================================
--- testsuite/unknown2.scm (revision 0)
+++ testsuite/unknown2.scm (working copy)
@@ -0,0 +1,3 @@
+;; No main method, a warning should be issued
+(vector-length command-line-arguments)
+;; Diagnostic: unknown2.scm:2:16: warning - no declaration seen for command-line-arguments
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Patch for wrong no declaration seen for command-line-arguments
2013-09-12 3:42 Patch for wrong no declaration seen for command-line-arguments Matthieu Vachon
@ 2013-09-12 20:09 ` Charles Turner
2013-09-12 20:20 ` Matthieu Vachon
2013-09-16 5:49 ` Per Bothner
1 sibling, 1 reply; 7+ messages in thread
From: Charles Turner @ 2013-09-12 20:09 UTC (permalink / raw)
To: Matthieu Vachon; +Cc: kawa
Hi Matt,
I can't comment on the patch as I haven't been following along.
The test scripts you've written can be installed by adding the
filenames (in this case unknown1.scm and unknown2.scm) to the
testsuite/Makefile.am file.Specifically, add them to the variable
SCRIPTS_TO_RUN. You then need to regenerate the Makefiles using
autotools. I'm no expert, but aclocal && autoreconf has worked for me
in the past from the top-level kawa directory. If you then do a make
check, your tests will be run. For your patch, I got the following
results:
# unknown1.scm passes
FAIL unknown2.scm: expected more diagnostics: \Qunknown2.scm:2:16:
warning - no declaration seen for command-line-arguments\E
# unknown2.scm fails
Which will require further investigation.
Hope that helps,
Charles.
On 12/09/2013, Matthieu Vachon <matthieu.o.vachon@gmail.com> wrote:
> Hi Per,
>
> Here my try at fixing the incorrect warning that occurs when
> generating a main method and using the `command-line-arguments`. It's
> probably not the optimal way to fix the issue but I hope it is good
> enough to be included in the trunk.
>
> I added two test cases but I'm unsure if the syntax I used is right.
> Talking about that, what is the procedure to run the test suite? I
> would like to know so I can test my patches when developing them prior
> to sending them to you.
>
> Regards,
> Matt
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Patch for wrong no declaration seen for command-line-arguments
2013-09-12 20:09 ` Charles Turner
@ 2013-09-12 20:20 ` Matthieu Vachon
0 siblings, 0 replies; 7+ messages in thread
From: Matthieu Vachon @ 2013-09-12 20:20 UTC (permalink / raw)
To: Charles Turner; +Cc: kawa
Hi Charles,
Thank you for the procedure. I'm going to check this out later tonight.
As for the failed tests, I'm going to take a look.
Regards,
Matt
On Thu, Sep 12, 2013 at 4:09 PM, Charles Turner <chturne@gmail.com> wrote:
> Hi Matt,
>
> I can't comment on the patch as I haven't been following along.
>
> The test scripts you've written can be installed by adding the
> filenames (in this case unknown1.scm and unknown2.scm) to the
> testsuite/Makefile.am file.Specifically, add them to the variable
> SCRIPTS_TO_RUN. You then need to regenerate the Makefiles using
> autotools. I'm no expert, but aclocal && autoreconf has worked for me
> in the past from the top-level kawa directory. If you then do a make
> check, your tests will be run. For your patch, I got the following
> results:
>
> # unknown1.scm passes
> FAIL unknown2.scm: expected more diagnostics: \Qunknown2.scm:2:16:
> warning - no declaration seen for command-line-arguments\E
> # unknown2.scm fails
>
> Which will require further investigation.
>
> Hope that helps,
> Charles.
>
>
> On 12/09/2013, Matthieu Vachon <matthieu.o.vachon@gmail.com> wrote:
>> Hi Per,
>>
>> Here my try at fixing the incorrect warning that occurs when
>> generating a main method and using the `command-line-arguments`. It's
>> probably not the optimal way to fix the issue but I hope it is good
>> enough to be included in the trunk.
>>
>> I added two test cases but I'm unsure if the syntax I used is right.
>> Talking about that, what is the procedure to run the test suite? I
>> would like to know so I can test my patches when developing them prior
>> to sending them to you.
>>
>> Regards,
>> Matt
>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Patch for wrong no declaration seen for command-line-arguments
2013-09-12 3:42 Patch for wrong no declaration seen for command-line-arguments Matthieu Vachon
2013-09-12 20:09 ` Charles Turner
@ 2013-09-16 5:49 ` Per Bothner
2013-09-16 17:16 ` Jamison Hope
1 sibling, 1 reply; 7+ messages in thread
From: Per Bothner @ 2013-09-16 5:49 UTC (permalink / raw)
To: Matthieu Vachon; +Cc: kawa
[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]
On 09/11/2013 08:42 PM, Matthieu Vachon wrote:
> Hi Per,
>
> Here my try at fixing the incorrect warning that occurs when
> generating a main method and using the `command-line-arguments`. It's
> probably not the optimal way to fix the issue but I hope it is good
> enough to be included in the trunk.
Thanks for prodding me into action. Your patch makes me think
that having a special case for command-line-arguments is the wrong approach.
Please try the attached patch instead. That is a simpler/cleaner
solution.
You might also consider using the (command-line) procedure.
It is specified by R6RS and R7RS so is more portable.
It also gives you the "command" used to invoke the application
(roughly argv[0]). The attached patch provides a more informative
result for (car (command-line)) - rather than plain "kawa".
I'd be interested in feedback in how portable it is: Windows,
MacOS, non-Sun/Oracle VMs (including ones that don't set the
sun.java.command property). (I also working on updating the
documentation.)
--
--Per Bothner
per@bothner.com http://per.bothner.com/
[-- Attachment #2: command-line-args.patch --]
[-- Type: text/x-patch, Size: 3513 bytes --]
Index: gnu/expr/ApplicationMainSupport.java
===================================================================
--- gnu/expr/ApplicationMainSupport.java (revision 7583)
+++ gnu/expr/ApplicationMainSupport.java (working copy)
@@ -43,7 +43,6 @@
public static void setArgs (String[] args, int arg_start)
{
int nargs = args.length - arg_start;
- Object[] array = new Object[nargs];
if (arg_start == 0)
commandLineArgArray = args;
else
@@ -53,12 +52,10 @@
strings[i] = args[i+arg_start];
commandLineArgArray = strings;
}
- for (int i = nargs; --i >= 0; )
- array[i] = new FString (args[i + arg_start]);
- commandLineArguments = new FVector (array); // FIXME scsh has list
- // FIXME scsh also has command-line proc
- Environment.getCurrent().put("command-line-arguments",
- commandLineArguments);
+
+ Object[] array = new Object[nargs];
+ System.arraycopy(args, arg_start, array, 0, nargs);
+ commandLineArguments = new ConstVector(array); // FIXME scsh has list
}
public static boolean processSetProperty (String arg)
Index: kawa/lib/rnrs/programs.scm
===================================================================
--- kawa/lib/rnrs/programs.scm (revision 7583)
+++ kawa/lib/rnrs/programs.scm (working copy)
@@ -3,8 +3,32 @@
(require <kawa.lib.prim_syntax>)
(define (command-line) :: list
- (let ((arg0 "kawa")) ;; FIXME
- (cons arg0 (gnu.lists.LList:makeList gnu.expr.ApplicationMainSupport:commandLineArgArray 0))))
+ (let* ((rest
+ (gnu.lists.LList:makeList
+ gnu.expr.ApplicationMainSupport:commandLineArgArray 0))
+ (command ::java.lang.String
+ (try-catch
+ (let ((raw (java.lang.System:getProperty
+ "sun.java.command")))
+ (if (eq? raw #!null) #!null
+ ;; Strip off the tail of the property value that
+ ;; duplicates the rest value.
+ (let* ((frest (format #f "~{ ~a~}" rest))
+ (rlen (raw:length))
+ (flen (frest:length))
+ (alen (- rlen flen)))
+ (cond ((= flen 0)
+ raw)
+ ;; Sanity check
+ ((and (>= alen 0)
+ ((raw:substring alen):equals frest))
+ (raw:substring 0 alen))
+ (else
+ #!null)))))
+ (exp java.lang.Throwable #!null)))
+ (arg0 (if (eq? command #!null) "kawa"
+ ("java ":concat command))))
+ (cons arg0 rest)))
(define (exit #!optional (code 0)) :: #!void
(invoke-static <output-port> 'runCleanups)
Index: kawa/standard/Scheme.java
===================================================================
--- kawa/standard/Scheme.java (revision 7583)
+++ kawa/standard/Scheme.java (working copy)
@@ -565,6 +565,8 @@
defProcStFld("exit", "kawa.lib.rnrs.programs");
defProcStFld("command-line", "kawa.lib.rnrs.programs");
+ defAliasStFld("command-line-arguments",
+ "gnu.expr.ApplicationMainSupport", "commandLineArguments");
defProcStFld("bitwise-arithmetic-shift",
"gnu.kawa.functions.BitwiseOp", "ashift");
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Patch for wrong no declaration seen for command-line-arguments
2013-09-16 5:49 ` Per Bothner
@ 2013-09-16 17:16 ` Jamison Hope
2013-09-18 13:19 ` Matthieu Vachon
0 siblings, 1 reply; 7+ messages in thread
From: Jamison Hope @ 2013-09-16 17:16 UTC (permalink / raw)
To: kawa
[-- Attachment #1: Type: text/plain, Size: 924 bytes --]
On Sep 16, 2013, at 1:48 AM, Per Bothner <per@bothner.com> wrote:
> The attached patch provides a more informative
> result for (car (command-line)) - rather than plain "kawa".
> I'd be interested in feedback in how portable it is: Windows,
> MacOS, non-Sun/Oracle VMs (including ones that don't set the
> sun.java.command property). (I also working on updating the
> documentation.)
With Oracle JDK 7u40 on Mac OS X 10.8, I get:
$ java -jar kawa-1.13.1.jar
#|kawa:1|# (write (car (command-line)))
"java kawa-1.13.1.jar"
$ java kawa.repl
#|kawa:1|# (write (car (command-line)))
"java kawa.repl"
$ cat /tmp/test.scm
(write (car (command-line)))
(newline)
$ java -jar kawa-1.13.1.jar -f /tmp/test.scm
"java kawa-1.13.1.jar -f /tmp/test.scm"
$ java kawa.repl -f /tmp/test.scm
"java kawa.repl -f /tmp/test.scm"
In other words, it seems to do what you'd expect.
--
Jamison Hope
The PTR Group
www.theptrgroup.com
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 204 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Patch for wrong no declaration seen for command-line-arguments
2013-09-16 17:16 ` Jamison Hope
@ 2013-09-18 13:19 ` Matthieu Vachon
2013-09-18 18:21 ` Per Bothner
0 siblings, 1 reply; 7+ messages in thread
From: Matthieu Vachon @ 2013-09-18 13:19 UTC (permalink / raw)
To: kawa
Hi Per,
I tested your patch and it is really more clearer than what I first
proposed. Very clean.
No more `command-line-arguments` warning when using the patch.
As for the new information for (car (command-line)), it ran the same
tests Jamison did but with the platforms and Java version I had access
to. The raw results can be found at the end of the email. From my
tests, java 5 support is not working correctly. It seems that property
`sun.java.command` is not available on Java 5:
$ java kawa.repl
#|kawa:1|# (java.lang.System:getProperty "sun.java.command")
#!null
Did not searched that much but did not found any information on
"sun.java.command" for java 5.
Regards,
Matt
P.S. The results:
Windows 7 x64 - Java 1.5.0_22 x64
Kawa compiled with Java 1.5.0_22 x64
=========================================
$ java -jar kawa-1.13.1.jar
#|kawa:1|# (write (car (command-line)))
"kawa"
$ java kawa.repl
#|kawa:1|# (write (car (command-line)))
"kawa"
$ java -jar kawa-1.13.1.jar -f test.scm
"kawa"
$ java kawa.repl -f test.scm
"kawa"
Windows 7 x64 - Java 1.6.0_38 x64
Kawa compile with Java 1.6.0_38 x64
=========================================
$ java -jar kawa-1.13.1.jar
#|kawa:1|# (write (car (command-line)))
"java kawa-1.13.1.jar"
$ java kawa.repl
#|kawa:1|# (write (car (command-line)))
"java kawa.repl"
$ java -jar kawa-1.13.1.jar -f test.scm
"java kawa-1.13.1.jar -f test.scm"
$ java kawa.repl -f test.scm
"java kawa.repl -f test.scm"
Windows 7 x64 - Java 1.7.0.11 x64
Kawa compile with Java 1.7.0.11 x64
=========================================
$ java -jar kawa-1.13.1.jar
#|kawa:1|# (write (car (command-line)))
"java kawa-1.13.1.jar"
$ java kawa.repl
#|kawa:1|# (write (car (command-line)))
"java kawa.repl"
$ java -jar kawa-1.13.1.jar -f test.scm
"java kawa-1.13.1.jar -f test.scm"
$ java kawa.repl -f test.scm
"java kawa.repl -f test.scm"
CentOS 6.3 x64 - Java 1.6.0_43 x64
Kawa compile with Java 1.6.0_43 x64
=========================================
$ java -jar kawa-1.13.1.jar
#|kawa:1|# (write (car (command-line)))
"java kawa-1.13.1.jar"
$ java kawa.repl
#|kawa:1|# (write (car (command-line)))
"java kawa.repl"
$ java -jar kawa-1.13.1.jar -f test.scm
"java kawa-1.13.1.jar -f test.scm"
$ java kawa.repl -f test.scm
"java kawa.repl -f test.scm"
CentOS 6.3 x64 - OpenJDK 1.6.0_24 x64 (IcedTea6 1.11.11.90)
Kawa compile with OpenJDK 1.6.0_24 x64 (IcedTea6 1.11.11.90)
=========================================
$ java -jar kawa-1.13.1.jar
#|kawa:1|# (write (car (command-line)))
"java kawa-1.13.1.jar"
$ java kawa.repl
#|kawa:1|# (write (car (command-line)))
"java kawa.repl"
$ java -jar kawa-1.13.1.jar -f test.scm
"java kawa-1.13.1.jar -f test.scm"
$ java kawa.repl -f test.scm
"java kawa.repl -f test.scm"
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Patch for wrong no declaration seen for command-line-arguments
2013-09-18 13:19 ` Matthieu Vachon
@ 2013-09-18 18:21 ` Per Bothner
0 siblings, 0 replies; 7+ messages in thread
From: Per Bothner @ 2013-09-18 18:21 UTC (permalink / raw)
To: Matthieu Vachon; +Cc: kawa
On 09/18/2013 06:19 AM, Matthieu Vachon wrote:
> I tested your patch and it is really more clearer than what I first
> proposed. Very clean.
>
> No more `command-line-arguments` warning when using the patch.
Thanks - I checked the patch in.
--
--Per Bothner
per@bothner.com http://per.bothner.com/
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-09-18 18:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-12 3:42 Patch for wrong no declaration seen for command-line-arguments Matthieu Vachon
2013-09-12 20:09 ` Charles Turner
2013-09-12 20:20 ` Matthieu Vachon
2013-09-16 5:49 ` Per Bothner
2013-09-16 17:16 ` Jamison Hope
2013-09-18 13:19 ` Matthieu Vachon
2013-09-18 18:21 ` Per Bothner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).