Skip to content

Combine Duplicated classes#399

Merged
ksh8281 merged 1 commit intoSamsung:mainfrom
kwonjeomsim:optimize
Apr 13, 2026
Merged

Combine Duplicated classes#399
ksh8281 merged 1 commit intoSamsung:mainfrom
kwonjeomsim:optimize

Conversation

@kwonjeomsim
Copy link
Copy Markdown
Contributor

This PR addresses Current Issues 1 in issues#398https://github.com/Samsung/walrus/issues/398#issue-4125897672.

For this, some macros are defined and functions which has similar content are combined with this macros.
Also, to reduce duplicated codes, parent classes are added.

@zherczeg
Copy link
Copy Markdown
Collaborator

zherczeg commented Apr 2, 2026

Looks nice!

@clover2123
Copy link
Copy Markdown
Collaborator

@zherczeg
It seems that one test case, redBlock.wasm, is still failing regardless of this PR update.
Could you help me figure out how to resolve this issue?

@zherczeg
Copy link
Copy Markdown
Collaborator

zherczeg commented Apr 9, 2026

Usually it is a hard task. First thing is reproducing the failing in your side. Then you need to locate the issue. If possible test reduction can help (simplify C code, then recompile). Another way is comparing the original and the new run. Dump the memory (stack) at checkpoints (e.g. at certain instructions), and see whether there is a difference. It is similar to bisecting: a wrong instruction is executed in a range of a correct (the dump of both the old and new code is the same) and incorrect dump. Reduce the range until you locate the instruction.

Another way is splitting the patch into small patches, and see which one cause the error. You only need to check that single test.

@ksh8281
Copy link
Copy Markdown
Collaborator

ksh8281 commented Apr 10, 2026

could you rebase your pr?

@kwonjeomsim
Copy link
Copy Markdown
Contributor Author

@ksh8281 I rebased pr. Please check again!

Comment thread src/interpreter/ByteCode.h Outdated
@@ -32,8 +32,10 @@ class FunctionType;

#if defined(NDEBUG)
#define F_NOP(F)
#define IF_DEBUG_CODE(x)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would call it IF_DEBUG_ENABLED.

@ksh8281 ksh8281 merged commit 0e30d99 into Samsung:main Apr 13, 2026
89 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.

4 participants