tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kkoli...@apache.org
Subject svn commit: r909860 - /tomcat/tc6.0.x/trunk/STATUS.txt
Date Sat, 13 Feb 2010 17:55:11 GMT
Author: kkolinko
Date: Sat Feb 13 17:55:11 2010
New Revision: 909860

URL: http://svn.apache.org/viewvc?rev=909860&view=rev
Log:
split EL fixes vote into individual patches, for clarity
veto the Enum EL patch backport

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=909860&r1=909859&r2=909860&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Sat Feb 13 17:55:11 2010
@@ -107,59 +107,64 @@
 
 * Fix various EL TCK failures
   http://svn.apache.org/viewvc?view=rev&rev=899653 (signatures)
+   +1: markt, kkolinko
+   -1:
+
   http://svn.apache.org/viewvc?view=rev&rev=899769 (CCE expected)
   http://svn.apache.org/viewvc?view=rev&rev=899770 (CCE expected)
+   +1: markt, kkolinko
+   -1:
+     kkolinko: Maybe better name for that message, because it says about
+     arrays, yet name is rather generic
+
   http://svn.apache.org/viewvc?view=rev&rev=899783 (ELException expected)
+   +1: markt, kkolinko
+   -1:
+
   http://svn.apache.org/viewvc?view=rev&rev=899788 (PNFE expected)
+   +1: markt, kkolinko
+   -1:
+     kkolinko: I think o.a.jasper.el.ELResolverImpl#getType(ELContext,Object,Object)
+     should likewise throw a PropertyNotFoundException instead of returning null.
+     I have no test, though.
+
   http://svn.apache.org/viewvc?view=rev&rev=899792 (ELException rather than IAE)
+   +1: markt, kkolinko
+   -1:
+
   http://svn.apache.org/viewvc?view=rev&rev=899916 (ELException rather than IAE)
+   +1: markt, kkolinko
+   -1:
+
   http://svn.apache.org/viewvc?view=rev&rev=899918 (Enum coercion test cases)
   http://svn.apache.org/viewvc?view=rev&rev=899919 (Enum coercion bug) 
+   +1: markt
+   -1: kkolinko:
+     As far as I am reading, there is no provision in the EL
+     spec, that enum -> enum conversion can be performed by using its
+     string value.
+
+     The current TC 6.0 behaviour here will be a ClassCastException, trying
+     to assign the value, and that should be an ELException instead.
+
+     In EL 2.1 and 2.2 specifications chapter 1.18.6 'Coerce A to an Enum Type T' says:
+       "If A is a String call Enum.valueOf(T.getClass(), A) and return the result."
+     Thus, our
+      result = Enum.valueOf(type, obj.toString());
+     should only be applicable if obj is a String.
+
+     In ELSupport#coerceToType(Object, Class), that implements 1.18.7, we
+     already throw an ELException if A is not a String.
+
   http://svn.apache.org/viewvc?view=rev&rev=899935 (ELException expected)
-  http://svn.apache.org/viewvc?view=rev&rev=899949 (ignore whitespace on comp)
-  +1: markt
-  +1: kkolinko:
-     899653: OK.  We do not have @Deprecated annotations in those classes,
-                  so the patch is about adding @SuppressWarnings("dep-ann")
-     899769: With 899770 that backports the message string used here. 
-     899770: OK
-         (Maybe better name for that message, because it says about arrays,
-         yet name is rather generic).
-     899783: OK
-     899788: OK
-         (Likewise, o.a.jasper.el.ELResolverImpl#getType(ELContext,Object,Object)
-         should probably throw a PropertyNotFoundException, instead of returning null.
-         I have no proof, though.)
-     899792: OK
-     899916: OK
-
-     899918, 899919: OK, but there is probably an omission in the EL spec:
-         I do not see why we do conversion Enum->Enum via toString() call.
-
-         The EL spec chapter 1.18.6 'Coerce A to an Enum Type T' says
-            "If A is a String call Enum.valueOf(T.getClass(), A) and return the result."
-         It does not say what to do if A is not a String. (There is no
-         explicit "Otherwise, error" statement below).
-
-         In 1.18.7 (aka ELSupport#coerceToType(Object, Class)) we throw
-         an error if A is not a String. Even if T has a PropertyEditor,
-         we do not do  editor.setAsText(obj.toString()),  as the spec does
-         not say to do so, but throw an exception.
-
-         (In 1.18.7 the spec says "Otherwise, apply T's PropertyEditor",
-         but PropertyEditor can be applied only is A is a String. Am I right?)
-
-         Without 899919 patch we will throw a ClassCaseException when object type
-         is a different type of enum, but other values are still converted
-         via toString() call.  The patch makes that behaviour consistent, even
-         if I do not understand why it is allowed.
-
-     899935: OK
-     899949: OK,
-         but why ValueExpressionImpl.equals() is implemented as comparing
-         the hash codes? What will happen with false positives?
+  +1: markt, kkolinko
+  -1:
 
-  -1: 
+  http://svn.apache.org/viewvc?view=rev&rev=899949 (ignore whitespace on comp)
+  +1: markt, kkolinko
+  -1:
+     kkolinko: Why ValueExpressionImpl.equals() is implemented as comparing
+     the hash codes? What will happen with false positives?
 
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48627
   Regression in re-working of EL parsing
@@ -176,7 +181,7 @@
   Provide option to stop server if there is an error during init
   Port of Filip's patch from trunk
   http://svn.apache.org/viewvc?view=revision&revision=752323
-  +1: markt
+  +1: markt, kkolinko
   -1: 
 
 * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48645



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message