Refactoring

[cr1901]

This is a long term goal. I think we can all agree- WLA needs a refactoring, to make the code easier to parse (As well as for me to finish the "adding a new architecture" guide :P).

This is a long term, not-at-all trivial goal. But I think it should be possible in baby steps. After learning about what happened to Netscape, I am convinced that a full rewrite is never a good idea, so changes would need to be made to preserve the old, working code while the new code is developed/tested, and eventually supplants the older code.

I think the first step would be to global-rename a number of global vars, and then trying to remove some of the global state. I think some vars, like the index into the current file (i), should stay global for now. There are likely other vars which make sense that other parts of the assembler should be able to modify them.

[vhelin]

On Sun, Nov 30, 2014 at 6:31 PM, William D. Jones notifications@github.com wrote:

This is a long term goal. I think we can all agree- WLA needs a refactoring, to make the code easier to parse (As well as for me to finish the "adding a new architecture" guide :P).

This is a long term, not-at-all trivial goal. But I think it should be possible in baby steps. After learning about what happened to Netscape, I am convinced that a full rewrite is never a good idea, so changes would need to be made to preserve the old, working code while the new code is developed/tested, and eventually supplants the older code.

I think the first step would be to global-rename a number of global vars, and then trying to remove some of the global state. I think some vars, like the index into the current file (i), should stay global for now. There are likely other vars which make sense that other parts of the assembler should be able to modify them.

True, true. Also lots of local vars should be renamed, and it might be wise to add more local vars instead of recycling them and having "general" vars... This is a good long term goal.

— Reply to this email directly or view it on GitHub https://github.com/vhelin/wla-dx/issues/23.

[cr1901]

I haven't really been work on WLA. since September, I don't really have much time, and WLA still really needs to be refactored. I've found it difficult to flush out parser bugs; to my knowledge, WLA's macro language is not context-free.

I suspect parsing rules being applied incorrectly is the biggest source of bugs. Did you ever write a grammar for WLA?

[vhelin]

On 19 Apr 2016 10:18, "William D. Jones" notifications@github.com wrote:

I haven't really been work on WLA. since September, I don't really have much time, and WLA still really needs to be refactored. I've found it difficult to flush out parser bugs; to my knowledge, WLA's macro language is not context-free.

That is more than probable. :)

I suspect parsing rules being applied incorrectly is the biggest source of bugs. Did you ever write a grammar for WLA?

Haha no. :) WLA DX is the result of pure organic growth and two partial rewrites. When I started making WLA DX I was learning C and I'm sure I had no idea about writing grammars LOL. :) For years and years I was the only one coding, didn't get many reports etc. mail about it, and here we are. :)

— You are receiving this because you were assigned. Reply to this email directly or view it on GitHub

[nicklausw]

Wait, what do you mean by a "grammar"? Like, a proper parser?

[vhelin]

I think something like this:

https://en.wikipedia.org/wiki/Formal_grammar

Yeah, well, rules for the parser. It's been possible for years, I think, to first create the grammar for a parser, and use a specific code generator to create (C? C++?) code that parses such data/"language". I didn't use anything like that when making WLA DX, just incrementally added new rules and stuff to the parser / assembler...

Man the hot weather here in Philippines (Thailand was equally hot) is still draining my batteries - evenings just fly by and I really can't get anything done. :) 50 days behind me and I still haven't gotten used to this weather. It's rarely this hot back in Finland, just a couple of days every summer...

On Tue, Apr 19, 2016 at 10:03 PM, nicklausw notifications@github.com wrote:

Wait, what do you mean by a "grammar"? Like, a proper parser?

— You are receiving this because you were assigned. Reply to this email directly or view it on GitHub https://github.com/vhelin/wla-dx/issues/23#issuecomment-211935338

[ssjason123]

I wrote up a sample grammar for WLAGB in Antlr 4, its got a common import for the generic WLA support to make it simple to expand out to the other hardware. Its pretty nasty though. I wanted the grammar to do most of work on directives/opcodes.

A smaller grammar could be written up to treat stuff like directives/opcodes/calls generically and do specific command processing outside of the grammar parse logic.

It's checked in to my branch at: https://github.com/ssjason123/wla-dx/tree/parserUpgrade/grammar

Issues:

• Comments using the * at the start of line wont work on the first line in the file. Antlr doesn't have a nice start of line command so it relies on finding a newline as the previous token.
• WLA allows labels/defines that match opcode/register names which gets pretty nasty in Antlr requiring extra defines to treat the keywords as labels.
• Antlr is a greedy parser which required preserving newlines for macros otherwise multiple lines get sucked in and treated as a single macro.

