Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Implemented a basic compiler that integrates lexer and parser + assembler directives explained #4
Implemented a basic compiler that integrates lexer and parser + assembler directives explained #4
Changes from all commits
946633d
c954e98
31a0743
da9de46
296bc53
0c3478a
a5d7536
f25432b
bfa2e48
5d60213
4a9d542
060d5db
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This leads to undefined behaviour, as per the C90 spec, since the value of
f()
is used by the driverexample_driver.c
:Section 6.6.6.4
If a return statement without an expression is executed, and the value of the function call is used by the caller, the behavior is undefined.
https://web.archive.org/web/20200909074736if_/https://www.pdf-archive.com/2014/10/02/ansi-iso-9899-1990-1/ansi-iso-9899-1990-1.pdf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The basic compiler incorrectly compiles
return
as if it wasreturn 5
to still pass the example test case and avoid undefined behaviour while providing the bare minimum AST implementation. Do you think that makes sense?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps best to revert
example.c
fromreturn;
toreturn 5;
, so as to avoid undefined behaviour?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree; as much as it's annoying to modify again, I don't think we want to be demonstrating/testing UB.
From the testing aspect, students must always pass this test case since it's UB (which I'm pretty sure we say that we won't include in tests somewhere).
From the teaching aspect, I had to stare at this for a few mins and do a fair bit of googling to figure out what this should actually do, even if the driver solution is hardcoded -- so I think that takes away from the simple example.
Hopefully its not much more to add than as per below:
Oh, and a few
{ $$ = $1; }
sAlthough, be warned that this may cause about 7 more seen tests to pass if I remember correctly :D