Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves #6 implement asm translator with documentation #7

Merged
merged 7 commits into from
Feb 18, 2024

Conversation

k1b24
Copy link
Contributor

@k1b24 k1b24 commented Feb 12, 2024

Реализация транслятора asm

Лазеев Сергей P33131

@k1b24 k1b24 changed the title resolves #6 implement asm translator with documentation Resolves #6 implement asm translator with documentation Feb 12, 2024
Copy link
Owner

@ryukzak ryukzak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general -- looks well. But as education purpose I plan to significantly rewrite your code to be more simple after you finish these fixes.

I will call you for review my PR.

| "jmp" arg
| "jz" arg

arg ::= integer
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need integer here? label_name should be enough.

Comment on lines 16 to 23
instr ::= "increment"
| "decrement"
| "right"
| "left"
| "print"
| "input"
| "jmp" arg
| "jz" arg
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
instr ::= "increment"
| "decrement"
| "right"
| "left"
| "print"
| "input"
| "jmp" arg
| "jz" arg
instr ::= op0
| op1 arg
op0 ::= "increment"
| "decrement"
| "right"
| "left"
| "print"
| "input"
op1 ::= "jmp"
| "jz"

``` ebnf
program ::= { line }

line ::= [ label ] [ instr ] [ comment ] "\n"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more simple:

Suggested change
line ::= [ label ] [ instr ] [ comment ] "\n"
line ::= label: [ comment ] "\n"
| instr [ comment ] "\n"
| [ comment ] "\n"

Реализован в модуле [translator_asm](./translator_asm.py)

Ассемблерные мнемоники один в один транслируются в машинные команды.
Метки из программы исчезают, а на место обращений к ним подставляются конкретные адреса.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need golden tests for the translator.

Реализован в модуле [translator_asm](./translator_asm.py)

Ассемблерные мнемоники один в один транслируются в машинные команды.
Метки из программы исчезают, а на место обращений к ним подставляются конкретные адреса.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two paragraphs?

"""Разбор текста на словарь с определением меток и список машинных инструкций."""
pc = 0
labels = {}
instrs = []
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to code, as in translator.py

@k1b24
Copy link
Contributor Author

k1b24 commented Feb 15, 2024

@ryukzak may you take a look, please

Copy link
Owner

@ryukzak ryukzak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сделал вам ПР с рефакторингом: k1b24#1

Можете если необходимо документацию обновить.

@@ -60,8 +62,12 @@ def test_translator_and_machine(golden, caplog):

# Создаём временную папку для тестирования приложения.
with tempfile.TemporaryDirectory() as tmpdirname:
# Определяем язык golden теста. По умолчанию предполагается язык Brainfuck
lang = golden.get("lang", "bf")
assert lang == "bf" or lang == "asm"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be a little bit more simple. Just pass: in_source xor in_source_asm.

If both or none -- just fail the test.

Copy link
Contributor Author

@k1b24 k1b24 Feb 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this code will be a little bigger, if I get you right

lang = None
source = None
if "in_source_asm" in golden:
  lang = "asm"
  source = golden["in_source_asm"]
if "in_source" in golden:
  assert lang is None, "both in_source and in_source_asm are present"
  lang = "bf"
  source = golden["in_source"]
assert lang is not None, "either in_source xor in_source_asm must be set"

And in this case we also need to add some logic about referencing the in_source param on this line below:
file.write(golden["in_source"])

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an idea

@k1b24
Copy link
Contributor Author

k1b24 commented Feb 17, 2024

Merged PR and fixed some typos in the documentation

Comment on lines +66 to +67
lang = golden.get("lang", "bf")
assert lang == "bf" or lang == "asm"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
lang = golden.get("lang", "bf")
assert lang == "bf" or lang == "asm"
assert "in_source" in golden != "in_source_asm" in golden

# Готовим имена файлов для входных и выходных данных.
source = os.path.join(tmpdirname, "source.bf")
source = os.path.join(tmpdirname, "source." + lang)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
source = os.path.join(tmpdirname, "source." + lang)
source = os.path.join(tmpdirname, "source")

No body see this files...

Comment on lines +83 to +86
if lang == "bf":
translator.main(source, target)
elif lang == "asm":
translator_asm.main(source, target)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if lang == "bf":
translator.main(source, target)
elif lang == "asm":
translator_asm.main(source, target)
if "in_source" in golden:
with open(source, "w", encoding="utf-8") as file:
file.write(golden["in_source"])
translator.main(source, target)
elif ...

@@ -60,8 +62,12 @@ def test_translator_and_machine(golden, caplog):

# Создаём временную папку для тестирования приложения.
with tempfile.TemporaryDirectory() as tmpdirname:
# Определяем язык golden теста. По умолчанию предполагается язык Brainfuck
lang = golden.get("lang", "bf")
assert lang == "bf" or lang == "asm"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an idea

@ryukzak ryukzak merged commit 2953b33 into ryukzak:master Feb 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants