On 11/25/2019 09:19 AM, Pavel Rappo wrote:
Hello,
Please review the following change for
https://bugs.openjdk.java.net/browse/JDK-8234746:
http://cr.openjdk.java.net/~prappo/8234746/webrev.00/
Before:
javax.rmi.ssl.client.enabledCipherSuites - Search tag in
javax.rmi.ssl.SslRMIClientSocketFactory
javax.rmi.ssl.client.enabledCipherSuites - Search tag in
javax.rmi.ssl.SslRMIClientSocketFactory
...
file.separator - Search tag in java.lang.System
After:
javax.rmi.ssl.client.enabledCipherSuites - Search tag in
javax.rmi.ssl.SslRMIClientSocketFactory
javax.rmi.ssl.client.enabledCipherSuites - Search tag in
javax.rmi.ssl.SslRMIClientSocketFactory.createSocket
...
file.separator - Search tag in java.lang.System.getProperties
Thanks,
-Pavel
You either need a test or mark the bug as noreg-something.
Uugh.
I agree that the fix will work, but I cringe at the methods in Utils
that make you fix it this way.
Readingline 441 of your fix, it is "obviously wrong" since the simple
name of an element is generally the last component of the qualified
name, but I see that "Utils.getFullyQualifiedName" will (and is
specified to) return the qualified name of the enclosing element in some
cases, which is not an intuitive definition based on the method's name.
I guess I wonder how often Utils.getFullyQualifiedName is invoked with a
field or method, and whether we should make those use sites use
Utils.getFullyQualifiedName(e.getEnclosingElement()) if they really want
the qualified name of the enclosing element, so that
Utils.getFullyQualifiedName() can throw ILA if called with an element
that does not have a fully qualified name.
Proactively applying that logic to your fix, it would be less jarring if
you were to change it to:
- 441 si.setHolder(utils.getFullyQualifiedName(e) + "." +
utils.getSimpleName(e)); + 441
si.setHolder(utils.getFullyQualifiedName(e.getEnclosingElement()) + "."
+ utils.getSimpleName(e));
That being said, I see your fix is the same as in line 435, which
suggests we might want to modify that line as well to better reflect the
notions of fully qualified name and simple name. Curiously, the code
for visitVariable "almost" gets it right, by getting the enclosing type
element (te) but then it ignores it.
--Jon
