search

mc-imperial / dredd

Framework for evaluating C/C++ compiler testing tools
Apache License 2.0
11 stars 3 forks source link

Attempt to fix issue 230 #231

Closed JonathanFoo0523 closed 2 months ago

JonathanFoo0523 commented 3 months ago

The GetRegularDreddPreludeCpp and GetRegularDreddPreludeC functions have been modified with the following changes:

(a) The return type is now bool instead of int. If the return expression is of type uint64_t, returning an int could truncate the value. By returning a bool, the result will be 0 if the value compares equal to 0; otherwise, the result will be 1.

(b) Any int literal involved in a bit shift operation is explicitly cast to uint64_t.

Additionally, we have added a new directory at test/compile_execute_files, which contains the C/C++ program, expected std output, and runner script for simple regression tests.

JamesLee-Jones commented 3 months ago

Provided @afd is happy with it, I can keep manually running the CI but it's useful to run scripts/check_all.sh as well as this will give you a good idea of whether it will pass without it being run.

JonathanFoo0523 commented 2 months ago

Thanks for this! Can you please:

  • Put your changes on a branch under JonoathanFoo0523 - this will allow me to check out that branch to review your changes locally.
  • Adapt the title of the PR to indicate what it's about. It is good practice to mention the issue in the PR, but you usually do that in the PR description, e.g. via "Fixes Attempt to fix issue 230 #231." In the title of the PR it's good to use words that relate to the content of the contribution, so that when developers look through a list of PRs they can tell roughly what they are about.
  • Also, since you're going to put this on a branch, etc., please rebase your changes over the latest changes to main, and re-generate the single file test cases (as other commits have added a few more in the meantime).

I don't think GitHub allows me to change the source branch (from JonathanFoo0523:main) of a PR to different branch (e.g., JonathanFoo0523:fix-something-branch). If necessary, I can close this PR and create a new PR with different source branch.

afd commented 2 months ago

@JonathanFoo0523 Yes, please open a new PR with this on a branch.

afd commented 2 months ago

@JonathanFoo0523 When you do your new PR for this, please can you see if you can set up the new test case as an "execute" test, using the infrastructure I have added via #255 ? Please see the README under tests/execute, as well as the tests/execute/add example - hopefully this can be made to fit the bill for what you are doing.

JonathanFoo0523 commented 2 months ago

@afd Sure! I am working on it.