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

Prevent expression-under-cast-bypass in Initializer Lists for more mutations and narrowing type handling #278

Merged
merged 8 commits into from
Jul 16, 2024

Conversation

JonathanFoo0523
Copy link
Collaborator

Avoid bypass of implicit inner cast mutation under default (optimize_mutations) mode for expressions in Initializer List. Besides producing more mutants, this change ensures that Dredd can correctly handle narrowing type expressions involving std::initializer_list without bypassing necessary static_cast operations.

Fixes #270

@JonathanFoo0523 JonathanFoo0523 changed the title Prevent implicit cast bypass in Initializer Lists for more mutations and narrowing type handling Prevent expression under cast bypass in Initializer Lists for more mutations and narrowing type handling Jul 12, 2024
@JonathanFoo0523 JonathanFoo0523 changed the title Prevent expression under cast bypass in Initializer Lists for more mutations and narrowing type handling Prevent expression-under-cast-bypass in Initializer Lists for more mutations and narrowing type handling Jul 12, 2024
@JonathanFoo0523 JonathanFoo0523 requested a review from afd July 12, 2024 21:18
@JonathanFoo0523
Copy link
Collaborator Author

Check failed due to

Found 68841 mutants when mutating the SPIR-V validator source code. Expected 68821. If Dredd changed recently, the expected value may just need to be updated, if it still looks sensible. Otherwise, there is likely a problem.

@JonathanFoo0523 JonathanFoo0523 requested review from afd and removed request for afd July 13, 2024 16:45
@afd
Copy link
Member

afd commented Jul 15, 2024

@JonathanFoo0523 The failure looks reasonable since your change introduce more mutants. Therefore can you please go into .github/workflows/cxx_apps.sh and change the expected value for the number of spirv-val mutants? Then try the CI again and it will probably fail for an analogous reason with respect to LLVM InstCombine, which you can also update (assuming the change is that the number of mutants goes up slightly). These checks are mainly in place to guard against unexpected changes - where we do a fix or refactoring that should have no effect on the number of mutants.

@afd
Copy link
Member

afd commented Jul 15, 2024

Oh, looks like you already did this for SPIR-V, so you probably just need to do it for LLVM. You'll have to download the log files to see the change that's needed, as the CI output for cxx_apps is very large.

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 - please merge once bots pass.

@afd
Copy link
Member

afd commented Jul 15, 2024

This is failing because the cognitive complexity of HandleExpr has gotten too high. This is normally where one needs to think about extracting out a helper function. Is there an obvious piece of logic related to your additions that could be extracted out?

@JonathanFoo0523 JonathanFoo0523 merged commit 542f1fd into mc-imperial:main Jul 16, 2024
9 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.

Mutation of expression used in initializer list results in ill-formed program
2 participants