<< Back to previous view

[CLJ-939] Exceptions thrown in the top level ns form are reported without file or line number Created: 24/Feb/12  Updated: 22/Nov/13  Resolved: 22/Nov/13

Status: Closed
Project: Clojure
Component/s: None
Affects Version/s: Release 1.3, Release 1.4, Release 1.5, Release 1.6
Fix Version/s: Release 1.6

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: errormsgs

Attachments: File 0001-report-load-exceptions-with-file-and-line.diff     File 0002-report-load-exceptions-with-file-and-line.diff     Text File clj-939-report-load-exceptions-with-file-and-line-patch-v2.txt     File clj-939-report-load-exceptions-with-file-and-line-patch-v3.diff     Text File clj-939-report-load-exceptions-with-file-and-line-patch-v3.txt     File clj-939-v4.diff     File screen-clj-939.org    
Patch: Code
Approval: Ok

 Description   

If there is an error in the `ns` form, an exception is thrown, which is not caught in `load`.

For example, with an invalid :only clause;

(ns clj14.myns
  (:use
   [clojure.core :only seq]))

With the latest Clojure master as of Aug 24 2013, this generates the following exception, with no source file or line number except the one shown in clojure.core:

Exception :only/:refer value must be a sequential collection of symbols  clojure.core/refer (core.clj:3854)

You can find a source file in your project if you painstakingly search through the stack trace, but it would be nice if it jumped out at you in the exception itself.

Patch: clj-939-v4.diff

Approach: The latest patch does not modify the behavior of any other exceptions thrown by the compiler. The throw-if function is only used in the load-related functions: this patch changes it to throw CompilerException instead of Exception. As a result, exceptions which occur while loading files will be decorated with file names and line/column numbers. The line/column numbers are not always accurate (see notes in attachment screen-clj-939.org) but the file names are correct.

This is an incremental improvement to compile-time error messages, but it does not solve some of the fundamental problems with reporting correct line/column numbers from errors thrown by the compiler.

Screened by: Stuart Sierra (re-screened by Alex for the specific comments by Rich)



 Comments   
Comment by Hugo Duncan [ 25/Feb/12 8:26 AM ]

Corrected patch

Comment by Andy Fingerhut [ 09/Mar/12 9:26 AM ]

Patch 0001-report-load-exception-with-file-and-line.diff fails build. Patch 0002-report-load-exception-with-file-and-line.diff applies, builds, and tests cleanly as of March 9, 2012. Hugo has signed a CA.

Comment by Andy Fingerhut [ 05/Oct/12 8:13 AM ]

clj-939-report-load-exceptions-with-file-and-line-patch-v2.txt dated Oct 5 2012 is intended to be an update to Hugo Duncan's patch 0002-report-load-exceptions-with-file-and-line.diff dated Feb 25 2012. Because of Brandon Bloom's recently commited patch adding column numbers in addition to line numbers, this is not simply updating some lines of context, but I think it is correct. It would be good if Hugo could take a look at it and confirm.

Comment by Stuart Sierra [ 09/Nov/12 9:38 AM ]

Screened.

The error messages are better than what we had before. The line/column numbers are not particularly informative, probably because ns is a macro.

Comment by Rich Hickey [ 13/Nov/12 3:37 PM ]

This patch doesn't change the reporting on any other (e.g. nested) exceptions? It looks like it might.

Comment by Alex Miller [ 28/Jul/13 10:12 PM ]

This issue has somewhat strange history. Starting over with Triaged so it can go through release assignment and patch process again.

Comment by Andy Fingerhut [ 25/Aug/13 6:01 AM ]

Patch clj-939-report-load-exceptions-with-file-and-line-patch-v3.txt dated Aug 25 2013 started based upon Hugo Duncan's
clj-939-report-load-exceptions-with-file-and-line-patch-v2.txt patch.

It was enhanced to avoid wrapping a CompilerException within another CompilerException first. That always gave line 1 and column 1 for all load/require/use exceptions, so I modified clojure.core/throw-if to throw a CompilerException instead. The line/column numbers with this approach are now usually those of the beginning of the enclosing ns form, if these operations are caused by an ns form.

I also added a little extra info to one exception message, and a new condition in which an exception is thrown if a require/use operand is neither a libspec nor a prefix list.

Comment by Stuart Sierra [ 27/Sep/13 9:45 AM ]

Screening in progress.

Comment by Stuart Sierra [ 27/Sep/13 12:57 PM ]

Screened. Notes in attachment screen-clj-939.org

Comment by Rich Hickey [ 25/Oct/13 7:06 AM ]

this (surreptitiously? adds a new test for vector or list. Please submit patches that address one specific issue. There's no reason for this concrete check, but if you think there is make another ticket.

Comment by Andy Fingerhut [ 25/Oct/13 9:06 AM ]

To: Operative Sierra
From: Chaos

Agent Hickey thwarted world domination plans yet again. Recommend plan omicron. InfoSec dept provided clj-939-v4.diff - should enable our insidious changes to slip past his Detect-O-Tron. Good luck, operative. Message ends.

Comment by Alex Miller [ 25/Oct/13 9:58 AM ]

Seems like the (declare list?) can go away too?

Comment by Andy Fingerhut [ 25/Oct/13 10:49 AM ]

Good catch, Alex. More coffee required here. Uploaded new clj-939-v4.diff in place of the previous one.

Comment by Alex Miller [ 25/Oct/13 10:59 AM ]

Verified questioned bits have been removed and patch still applies cleanly and tests pass. Re-marking Screened with new patch.

Generated at Sat Sep 20 13:02:28 CDT 2014 using JIRA 4.4#649-r158309.