Bugzilla – Full Text Bug Listing
| Summary: | react (possibly) to compiler change in maintenance stream | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP Releng | Reporter: | David Williams <david_williams> | ||||||
| Component: | releng | Assignee: | David Williams <david_williams> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | David Williams <david_williams> | ||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | ccc, Olivier_Thomann, thatnitind | ||||||
| Version: | 3.10 | ||||||||
| Target Milestone: | 3.10.0 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 7 | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
David Williams
The comparator logs do not show up yet, and the rest of the JUnit tests are still running (I "peek" directly on build server) but the build that will include this change is http://build.eclipse.org/webtools/committers/wtp-R3.2.3-M/20110210015622/M-3.2.3-20110210015622/ (And, for future reference, that is a short lived URL ... will change in week or so). Once you have the name of the 7 different class files, I'd like to investigate. I would need to old .class files and the new ones. Thanks. Sorry, just saw the attachment. I would need to "old" .class files and the new ones. I'd like to find out what is different. I need the .class files for: org/eclipse/jst/jsp/core/internal/java/jspel/Token.class org/eclipse/wst/sse/ui/internal/StorageModelProvider.class org/eclipse/wst/xsl/internal/debug/ui/tabs/main/TransformsBlock.class org/eclipse/wst/jsdt/internal/codeassist/SelectionEngine.class org/eclipse/wst/xml/core/internal/contenttype/XMLHeadTokenizer.class org/eclipse/jst/j2ee/model/internal/validation/EJBJar20VRule.class org/eclipse/wst/jsdt/core/tests/compiler/regression/AbstractRegressionTest.class I can either get the .class files isolated or the jars that contain both versions of the .class files. Thanks. Thanks for the offer to check the byte codes. I'd probably be less error prone if I gave you links for "everything" then let you download/navigate to them in a way easiest for you. The first 6 are in "real" code, the last one in unit tests. For the real code, there is an archived repo at http://build.eclipse.org/webtools/committers/wtp-R3.2.3-M/20110210015622/M-3.2.3-20110210015622/wtp-repo-M-3.2.3-20110210015622.zip to compare to http://build.eclipse.org/webtools/committers/wtp-R3.2.3-M/20110209071934/M-3.2.3-20110209071934/wtp-repo-M-3.2.3-20110209071934.zip Similar for the unit test class: http://build.eclipse.org/webtools/committers/wtp-R3.2.3-M/20110210015622/M-3.2.3-20110210015622/wtp-tests-repo-M-3.2.3-20110210015622.zip to compare to http://build.eclipse.org/webtools/committers/wtp-R3.2.3-M/20110209071934/M-3.2.3-20110209071934/wtp-tests-repo-M-3.2.3-20110209071934.zip If easier for you to use a shell account or scp, the file locations are the same as above, but with '/shared' instead of 'http://build.eclipse.org'. Or, your third option, if easier to use a shell account to directly to the repo, instead of the final zip file segment, you would use '/repository/ and '/repositoryunittests/' Or, your fourth option ... tell me you'd want me to sort it all out :) Above, I list only the "previous build", where compiler was only change. The comparator tests name differences in 3 or 4 various reference repositories. We do use all those references in our comparator tests (they are previously declared weekly builds) but I think there must be some indeterminacy in the way the mirror task (and its associated comparator) pick out the "latest bundle with same qualifier". The same "old version" must be in the previous build, I reference above. I'd be happy to "hand pick" the jars instead of you ... but, will take me a few hours to get to. So, I'll start that task later in the day if you'd prefer. doh, in comment #4 I listed the "new compiled jars" in /repository and /repositoryunittests, but ... now that I'm waking up :) ... I realized those repos contain the _results_ of the mirror operation, which would take the "old" bundle with same qualifier ... the newly compiled bundles are indeed only on build machine (not in zip files) in the various directories listed in comparator log, such as .../buildrepository/jst-sdk .../buildrepository/wst-sdk .../buildrepository/wst.tests I don't have ssh access to the build machine. So unless you find a way to give me the required jars, I cannot investigate the differences. Created attachment 188697 [details]
zip containing jars to compare, in 'old' and 'new' subdirectories
Here's the 'old' and 'new' jars. The zip file is 17Meg, so I marked as "big file" so it will be removed from bugzilla, at some point. (And ... I will also email to you, Olivier, so maybe one of the methods will work).
I got it. I am investigating. org/eclipse/jst/jsp/core/internal/java/jspel/Token.class
is a difference about the way the following statement is compiled:
switch(ofKind)
{
default : return new Token(ofKind, image);
}
This is a consequence of the fix for bug 314830.
org/eclipse/wst/sse/ui/internal/StorageModelProvider.class
if (document == null) {
document = model.getStructuredDocument();
}
We now generate the null check. Before we skipped it as document can only be null at this location. So the change is perfect safe.
org/eclipse/wst/xsl/internal/debug/ui/tabs/main/TransformsBlock.class
Same code pattern than the first issue:
protected void setSortColumn(int column) {
switch (column) {
// case 1:
// sortByName();
// break;
// case 2:
// sortByType();
// break;
}
super.setSortColumn(column);
}
I would say the whole switch statement should be removed.
org/eclipse/wst/jsdt/internal/codeassist/SelectionEngine.class
Same thing again:
The code is:
switch (kind) {
default:
....
}
I don't see the point of having a switch statement.
org/eclipse/wst/xml/core/internal/contenttype/XMLHeadTokenizer.class
Same thing again.
org/eclipse/jst/j2ee/model/internal/validation/EJBJar20VRule.class
This looks more aggressive as we removed some null checks. The change for the catch Throwable looks suspicious.
So I'll take a second look at the EJBJar20VRule class.
(In reply to comment #9) > org/eclipse/jst/j2ee/model/internal/validation/EJBJar20VRule.class > This looks more aggressive as we removed some null checks. The change for the > catch Throwable looks suspicious. > So I'll take a second look at the EJBJar20VRule class. Everything is fine. I was confused with another null check being removed. This is the one for the logger check and not the bean inside the Throwable catch blocks. So I reviewed all changes in "real" code and none of them is wrong or a false positive. Some code could be changed to "improve" the code generation like unnecessary switch statements, but as it stands, there is no risk going with the .class files generated by the "new" compiler. Hope this help. (In reply to comment #10) > (In reply to comment #9) > > org/eclipse/jst/j2ee/model/internal/validation/EJBJar20VRule.class > > This looks more aggressive as we removed some null checks. The change for the > > catch Throwable looks suspicious. > > So I'll take a second look at the EJBJar20VRule class. > Everything is fine. I was confused with another null check being removed. This > is the one for the logger check and not the bean inside the Throwable catch > blocks. > So I reviewed all changes in "real" code and none of them is wrong or a false > positive. Some code could be changed to "improve" the code generation like > unnecessary switch statements, but as it stands, there is no risk going with > the .class files generated by the "new" compiler. > Hope this help. Olivier, I don't understand your statement: "This is the one for the logger check". Is there unnecessary code that we can clean up in EJBJar20VRule ? If so, what? I retagged org.eclipse.jst.j2ee.core with v201102101900 and updated the tag in the jst-j2ee-basic.map file. Nitin will have to update the tag for the org.eclipse.jst.web_core.feature, since that is in sourceediting (In reply to comment #11) > Olivier, I don't understand your statement: "This is the one for the logger > check". Is there unnecessary code that we can clean up in EJBJar20VRule ? If > so, what? Sorry my mistake. We do keep the null check on logger. I got confused by the diff between old and new .class files. The only thing that can be cleaned is the if (bean != null) { ... } line 268. bean cannot be null at this location. Sorry for the misunderstanding. (In reply to comment #12) > I retagged org.eclipse.jst.j2ee.core with v201102101900 and updated the tag in > the jst-j2ee-basic.map file. > > Nitin will have to update the tag for the org.eclipse.jst.web_core.feature, > since that is in sourceediting I've tagged org.eclipse.jst.web_core.feature as v201102101900 and the remainder as v201102102045. Thanks to all. I'm marking as fixed since work has been done. The rebuild has been started. ETA: midnight? |