[Bug 53644] New: vbscript can not compile classes with lists of private / public / dim declarations
https://bugs.winehq.org/show_bug.cgi?id=53644 Bug ID: 53644 Summary: vbscript can not compile classes with lists of private / public / dim declarations Product: Wine Version: 7.16 Hardware: x86-64 OS: Mac OS X Status: UNCONFIRMED Severity: normal Priority: P2 Component: vbscript Assignee: wine-bugs(a)winehq.org Reporter: jsm174(a)gmail.com I am working on porting Visual Pinball to MacOS. I am using the vbscript engine from Wine and just found out that it is having issues compiling classes that have lists of private / public declarations. For example: Class FlipperPolarity Public DebugOn, Enabled private Flipper, FlipperStart,FlipperEnd, FlipperEndY, LR, PartialFlipCoef Private Balls(20), balldata(20) dim PolarityIn, PolarityOut dim VelocityIn, VelocityOut dim YcoefIn, YcoefOut . . . End Class To get this to compile, I had to split everything to a separate line. I also had to switch the arrays to dim Class FlipperPolarity Public DebugOn Public Enabled private Flipper private FlipperStart private FlipperEnd private FlipperEndY private LR private PartialFlipCoef dim Balls(20) dim balldata(20) dim PolarityIn dim PolarityOut dim VelocityIn dim VelocityOut dim YcoefIn dim YcoefOut . . . End Class I think an update is required in parser.y, but I'm not sure yet: ClassBody : /* empty */ { $$ = new_class_decl(ctx); } | FunctionDecl { $$ = add_class_function(ctx, new_class_decl(ctx), $1); CHECK_ERROR; } | FunctionDecl StSep ClassBody { $$ = add_class_function(ctx, $3, $1); CHECK_ERROR; } /* FIXME: We should use DimDecl here to support arrays, but that conflicts with PropertyDecl. */ | Storage tIdentifier { dim_decl_t *dim_decl = new_dim_decl(ctx, $2, FALSE, NULL); CHECK_ERROR; $$ = add_dim_prop(ctx, new_class_decl(ctx), dim_decl, $1); CHECK_ERROR; } | Storage tIdentifier StSep ClassBody { dim_decl_t *dim_decl = new_dim_decl(ctx, $2, FALSE, NULL); CHECK_ERROR; $$ = add_dim_prop(ctx, $4, dim_decl, $1); CHECK_ERROR; } | tDIM DimDecl { $$ = add_dim_prop(ctx, new_class_decl(ctx), $2, 0); CHECK_ERROR; } | tDIM DimDecl StSep ClassBody { $$ = add_dim_prop(ctx, $4, $2, 0); CHECK_ERROR; } | PropertyDecl { $$ = add_class_function(ctx, new_class_decl(ctx), $1); CHECK_ERROR; } | PropertyDecl StSep ClassBody { $$ = add_class_function(ctx, $3, $1); CHECK_ERROR; } -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 Jason Millard <jsm174(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jsm174(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 Robert Wilhelm <sloper42(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Status|UNCONFIRMED |NEW CC| |sloper42(a)yahoo.com --- Comment #1 from Robert Wilhelm <sloper42(a)yahoo.com> --- Confirming. We may have to use something like | Storage DimDeclList ... | tDim DimDeclList There is already this FIXME comment, therefore more changes to the parser might be necessary... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #2 from Robert Wilhelm <sloper42(a)yahoo.com> --- Created attachment 73051 --> https://bugs.winehq.org/attachment.cgi?id=73051 parse declarations lists -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #3 from Jason Millard <jsm174(a)gmail.com> --- Thanks for looking into this! I tried that patch, and while it made it through the declarations, it then failed on a "Public Property let Object(aInput).." report_script_error shows the p pointer at "let ObjecT" Note, the same code did compile, when I manually separated all the declarations. So maybe that was what the FIXME was about? Class FlipperPolarity Public DebugOn, Enabled Private FlipAt 'Timer variable (IE 'flip at 723,530ms...) Public TimeDelay 'delay before trigger turns off and polarity is disabled TODO set time! private Flipper, FlipperStart,FlipperEnd, FlipperEndY, LR, PartialFlipCoef Private Balls(20), balldata(20) dim PolarityIn, PolarityOut dim VelocityIn, VelocityOut dim YcoefIn, YcoefOut Public Sub Class_Initialize redim PolarityIn(0) : redim PolarityOut(0) : redim VelocityIn(0) : redim VelocityOut(0) : redim YcoefIn(0) : redim YcoefOut(0) Enabled = True : TimeDelay = 50 : LR = 1: dim x : for x = 0 to uBound(balls) : balls(x) = Empty : set Balldata(x) = new SpoofBall : next End Sub Public Property let Object(aInput) : Set Flipper = aInput : StartPoint = Flipper.x : End Property . . . End Class -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #4 from Robert Wilhelm <sloper42(a)yahoo.com> --- (In reply to Jason Millard from comment #3)
So maybe that was what the FIXME was about?
Thanks for testing my patch. The patch added two shift/reduce conflicts. vbscript allows some keywords like "property" to be used as an identifier. E.g. it is legal to write "Dim property". If I disallow tPROPERTY as identifier like below , I can parse your sample. This is of course not an acceptable solution. @@ -498,7 +495,7 @@ Identifier | tDEFAULT { ctx->last_token = tIdentifier; $$ = $1; } | tERROR { ctx->last_token = tIdentifier; $$ = $1; } | tEXPLICIT { ctx->last_token = tIdentifier; $$ = $1; } - | tPROPERTY { ctx->last_token = tIdentifier; $$ = $1; } + /* | tPROPERTY { ctx->last_token = tIdentifier; $$ = $1; } */ | tSTEP { ctx->last_token = tIdentifier; $$ = $1; } -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 Robert Wilhelm <sloper42(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73051|0 |1 is obsolete| | --- Comment #5 from Robert Wilhelm <sloper42(a)yahoo.com> --- Created attachment 73059 --> https://bugs.winehq.org/attachment.cgi?id=73059 parse declarations lists Next try. Do not allow tProperty for private or public members. Compiles the new test case and existing tests. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 Robert Wilhelm <sloper42(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #73059|0 |1 is obsolete| | --- Comment #6 from Robert Wilhelm <sloper42(a)yahoo.com> --- Created attachment 73069 --> https://bugs.winehq.org/attachment.cgi?id=73069 parse declarations lists Fix for last patch. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #7 from Jason Millard <jsm174(a)gmail.com> --- Finally had a chance to test the latest patch and it worked! The project I'm working has scripts of varying quality. I've found a few more parsing issues. Should I create a new bug, or post them here? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #8 from Robert Wilhelm <sloper42(a)yahoo.com> --- (In reply to Jason Millard from comment #7)
Finally had a chance to test the latest patch and it worked!
The project I'm working has scripts of varying quality. I've found a few more parsing issues. Should I create a new bug, or post them here?
Thanks for testing. Please create a new bug for each parsing issue. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #9 from Jason Millard <jsm174(a)gmail.com> --- Just wanted to follow up on this. Since I'm new to Wine's bug process, will these fixes eventually make it in main? Related to this, I've made some really good progress on getting the vbs engine to talk back to my vpinball port. There are a few internal functions that seem to not be implemented like ExecuteGlobal and Execute. (vpinball pulls in other scripts using these) I could try to help get these going. Do you know of any technical reasons why these are not yet implemented? -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #10 from Robert Wilhelm <sloper42(a)yahoo.com> --- (In reply to Jason Millard from comment #9) Patches are not picked up from Bugzilla. I need to add some test cases and open a merge requests in gitlab. Will try to submit the patches in the next weeks. ExecuteGlobal, Execute and Eval are missing because nobody implemented them so far. Feel free to work on them. I think this is not an easy task, it may require some refactoring of current code. For Windows version of VP also AddTypelib is needed. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #11 from Jason Millard <jsm174(a)gmail.com> --- Okay, thank you for the information! As for AddTypelib, I've worked around that by writing a utility that parses the MIDL file and generates GetIDsOfNames and Invoke methods. (GetIDsOfNames isn't very efficient right now, but I will optimize later) https://github.com/jsm174/vpvr/blob/044701951816190fadf3a5023f4fd1b49fabefe4... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #12 from Jason Millard <jsm174(a)gmail.com> --- I know this issue is pretty old, but is there a chance Robert's patch could eventually make it's way into Wine? We've been using it for well over a year in Visual Pinball standalone, and I can't imagine our project getting as far as it has if it wasn't for this patch. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #13 from Robert Wilhelm <sloper42(a)yahoo.com> --- Thanks for the reminder. I will try to prepare the MR soon. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #14 from Jason Millard <jsm174(a)gmail.com> --- (In reply to Robert Wilhelm from comment #13)
Thanks for the reminder. I will try to prepare the MR soon.
fantastic thank you so much! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 francisdb <francisdb(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |francisdb(a)gmail.com --- Comment #15 from francisdb <francisdb(a)gmail.com> --- Sorry about bringing this one up again but any chance this could be merged? I'm willing to crate a MR myself but don't want to go running with your credits. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #16 from francisdb <francisdb(a)gmail.com> --- https://gitlab.winehq.org/wine/wine/-/merge_requests/7068 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #17 from Robert Wilhelm <sloper42(a)yahoo.com> --- Hi francis, thanks for creating the tests and the MR! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #18 from francisdb <francisdb(a)gmail.com> --- This can be closed as the linked PR has been merged. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 Robert Wilhelm <sloper42(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |FIXED Fixed by SHA1| |29446a66ce2aab5d03320d8b8eb | |425213b84727f --- Comment #19 from Robert Wilhelm <sloper42(a)yahoo.com> --- Fixed with MR from Francis. https://gitlab.winehq.org/wine/wine/-/commit/29446a66ce2aab5d03320d8b8eb4252... Thanks for working on it. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 --- Comment #20 from francisdb <francisdb(a)gmail.com> --- Thanks Robert for the initial patch! -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53644 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #21 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 10.1. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla