Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 372694 - Adjust parser generator tools
Summary: Adjust parser generator tools
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.8   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 3.8 M7   Edit
Assignee: Ayushman Jain CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-27 23:37 EST by Srikanth Sankaran CLA
Modified: 2012-04-30 04:20 EDT (History)
2 users (show)

See Also:
satyam.kandula: review+

Attachments
proposed fix (27.81 KB, patch)
2012-04-12 01:54 EDT, Ayushman Jain CLA 		no flags Details | Diff
patch for ParserUpdater (973 bytes, patch)
2012-04-12 02:05 EDT, Ayushman Jain CLA 		no flags Details | Diff
proposed fix v2 (27.32 KB, patch)
2012-04-13 07:00 EDT, Ayushman Jain CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Srikanth Sankaran CLA 2012-02-27 23:37:36 EST 
This is a follow up from bug 353553.

The fix for bug 353553 directly changed a generated file.
This will bring back the bug anytime the parser related
files are regenerated. We need to adjust the relevant classes in 
org.eclipse.jdt.core.tools project so that the generated files
will continue to have the fix.
Comment 1 Ayushman Jain CLA 2012-02-28 01:23:01 EST 
The right way here would be to change the file extension to something like .props. It doesn't really need translation.
For this, apart from changing the extension we need to find an alternative to using Resource.getBundle(..) in org.eclipse.jdt.internal.compiler.parser.Parser.readReadableNameTable(String), so that a .properties file is not expected there anymore.
Comment 2 Ayushman Jain CLA 2012-04-12 01:54:52 EDT 
Created attachment 213878 [details]
proposed fix

This patch changes the extension of readableNames to .props. It also now reads it as an input stream and then generates the readable names table.
Comment 3 Ayushman Jain CLA 2012-04-12 02:05:58 EDT 
Created attachment 213879 [details]
patch for ParserUpdater

Updates the tools which generate the parser files.
Comment 4 Ayushman Jain CLA 2012-04-12 02:06:32 EDT 
Satyam, can you please review? Thanks!
Comment 5 Satyam Kandula CLA 2012-04-12 07:08:04 EDT 
Ayush, I think you should read the file as a properties file rather than a resource file. Sorry, I didn't realize when we talked about it :(. One major approach with this issue is the call to fileContents.indexOf() could go bad. It could just give the index of another string which starts with the same name :). Moreover, I could also see that the file contents is sorted badly for this particular case also. 
Please explore the properties file approach.
Comment 6 Satyam Kandula CLA 2012-04-12 07:08:51 EDT 
(In reply to comment #3)
> Created attachment 213879 [details]
> patch for ParserUpdater
> 
> Updates the tools which generate the parser files.
This change looks good.
Comment 7 Ayushman Jain CLA 2012-04-13 07:00:43 EDT 
Created attachment 213965 [details]
proposed fix v2

This patch uses java.util.Properties. Thanks Satyam for pointing me to it. All tests pass.
Comment 8 Satyam Kandula CLA 2012-04-16 05:24:07 EDT 
Changes look good. +1
Comment 9 Ayushman Jain CLA 2012-04-18 05:31:14 EDT 
Released to master via commit e8c9a8ad63c042540bdf2089eae8eda8857cd19d
Comment 10 Srikanth Sankaran CLA 2012-04-30 04:20:07 EDT 
Verified for 3.8 M7