Thread: [Clirr-devel] comments on recent commits
Status: Alpha
Brought to you by:
lkuehne
Menu
▾
▴
clirr-devel
|
From: Simon K. <si...@ec...> - 2004-06-18 08:07:25
|
Hi Lars,
Thought I'd gone a bit quiet on the clirr development? Nope, just was
working on a rather tricky bit of code :-).
It took me a while to figure out how to determine the real scope of a
nested class - see method ScopeSelector.getClassScope, it's a beauty!
I'm slowly working my way through chapter 13 of the java specification
(the binary compatibility bit); I presume you've been though that too.
That's something I would never have done if I hadn't started working on
clirr!
Hopefully these changes are OK by you. There isn't anything
controversial in there, I think, except possibly having the check method
return a boolean. As you can see from the ClassScopeCheck method, this
allows a check to indicate that there isn't any point applying further
changes. Maybe this feature could be used in the GenderCheck, to avoid
having to have this rather ugly check in MethodSetCheck?
if (compatBaseline.isInterface() ^ currentVersion.isInterface())
{
return true;
}
Catch you later,
Simon
|
|
From: <lak...@t-...> - 2004-06-20 17:06:16
|
Simon Kitching wrote:
>Hi Lars,
>
>Thought I'd gone a bit quiet on the clirr development? Nope, just was
>working on a rather tricky bit of code :-).
>
>It took me a while to figure out how to determine the real scope of a
>nested class - see method ScopeSelector.getClassScope, it's a beauty!
>
>
>
Yep, nice work! I once wrote a sth similar for checkstyle to ensure that
you don't have to Javadoc public methods of anonymous inner classes.
Funny that this information is not available directly in BCEL...
BTW, if we wrote our own BCEL adaptor I mentioned on IRC, this would be
one of the methods for a class, so we'd have a more OO API to work with.
No big deal however, and the work involved in such an abstraction would
not be justified...
>I'm slowly working my way through chapter 13 of the java specification
>(the binary compatibility bit); I presume you've been though that too.
>That's something I would never have done if I hadn't started working on
>clirr!
>
>
>
Maybe we should start an Open Source Java Profiler tool (competitor to
JProbe, Optimizeit, etc.) so we get to read "Inside the Java Virtual
Machine" as well ;-)
>Hopefully these changes are OK by you. There isn't anything
>controversial in there, I think, except possibly having the check method
>return a boolean. As you can see from the ClassScopeCheck method, this
>allows a check to indicate that there isn't any point applying further
>changes. Maybe this feature could be used in the GenderCheck, to avoid
>having to have this rather ugly check in MethodSetCheck?
>
> if (compatBaseline.isInterface() ^ currentVersion.isInterface())
> {
> return true;
> }
>
>
>
>
Tried to work on it for a few minutes. Changing the code is easy and I
think Clirr works correctly. But getting the unit tests to run is
trickier than I first thought: Because our unit test framework only
works with one check at a time, the GenderChange check does not abort
the MethodSetCheck for ClassBecomesInterface (like it would in the real
world), and MethodSetCheck reports a missing constructor.
This demonstrates that our test framework really needs to use your class
selector better, MethodSetCheckTest should not check that class. I'll
look into that this week.
Lars
|
|
From: Simon K. <si...@ec...> - 2004-06-21 00:36:01
|
On Mon, 2004-06-21 at 05:13, Lars K=FChne wrote: > BTW, if we wrote our own BCEL adaptor I mentioned on IRC, this would be= =20 > one of the methods for a class, so we'd have a more OO API to work with= .=20 > No big deal however, and the work involved in such an abstraction would= =20 > not be justified... The ASM library appears to handle this stuff automatically; see http://asm.objectweb.org/current/doc/javadoc/user/org/objectweb/asm/tree/= InnerClassNode.html I also think that BCEL's OO design is odd in places. AccessFlags? Should a JavaClass really have an "is-a" relationship with (ie inherit from) "AccessFlags"? A has-a relationship seems much more sensible. And I've also subscribed to the BCEL user email list, and can report it is absolutely *silent*. So I would be quite keen for clirr to move to ASM sooner or later. BCEL is working for now, so there's no urgency, but I think it would be a good idea in the long run. Building a "wrapper" that allows BCEL or ASM to be "plugged in" is an option, but it would be significant work and I don't really see any benefits over simply using ASM directly. And one last issue: java 1.4 includes the BCEL classes, because it includes Xalan; try "jar tf rt.jar | grep BCEL". So it is not possible to provide an updated BCEL distribution with clirr (classes in rt.jar override the user classpath, unless serious trickery is applied). Note that this is a problem *only* for java1.4; sun have fixed this in 1.5. >=20 > >I'm slowly working my way through chapter 13 of the java specification > >(the binary compatibility bit); I presume you've been though that too. > >That's something I would never have done if I hadn't started working o= n > >clirr! > > > > =20 > > >=20 > Maybe we should start an Open Source Java Profiler tool (competitor to=20 > JProbe, Optimizeit, etc.) so we get to read "Inside the Java Virtual=20 > Machine" as well ;-) Hey, why not?? I have noticed a need for a tool that determines whether code is invoking any methods not present in Java version XXX. A few Apache commons releases have accidentally gone out with code that doesn't work in java 1.3.=20 This should be pretty simple to implement:=20 (a)=20 get a list of all accessable classes, methods,fields in release X of java (b)=20 iterate over all the code in a set of jars, determining whether there are any references to classes/methods/fields that aren't present in the list from step (a). I don't know whether this is within the scope of the clirr project or not....it is sort of a "binary compatibility test", but this time testing whether program W is compatible with library X, rather than whether there are any differences between library X version 1 and version 2. Regards, Simon |
|
From: <lak...@t-...> - 2004-06-21 05:43:35
|
Simon Kitching wrote: >On Mon, 2004-06-21 at 05:13, Lars Kühne wrote: > > >>BTW, if we wrote our own BCEL adaptor I mentioned on IRC, this would be >>one of the methods for a class, so we'd have a more OO API to work with. >>No big deal however, and the work involved in such an abstraction would >>not be justified... >> >> > >The ASM library appears to handle this stuff automatically; see > >http://asm.objectweb.org/current/doc/javadoc/user/org/objectweb/asm/tree/InnerClassNode.html > >I also think that BCEL's OO design is odd in places. AccessFlags? Should >a JavaClass really have an "is-a" relationship with (ie inherit from) >"AccessFlags"? A has-a relationship seems much more sensible. > > > Yes, I have found that the BCEL design is ... errrmmhhh ... a bit strange in places. You mentioned one example, the classloader/repository handling is another one. I've also found that bcel accessors tend to hand out their private data structures, e.g. JavaClass.getFields() >And I've also subscribed to the BCEL user email list, and can report it >is absolutely *silent*. > >So I would be quite keen for clirr to move to ASM sooner or later. BCEL >is working for now, so there's no urgency, but I think it would be a >good idea in the long run. Building a "wrapper" that allows BCEL or ASM >to be "plugged in" is an option, but it would be significant work and I >don't really see any benefits over simply using ASM directly. > > > * It would minimize our external dependencies, check implementations would only reference our own stuff. * Some IDEs provide their own API for accessing the class info we need, at least for the current version of the code that is checked. For example I know that IDEA provides a ProgramStructure interface (PSI) to access the source code. PSI isn't based on class files, but it provides all info required for clirr, like inheritance hierarchy, method signature, etc. Typically the IDE caches that information, minimizing disk activity. If clirr is used interactively from an IDE plugin, using PSI instead of accessing class files could make a real difference in terms of performance. >And one last issue: java 1.4 includes the BCEL classes, because it >includes Xalan; try "jar tf rt.jar | grep BCEL". > No results for me (1.4.2-b28), it seems Sun has fixed it within 1.4.2 as well (?). Still it's worrying. I've checked the bcel-devel archives, and that's not too encouraging either... > So it is not possible >to provide an updated BCEL distribution with clirr (classes in rt.jar >override the user classpath, unless serious trickery is applied). Note >that this is a problem *only* for java1.4; sun have fixed this in 1.5. > > > >>>I'm slowly working my way through chapter 13 of the java specification >>>(the binary compatibility bit); I presume you've been though that too. >>>That's something I would never have done if I hadn't started working on >>>clirr! >>> >>> >>> >>> >>> >>Maybe we should start an Open Source Java Profiler tool (competitor to >>JProbe, Optimizeit, etc.) so we get to read "Inside the Java Virtual >>Machine" as well ;-) >> >> > >Hey, why not?? > >I have noticed a need for a tool that determines whether code is >invoking any methods not present in Java version XXX. A few Apache >commons releases have accidentally gone out with code that doesn't work >in java 1.3. > >This should be pretty simple to implement: >(a) >get a list of all accessable classes, methods,fields in release X of >java >(b) >iterate over all the code in a set of jars, determining whether >there are any references to classes/methods/fields that aren't present >in the list from step (a). > >I don't know whether this is within the scope of the clirr project or >not....it is sort of a "binary compatibility test", but this time >testing whether program W is compatible with library X, rather than >whether there are any differences between library X version 1 and >version 2. > > That would probably be in scope for clirr, but I'm not sure it's feasible: Things like reflection and casting may turn out to be significant road blocks to detect these problems reliably. But maybe we could start by checking that the class file format version of each class has stayed the same? On a related note, I have recently found out that messing around with a method's declared exceptions will not result in binary compatibility problems (surprising to me). Should we extend clirr's scope to check for source compatibility as well? Cheers, Lars |
|
From: Simon K. <si...@ec...> - 2004-06-21 06:11:24
|
On Mon, 2004-06-21 at 17:50, Lars K=FChne wrote: > Simon Kitching wrote: >=20 > >On Mon, 2004-06-21 at 05:13, Lars K=FChne wrote: > > =20 > > > > > >So I would be quite keen for clirr to move to ASM sooner or later. BCE= L > >is working for now, so there's no urgency, but I think it would be a > >good idea in the long run. Building a "wrapper" that allows BCEL or AS= M > >to be "plugged in" is an option, but it would be significant work and = I > >don't really see any benefits over simply using ASM directly. > > > > =20 > > >=20 > * It would minimize our external dependencies, check implementation= s > would only reference our own stuff. > * Some IDEs provide their own API for accessing the class info we > need, at least for the current version of the code that is > checked. For example I know that IDEA provides a ProgramStructure > interface (PSI) to access the source code. PSI isn't based on > class files, but it provides all info required for clirr, like > inheritance hierarchy, method signature, etc. Typically the IDE > caches that information, minimizing disk activity. If clirr is > used interactively from an IDE plugin, using PSI instead of > accessing class files could make a real difference in terms of > performance. The PSI stuff is a good point. If IDEA have gone to the effort of designing a nice API for accessing class structure, then can we use *that* as our API too? ie write adaptors from PSI->BCEL, PSI->ASM, PSI->PSI? It seems silly to invent our own API...and presumably APIs are in the public domain (unlike implementations). >=20 >=20 > >And one last issue: java 1.4 includes the BCEL classes, because it > >includes Xalan; try "jar tf rt.jar | grep BCEL". > > >=20 > No results for me (1.4.2-b28), it seems Sun has fixed it within 1.4.2 a= s=20 > well (?). Still it's worrying. I've checked the bcel-devel archives, an= d=20 > that's not too encouraging either... Sorry,my mistake. I'm sure I saw it there, but can't find it now. I must have had sun-stroke or something :-) Bcel is in java 1.5, but isn't an issue because with 1.5 sun now make sure any non-sun software they distribute is given a sun package prefix, eg 1.5 contains: com.sun.org.apache.bcel... com.sun.org.apache.xerces.... This is a reasonably nice solution; they don't hide the real authors of the code, but do allow downloads direct from apache or others to be used without conflicting with the classes in the java std libs. > >>> > >>Maybe we should start an Open Source Java Profiler tool (competitor t= o=20 > >>JProbe, Optimizeit, etc.) so we get to read "Inside the Java Virtual=20 > >>Machine" as well ;-) > >> =20 > >> > > > >Hey, why not?? > > > >I have noticed a need for a tool that determines whether code is > >invoking any methods not present in Java version XXX. A few Apache > >commons releases have accidentally gone out with code that doesn't wor= k > >in java 1.3.=20 > > >=20 > That would probably be in scope for clirr, but I'm not sure it's=20 > feasible: Things like reflection and casting may turn out to be=20 > significant road blocks to detect these problems reliably. But maybe we= =20 > could start by checking that the class file format version of each clas= s=20 > has stayed the same? Yes, there could be gotchas.=20 >=20 > On a related note, I have recently found out that messing around with a= =20 > method's declared exceptions will not result in binary compatibility=20 > problems (surprising to me). Should we extend clirr's scope to check fo= r=20 > source compatibility as well? I'm +1 on reporting compile-time incompatibilities too.=20 I agree that changing the declared exceptions should be reported; a WARNING seems most appropriate to me. What do you think? Regards, Simon |
|
From: <lak...@t-...> - 2004-06-22 18:59:31
|
Simon Kitching wrote: >On Mon, 2004-06-21 at 17:50, Lars Kühne wrote: > > >>Simon Kitching wrote: >> >> >> >>>On Mon, 2004-06-21 at 05:13, Lars Kühne wrote: >>> >>> >>> >>>So I would be quite keen for clirr to move to ASM sooner or later. BCEL >>>is working for now, so there's no urgency, but I think it would be a >>>good idea in the long run. Building a "wrapper" that allows BCEL or ASM >>>to be "plugged in" is an option, but it would be significant work and I >>>don't really see any benefits over simply using ASM directly. >>> >>> >>> >>> >>> >> * It would minimize our external dependencies, check implementations >> would only reference our own stuff. >> * Some IDEs provide their own API for accessing the class info we >> need, at least for the current version of the code that is >> checked. For example I know that IDEA provides a ProgramStructure >> interface (PSI) to access the source code. PSI isn't based on >> class files, but it provides all info required for clirr, like >> inheritance hierarchy, method signature, etc. Typically the IDE >> caches that information, minimizing disk activity. If clirr is >> used interactively from an IDE plugin, using PSI instead of >> accessing class files could make a real difference in terms of >> performance. >> >> > >The PSI stuff is a good point. > >If IDEA have gone to the effort of designing a nice API for accessing >class structure, then can we use *that* as our API too? ie write >adaptors from PSI->BCEL, PSI->ASM, PSI->PSI? It seems silly to invent >our own API...and presumably APIs are in the public domain (unlike >implementations). > > The IDEA license does not allow redistribution of the psi classes (at least not in IDEA 3, which is what I have), because they are bundled with the idea implementation classes (one jar file). The Eclipse JDT probably has something similar, but I'm not sure we really want to develop against such an API: It's too detailled. I don't know much about JDT, but the IDEA psi API gives you access to the actual source code of a method, the source file, etc. Clirr does not need all that, and it would be really hard to write an adapter that maps BCEL to PSI because of all the stuff you have to implement. In fact I think the API we need would not be very large, it would basically consist of the following interfaces: JavaClass + Method[] getMethods() + String getName() + JavaClass[] getSuperClasses() + JavaClass[] getImplementedInterfaces() Method + String getName() + String getSignature() // would return sth. like "public void blah()" + JavaClass[] getParameterTypes() + JavaClass[] getDeclaredExceptions() Field + String getName() + JavaClass getType() + Object getConstantValue() Some parts are missing (like access scope), but I think you get the idea: It's not very complicated (quite similar to the reflection API), it's trivial to implement with BCEL, and I guess it would be trivial to implement with ASM, IDEA psi, Eclipse JDT or whatever... But like you said, I don't think such a refactoring is important near term - let's reconsider it when an IDE plugin appears on the horizon... Lars |
|
From: <lak...@t-...> - 2004-06-22 18:25:52
|
Simon Kitching wrote: >>On a related note, I have recently found out that messing around with a >>method's declared exceptions will not result in binary compatibility >>problems (surprising to me). Should we extend clirr's scope to check for >>source compatibility as well? >> >> > >I'm +1 on reporting compile-time incompatibilities too. > >I agree that changing the declared exceptions should be reported; a >WARNING seems most appropriate to me. What do you think? > > > My idea currently is to introduce two severity levels in ApiDeffierence, one for binary compatibility and one for source compatibility. For most differences we detect today they would have the same value. For added checked exceptions binary compatibility severity would be WARNING (because strange things will happen if the exception is actually thrown and the client does not handle it), for removed checked exceptions it would be INFO. In both cases source compatibility would be ERROR. This would have some effect on the Ant task, e.g. it should be possible to break the build only on binary errors. I'm currently working on that. You like the idea? Lars |
|
From: Simon K. <si...@ec...> - 2004-06-22 22:42:07
|
On Wed, 2004-06-23 at 06:33, Lars K=FChne wrote: > Simon Kitching wrote: >=20 > >>On a related note, I have recently found out that messing around with= a=20 > >>method's declared exceptions will not result in binary compatibility=20 > >>problems (surprising to me). Should we extend clirr's scope to check = for=20 > >>source compatibility as well? > >> =20 > >> > > > >I'm +1 on reporting compile-time incompatibilities too.=20 > > > >I agree that changing the declared exceptions should be reported; a > >WARNING seems most appropriate to me. What do you think? > > > > =20 > > >=20 > My idea currently is to introduce two severity levels in ApiDeffierence= ,=20 > one for binary compatibility and one for source compatibility. For most= =20 > differences we detect today they would have the same value. >=20 > For added checked exceptions binary compatibility severity would be=20 > WARNING (because strange things will happen if the exception is actuall= y=20 > thrown and the client does not handle it), for removed checked=20 > exceptions it would be INFO. In both cases source compatibility would b= e=20 > ERROR. >=20 > This would have some effect on the Ant task, e.g. it should be possible= =20 > to break the build only on binary errors. I'm currently working on that. >=20 > You like the idea? That sounds good to me. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X