1.23: SD-4095 finalize change notes for C/C++ #2434
Conversation
change-notes/1.23/analysis-cpp.md
Outdated
| overriding `int explorationLimit()`. | ||
| * The data-flow library has been extended with a new feature to aid debugging. | ||
| If you want to explore the possible flow from a source, replace | ||
| `isSink(Node n) { any() }` with the new `Configuration::hasPartialFlow` predicate. |
There was a problem hiding this comment.
In the second sentence, replace sounds like it should be taken literally, but it shouldn't. Writing isSink(Node n) { any() } was previously a good way to debug data flow, but not everyone did that, so there will be nothing to replace in many cases. Instead, a call to hasPartialFlow should literally replace the call to hasFlow or hasFlowPath.
I propose this text instead for the second sentence:
To explore the possible flow from all sources, including flow that doesn't reach any sink, use the hasPartialFlow predicate on Configuration instead of hasFlow or hasFlowPath.
@aschackmull and @hvitved, what do you think?
There was a problem hiding this comment.
I suggest using the original change not from C#: https://github.com/Semmle/ql/pull/2430/files#diff-95682413e8e7d29056c3f97a3d0b8f43L38-L40
There was a problem hiding this comment.
I agree. The original C# change note was clear.
There was a problem hiding this comment.
I was trying to reorganize the sentence so that it started with what the user was trying to do, rather than how they'd coded it. As well as making the sentence shorter. Clearly my changes were bad. Would this work any better?
Previously, to explore the possible flow from all sources you could specify isSink(Node n) { any() } on a configuration. Now you can use the new Configuration::hasPartialFlow predicate, which gives a more complete picture of the partial flow paths from a given source, including flow that doesn't reach any sink.
There was a problem hiding this comment.
@felicitymay Your latest proposal sounds good to me. I assume it will still be preceded by "The data-flow library has been extended with a new feature to aid debugging." and followed by "The feature is disabled ...".
There was a problem hiding this comment.
Thanks for everyone's patience with this. Now that I'm out of meetings, I'll update this PR and the other change notes.
|
My apologies - I've got my self confused. Will amend that last commit after my next meeting 😞 |
felicitymay
force-pushed
the
1.23/SD-4095-finalize-change-notes-cpp
branch
from
507c272 to
4f66608
Compare
|
I think that the change notes file should now contain the correct wording. |
@Semmle/cpp - I've made some small changes to the text in the first commit. The second commit just fixes the sort order in the first table.
I revised the text describing
Configuration::hasPartialFlowso that it matched the bullet point in the C# change notes (see #2430). Happy to make changes in either/both PRs but it makes sense for the description to be shared text.