+ Does something like this sound desirable?
+ Is it worth creating a wiki page to document design ideas?
Please correct me if I've got any of this wrong, but it is my impression that using (set! *warn-on-reflection* true) at the beginning of a file, or the Leiningen :warn-on-reflection key, is somewhat annoying, in the following way.
While it helps you find reflection warnings, typically you fix the ones that you want to fix, and then there are often warnings left over that you either don't know how to fix, or you consider that they are necessary uses of reflection, or the code isn't performance critical so you leave it as is.
However, you don't want to see those handful of warnings every time you compile or test, because then it makes you accustomed to seeing a handful of warnings, and you would rather that warnings indicate something important to investigate, not "noise" to ignore. So you remove the (set! *warn-on-reflection* true) or :warn-on-reflection before checking in code. It requires a conscious effort to turn them back on later if you want to look for new reflection warnings in your code, and then you will also typically revisit the ones you saw in the past, too.
I've got my own local Clojure tree where I've tried out the following experiment (I'm happy to publish the code somewhere, e.g. a github fork, if there is interest):
Change the default value of *warn-on-reflection* to true in the compiler.
Create a new expression (known-reflection ...) that behaves identically to (do ...), except that it sets *warn-on-reflection* to false within its scope.
The idea is that reflection warnings are always on, everywhere, except where you explicitly turn them off, either through setting *warn-on-reflection* to false, wrapping an expression in (known-reflection), or using the Java system property before invoking the JVM. The preferred method for production code is to eliminate the reflection warning, or if you want to keep the warning code unchanged, wrap it in (known-reflection ...). If it is test code, a blanket use of (set! *warn-on-reflection* false) near the beginning of the file seems appropriate to me. That can be commented out temporarily if you want to try to optimize your tests.
Run "ant" to build and test.
Insert type hints, uses of known-reflection around individual expressions, or in a few cases (primarily in test files) add (set! *warn-on-reflection* false) before code that has lots of reflection warnings.
From some quick grep'ing through my diffs, I estimate that I've added:
37 type hints (my grep regexp here might be a little off)
27 uses of known-reflection
4 bracketed pairs of (set! *warn-on-reflection* false) followed shortly later by (set! *warn-on-reflection true) around a few functions grouped together that had reflection warnings that seemed either necessary, or would require too many sprinklings of (known-reflection ...) to do it more surgically.
9 uses of (set! *warn-on-reflection* false) that are in effect from that point until the end of the file, all in test files where performance is less of a concern, and where I did not want to unintentionally modify the intent of the test. That might be the best policy for test code in general, especially in Clojure core tests, because some of them are specifically testing for whether type hints make a difference.
I can go on to see what the effect would be on particular contrib libraries if there is interest, but given their relative sizes to the Clojure code, I suspect the number of warnings, and changes required to fix them, would be significantly smaller.
Andy
David
> --
> You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
> To post to this group, send email to cloju...@googlegroups.com.
> To unsubscribe from this group, send email to clojure-dev...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/clojure-dev?hl=en.
>
An additional advantage of the proposed (known-reflection ...) form is that you can surgically disable warnings permanently, for single expressions. That way, you can focus on new warnings without having to sift through old ones, too.
Andy
Sent from my iPhone
> An additional advantage of the proposed (known-reflection ...) form is
> that you can surgically disable warnings permanently, for single
> expressions. That way, you can focus on new warnings without having
> to sift through old ones, too.
known-reflection sounds useful in it's own right, independent of the
*warn-on-reflection* default, but I also would argue that tool support
is a better way of making sure reflection warnings are handled.
For interactive code development, I think the reflections warnings would
quickly become intrusive.
Hugo
>> An additional advantage of the proposed (known-reflection ...) form is
>> that you can surgically disable warnings permanently, for single
>> expressions. That way, you can focus on new warnings without having
>> to sift through old ones, too.
>
> known-reflection sounds useful in it's own right, independent of the
> *warn-on-reflection* default, but I also would argue that tool support
> is a better way of making sure reflection warnings are handled.
Isn't known-reflection just (binding [*warn-on-reflection* false] ~@body) ?
> For interactive code development, I think the reflections warnings would
> quickly become intrusive.
As someone who has spent three years in Clojure and only first turned on
reflection warnings a few weeks ago, I heartily agree.
-Phil
> Hugo Duncan <dunca...@gmail.com> writes:
>
>> For interactive code development, I think the reflections warnings would
>> quickly become intrusive.
>
> As someone who has spent three years in Clojure and only first turned on
> reflection warnings a few weeks ago, I heartily agree.
I agree that changing Clojure to have reflection warnings on by default is a bad idea. I had not considered how annoying it would be to enable them for interactive use, and was not aware how easy it is with Leiningen and ant to turn them on or off for a whole project.
For Clojure, uncommenting the following line in build.xml enables reflection warnings when compiling Clojure, but not the tests.
<!-- <sysproperty key="clojure.compile.warn-on-reflection" value="true"/> -->
With 30 type hints (eliminating reflection), 20 additions of known-reflection, and one blanket use of:
(set! *warn-on-reflection* false)
near the beginning of pretty_writer.clj, there are no reflection warnings when compiling Clojure. I believe there is a good way to eliminate the uses of reflection in pretty_writer.clj, not just silence the warnings, but I'm still thinking about a satisfying way to do it.
If you want to see any of these changes, I've created a patch [1] that contains 3 separate commits, described below.
[1] http://homepage.mac.com/jafingerhut/files/clojure/reflect-warn-patches-v1.txt
Andy
Commit 1: Add type hints to avoid reflection
Adds 30 type hints. All of them eliminate some use of reflection in the code. They should be double-checked by someone else, in case I've added an incorrect one. All Clojure tests pass.
Commit 2: Add special form known-reflection to Clojure compiler
It is identical to do, except that *warn-on-reflection* is false within its scope.
Commit 3: Add uses of known-reflection to silence all current reflection warnings
The reflection is still there, of course, just not warned about, even when you enable *warn-on-reflection* at a file level.
File pretty_writer.clj currently has a blanket (set! *warn-on-reflection* false). There are ways to eliminate the reflection, not merely silence it, but it would be better if it could be made syntactically cleaner.
P.S.: One motivation for a relatively long name like known-reflection is making it easy to search source code for uses of it. This is handy if you want to review the code and find uses of reflection that you want to try to eliminate.
Another idea would be to have another Java property like clojure.compile.warn-on-reflection that enables reflection warnings even inside of known-reflection expressions. I haven't implemented that.