Skip to:

  1. Skip to Jira Navigation
  2. Skip to Side Navigation
  3. Skip to Main Content

type-reflect with AsmReflector throws exceptions for classes with annotations

Description

REPL session reproducing the problem:

% java -cp clojure.jar clojure.main Clojure 1.6.0-master-SNAPSHOT user=> (use 'clojure.reflect) nil user=> (import '[clojure.reflect AsmReflector]) clojure.reflect.AsmReflector user=> (def cl (.getContextClassLoader (Thread/currentThread))) #'user/cl user=> (def asm-reflector (AsmReflector. cl)) #'user/asm-reflector user=> (type-reflect 'java.lang.SuppressWarnings :reflector asm-reflector) AbstractMethodError clojure.reflect.AsmReflector$reify__9181.visitAnnotation(Ljava/lang/String;Z)Lclojure/asm/AnnotationVisitor; clojure.asm.ClassReader.accept (ClassReader.java:593)

Issue discovered when trying out a build of Clojure source code with JDK8 early access version b103. In Clojure's set of tests, one of the classes tested with type-reflect fails with JDK8 because annotations were added to that class in JDK8, but it did not have annotations in earlier JDK versions. The same issue exists with JDK6 and JDK7 for any class with annotations, though – it is not unique to JDK8.

Analysis:

Definition of AsmReflector type in src/clj/clojure/reflect/java.clj has a reify for a ClassVisitor with no definition for a visitAnnotation method. This method is called for classes with annotations.

Patch: clj-1246-fix-type-reflect-exception-patch-v1.diff
Screened by: Alex Miller

Environment

None

Attachments

2

Activity

Show:

Alex Miller October 24, 2013 at 8:14 PM

Ah, thank you. Diff blindness.

Andy Fingerhut October 24, 2013 at 3:05 PM

The line where java.lang.SuppressWarnings is added is not an import, but naming another class in a sequence of classes being iterated over via doseq in a test.

Alex Miller October 24, 2013 at 4:56 AM

What new test? I just see the SuppressWarnings import?

Andy Fingerhut October 23, 2013 at 3:52 AM

I added that because that new test fails on JDK6 and 7 without the patch. Without the additional test, the bug is not exposed by any existing tests unless you run on JDK8.

Alex Miller October 23, 2013 at 3:25 AM

Is there any reason for the addition of SuppressWarnings in the test file?

Completed

Details

Assignee

Reporter

Labels

Approval

Ok

Patch

Code and Test

Priority

Affects versions

Fix versions

Created August 22, 2013 at 1:55 AM
Updated October 25, 2013 at 5:01 PM
Resolved October 25, 2013 at 5:01 PM