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

Avoid mutating variables that require const initialization #236

Merged
merged 8 commits into from
Jul 11, 2024
Merged

Conversation

JamesLee-Jones
Copy link
Collaborator

We cannot mutate variable declarations that require constant initialization as Dredd's mutations occur dynamically.

Fixes: #235.

Copy link
Member

@afd afd left a comment

Choose a reason for hiding this comment

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

This looks great - a minor comment change and (if possible) some extra tests requested.

src/libdredd/src/mutate_visitor.cc Outdated Show resolved Hide resolved
@afd
Copy link
Member

afd commented Jul 8, 2024

@JamesLee-Jones This fails with:

2024-07-05T13:45:23.8860447Z + echo 'Found 67841 mutants when mutating the SPIR-V validator source code. Expected 67998. If Dredd changed recently, the expected value may just need to be updated, if it still looks sensible. Otherwise, there is likely a problem.'
2024-07-05T13:45:23.8861595Z + exit 1
2024-07-05T13:45:23.8862744Z Found 67841 mutants when mutating the SPIR-V validator source code. Expected 67998. If Dredd changed recently, the expected value may just need to be updated, if it still looks sensible. Otherwise, there is likely a problem.

It therefore looks as if your change is slightly reducing the number of mutants that are available when spirv-val is mutated. We should look into this. The approach would be:

  • mutate spirv-val just like the CI does with a version of dredd before this change
  • mutate a different checkout of spirv-val with dredd after this change
  • diff the mutated code - this should shed light on which file contains the code that exhibits the discrepancy

After that some manual or automated test case reduction should home in on where the problem is.

@JamesLee-Jones
Copy link
Collaborator Author

JamesLee-Jones commented Jul 8, 2024

@JamesLee-Jones This fails with:

2024-07-05T13:45:23.8860447Z + echo 'Found 67841 mutants when mutating the SPIR-V validator source code. Expected 67998. If Dredd changed recently, the expected value may just need to be updated, if it still looks sensible. Otherwise, there is likely a problem.'
2024-07-05T13:45:23.8861595Z + exit 1
2024-07-05T13:45:23.8862744Z Found 67841 mutants when mutating the SPIR-V validator source code. Expected 67998. If Dredd changed recently, the expected value may just need to be updated, if it still looks sensible. Otherwise, there is likely a problem.

It therefore looks as if your change is slightly reducing the number of mutants that are available when spirv-val is mutated. We should look into this. The approach would be:

* mutate spirv-val just like the CI does with a version of dredd before this change

* mutate a different checkout of spirv-val with dredd after this change

* diff the mutated code - this should shed light on which file contains the code that exhibits the discrepancy

After that some manual or automated test case reduction should home in on where the problem is.

From looking back at the single file test that was modified, I believe it is because the check i used catches const marked variables as well as those marked as requiring const initialization and thus no mutants are produced for expressions of the form const type = value;. If this is the case, then the test case signed_int_constants.cc that was modified by these changes (and is now not) would catch this.

@afd
Copy link
Member

afd commented Jul 11, 2024

@JamesLee-Jones can you rebase this, please?

@JonathanFoo0523 after James has rebased, can you investigate why James's change is reducing the number of mutants that get created for spirv-val? (See cxx_apps.sh under .github/workflows for details of how spirv-val is handled.)

@JamesLee-Jones
Copy link
Collaborator Author

@JamesLee-Jones can you rebase this, please?

@JonathanFoo0523 after James has rebased, can you investigate why James's change is reducing the number of mutants that get created for spirv-val? (See cxx_apps.sh under .github/workflows for details of how spirv-val is handled.)

I believe this commit already resolved the issue.

@JamesLee-Jones JamesLee-Jones requested a review from afd July 11, 2024 10:19
Copy link
Member

@afd afd left a comment

Choose a reason for hiding this comment

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

LGTM!

@afd afd merged commit 776912d into main Jul 11, 2024
9 checks passed
@afd afd deleted the issue235 branch July 11, 2024 10:22
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.

Dredd mutates variables that should have a constant initializer
2 participants