[lhsazevedo]

True, true. Also lots of local vars should be renamed, and it might be wise to add more local vars instead of recycling them and having "general" vars... This is a good long term goal.

Interesting, I think I can try :)

I think some vars, like the index into the current file (i), should stay global for now

Maybe rename it to something like current_file_index? That would free i for loops.

[vhelin]

It would be good to use local vars for running numbers etc. local needs. Currently global variables like ind and inz are used like local variables. This is because when I started this project 1998 I was also learning C at the same time and couldn't do better. And I just kept on using ind and inz as local vars afterwards, didn't refactor... :)

But getting rid of global ind and inz might be a big task, but a good one. Getting rid of global i would also be a big thing, but also a big task...

[jeffythedragonslayer]

I'm available if you need volunteers. I love refactoring code.

[vhelin]

Here's a file that needs more loving:

https://github.com/vhelin/wla-dx/blob/master/decode.c

g_ind and g_inz are global variables, but are used in functions as local variables -> add local vars with good names to those functions and use them instead of g_ind and g_inz. :)

[vhelin]

Moved from...

https://github.com/vhelin/wla-dx/pull/556

...

One issue, I think, I just went looking for such, are the instruction files:

https://github.com/vhelin/wla-dx/blob/master/i65816.c

{ "ADC x", 0x65, 0xA, 2 }, { "ADC ?", 0x6D, 2, 1 }, { "ADC &", 0x6F, 3, 0 },

First comes the instruction, then the opcode, then the parser used in decode.c...

... and then something called skip_xbit which if 2 let's the parser to flow from 8bit into the next, 16bit argument version of the instruction. If it's 1 then flowing from 16bit -> 24bit is allowed.

Perhaps the elements of instruction struct in defines.h could have better names, and we might want to use #defines instead of 2, 1 and 0 there? Also the parser group (type in instruction) could be called e.g., well, parser?

Any other ideas, anyone?

[vhelin]

Also the instruction struct in defines.h is too complex:

Maybe unpack that struct into multiple copies of it, one per architecture, and move those higher up in the file, like the 65816 version of struct instruction goes into here

[jeffythedragonslayer]

I agree about #defines instead of 0, 1, 2. How do you feel about stdbool.h? Quite a few things in that struct instruction look like booleans.

[vhelin]

I think stdbool.h comes in ANSI C99 and as we are using ANSI C90 here it would not work.

[jeffythedragonslayer]

Are there users who are stuck on an ANSI C90 compiler?

[vhelin]

Well, WLA DX itself is stuck with ANSI C90 as we want it to compile on an Amiga, MSDOS... Old systems.

[jeffythedragonslayer]

So it sounds like in order to move WLA DX to C99, there would have to be good C99 compilers for all of those old systems. That could take quite a long time to happen.

[vhelin]

Looking at this

https://en.wikipedia.org/wiki/ANSI_C

I don't see that much useful stuff in C95 and C99 that could be used in WLA DX. IMHO there would have to be some serious and numerous reasons to switch to C99 in order to do it.

[vhelin]

One thing one can do to make the code better is just to go through the source files, and if you spot a function that has local variables named like i, x, y, whatever-really-non-describing then just rename the local variable to what it really is. :)

[jeffythedragonslayer]

I actually prefer i over loopIndex when that's what it means xD

[vhelin]

Me too! What I meant that if there is code like

/ calculate the area of a box / j = box_width * box_height;

then j should really be called "area", not j...

[jeffythedragonslayer]

Here's one that's not clear what it's declaring:

https://github.com/vhelin/wla-dx/blob/master/wlalink/discard.c#L26

[vhelin]

True, a and b should have better names…

On Thu, 9 Feb 2023 at 07:55, Jeff Linahan @.***> wrote:

Here's one that's not clear what it's declaring:

https://github.com/vhelin/wla-dx/blob/master/wlalink/discard.c#L26

— Reply to this email directly, view it on GitHub https://github.com/vhelin/wla-dx/issues/23#issuecomment-1423674423, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ62US6TSHUB2IFOLZJ5XTWWSBD7ANCNFSM4AYITDEA . You are receiving this because you were assigned.Message ID: @.***>

[jeffythedragonslayer]

How about num_dropped_before and num_dropped_after ?

[vhelin]

Sounds good.

But now that I've looked at the function they are in, wouldn't it make more sense to make discard_iteration() to return the number of discarded sections? The the while loop would just be

while (discard_iteration() > 0)
   ;

EDIT: I think I'll do this later...