Skip to content

Conversation

@clzll
Copy link
Contributor

@clzll clzll commented Feb 22, 2025

About 300 lines of code are the same in add.asm, sub.asm and mul.asm, thus it makes sense to move that code out.

Copy link
Member

@dedlocc dedlocc left a comment

Choose a reason for hiding this comment

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

не вижу смысла переусложнять и разбивать на 100500 файлов и обмазываться макросами

если уж на то пошло, я бы просто сделал условный common.asm, в который продублировал из add.asm всё, кроме собственно самого сложения и _start, и добавил его pre-include'ом (https://www.nasm.us/doc/nasmdoc2.html#section-2.1.19)

@clzll clzll force-pushed the feature/extract_common_code branch from f3aa198 to a07a27e Compare February 23, 2025 14:21
Copy link
Member

@dedlocc dedlocc left a comment

Choose a reason for hiding this comment

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

  1. Давай добавим в README пояснение того, что тут происходит. Может быть неочевидно, откуда функции берутся.
  2. В исходных sub.asm и mul.asm возможно всё же стоило бы оставить копию add_long_long (пусть даже с другим названием), так проще смотреть дифф, в частности для sub

@clzll clzll force-pushed the feature/extract_common_code branch from a07a27e to 89f9608 Compare February 23, 2025 17:20
Copy link
Member

@dedlocc dedlocc left a comment

Choose a reason for hiding this comment

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

Спасибо. Мёрджить в этом году уже не будем, чтоб не создавать конфликты в случае, если будут обновления, но в следующем может быть полезно.

@clzll clzll force-pushed the feature/extract_common_code branch from b2c22a6 to 89f9608 Compare March 4, 2025 20:02
@dedlocc dedlocc mentioned this pull request Mar 9, 2025
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