public inbox for kawa@sourceware.org
 help / color / mirror / Atom feed
From: Matthieu Vachon <matthieu.o.vachon@gmail.com>
To: "kawa@sourceware.org" <kawa@sourceware.org>
Subject: Patch for wrong no declaration seen for command-line-arguments
Date: Thu, 12 Sep 2013 03:42:00 -0000	[thread overview]
Message-ID: <CAOTvmo=VWjMv2Etk9gagPTAa2OgnM-3XG9USdLWrARbJ22dTcg@mail.gmail.com> (raw)

[-- 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

             reply	other threads:[~2013-09-12  3:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-12  3:42 Matthieu Vachon [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOTvmo=VWjMv2Etk9gagPTAa2OgnM-3XG9USdLWrARbJ22dTcg@mail.gmail.com' \
    --to=matthieu.o.vachon@gmail.com \
    --cc=kawa@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).