dfgordon / vscode-language-merlin6502

Merlin 6502 assembly language support for Visual Studio Code
GNU General Public License v3.0
3 stars 0 forks source link

Having trouble with USE and PUT #1

Closed d5aa962e closed 11 months ago

d5aa962e commented 1 year ago

Is there any trick to getting USE and PUTs working? I have these three files all in the same worksapce folder, and in MAIN.S, the USE and PUT files are showing as not found, and in SECOND.S, Label1 is not found. The code compiles with Merlin8 and Merlin32. Thanks!

MAIN.S:

USE MACROS  <--- "file not found in workspace"

put SECOND <--- "file not found in workspace"

Label1 lda #5 jsr $FDED RTS

=======

MACROS.S

TestMac MAC lda #5 <<<

======

SECOND.S

Start jsr Label1 <--- global label is undefined RTS

dfgordon commented 1 year ago

It looks like there is a difference of behavior on Linux vs. Windows. I'm guessing you are on Windows? There is likely something Linux specific that found its way into the path manipulations. There is a further issue with labels that are forward referenced in a PUT file. So there are a couple of bugs, I will try to patch soon.

d5aa962e commented 1 year ago

Correct. Windows 10. I'll try testing on Linux. Thanks!

dfgordon commented 1 year ago

There was a POSIX path hard coded into a glob pattern. Replacing with the appropriate joinPath function fixes the issue of finding the files. I will push that up shortly. There is also a deeper issue that needs more thought, see below.

dfgordon commented 1 year ago

This issue raises the following points that will take more time to address.

  1. If there is a label referenced in a PUT file, but defined in the main file, it is handled correctly provided the user is editing the main file. If the user is editing the include, the extension will not look in the master file, and will flag the label as undefined. Instead of doing this, the label should probably be flagged in some other way to alert the user that this file will not assemble in isolation. At a minimum hovering should indicate where the label is defined. We might also consider underlining with an info squiggle or some other visual cue.

  2. If there is an error in an include, and the user is editing the main file, the include's path is underlined with an error squiggle, and hovering shows all the errors in the include. This is probably not the right way to indicate the problem. A starting point is to take a look at how the big extensions handle a similar situation, e.g. errors in python imports, etc., and perhaps do as they do.

  3. CI tests are running only on Ubuntu. Running the tests on Windows and Mac as well might help catch more issues.

d5aa962e commented 1 year ago

If the extension looks at all .S files in the workspace, every reference should be resolvable. This would be similar to other languages where if a file is added to the "path" (i.e. into the workspace), definitions in the new file are recognized, and the new file recognizes definitions from other files. Two concerns with this approach are the actual compile would fail if a PUT is missing, and nested PUTs could get very messy. There'd have to be a "PUT tree" defined, and the main file derived.

There shouldn't be too much of a concern about what happens if the file couldn't compile in isolation. Since Merlin didn't have a robust conditional compilation feature, the PUTs were mainly to address large source files as opposed to having truly reusable files. Truely reusable modules (REL/ENT/EXT) would be properly handled via the Linker. If there is a concern, a configuration option could be added to the effect of "Flag 'external' unknown labels".

dfgordon commented 1 year ago

Thanks for the thoughts. What I'm thinking to do is during the workspace sweep build a map from include file URI's to lists of master file URI's they are included in. Then while editing an include, every master file context can be checked, and label references can be flagged as usual. Effect on diagnostic performance in a big project could be an issue, we'll see.

Recursive includes are treated as illegal, that should already be handled. According to Merlin 8/16 documentation they are illegal. Merlin 32 site doesn't seem to say.

If there are any issues with EXT/ENT I would put it in a separate issue.

dfgordon commented 12 months ago

Version 2.0.0 is supposed to address the remaining issues.

d5aa962e commented 11 months ago

Looks good! Thanks!