C++: Reveal false negative in test case

[asgerf]

Some of the sinks in this test were flagged for the wrong reason.

The flow into these sinks isn't working properly, but this was not revealed by the test since an alternate, spurious path exists for each of them. The spurious path goes through the implicit read at one of the prior sinks and takes a use-use step.

In one case the false negative can be revealed by swapping the order of the sinks, but in the other case I had to remove some of the sinks to avoid the interference.

This came up while trying to fix the issue with use-use flow after an implicit read, and this test started failing because the fix removes the spurious paths that the test was relying on.

[geoffw0]

Changes look fine.

Is there any particular reason you want to change the tests, rather than just changing the expected results when you fix the issue? (with a note about why they've happened, I suppose)

[geoffw0]

... on a second look, I think some of these tests and the related models are a bit naive about collection content, which we do now have in CPP (mostly). I can perhaps find time to fix things a bit.

[asgerf]

Is there any particular reason you want to change the tests, rather than just changing the expected results when you fix the issue? (with a note about why they've happened, I suppose)

Mainly I wanted to be confident that the fix is 100% correct, and it's just easier to get there when there are no suspicious-looking test changes, like losing a TP. I guess I could have described the issues in the PR, but I worry that the discussion could get a bit muddled in the weeds of a PR that affects multiple languages (Java also had a similar PR).

[jketema]

Instead of modifying the existing tests, I think it would be better to add some tests. It's not clear to me that moving things around here means that we are still testing what we were testing before.

[asgerf]

Instead of modifying the existing tests, I think it would be better to add some tests

The problem is that the test is going to fail once I merge #17262, and we obviously can't make it not-fail by adding more tests.

[jketema]

Then the test results should be updated with #17262, and the test shouldn't be modified.

[asgerf]

Sounds good to me. I've merged the shared PR. This PR now just updates the inline test annotation, but feel free to close and handle the test however you like.

[jketema]

Thanks.