In D71524#1917251, @Szelethus wrote:

Are we sure this is what we want? If this is a heuristic, we should document it well, and even then I'm not sure whether we want it. I'm also pretty sure this would make the eventual change to CallDescriptionMap more difficult, because the way taintedness is propagated around std::basic_istream not really the property of the global >> operator and what its parameters are, but rather the property of std::basic_istream<CharT,Traits>::operator>>, right? What do you think?

This patch introduced a crash while I was analyzing the libpressio.
I was using the CodeChecker to drive the analysis with the --enable-all flag.

In D71524#1902654, @steakhal wrote:

In D71524#1889566, @boga95 wrote:

@steakhal's revision is on the top of this. Changing the order will only cause unnecessary work on both sides.

I would happily rebase this patch if you want.

I don't have any technical comments on this patch since I haven't used NoteTags yet, only a couple of readability ones.

In D71524#1889566, @boga95 wrote:

@steakhal's revision is on the top of this. Changing the order will only cause unnecessary work on both sides.

In D72035#1889320, @Szelethus wrote:

Wow. Its a joy to see you do C++. LGTM. Are you planning to introduce CallDescriptionMap at one point? :)

Marking comments done.

Declaring the purpose of this checker:

[...]
This would help to reduce the *noise* that the `TaintTest` debug checker would
introduce and let you focus on the `expected-warning`s that you really care
about.

If this patch is good to go, could someone commit it?
I don't have commit access (yet).

In D74806#1882300, @NoQ wrote:

This is fantastic, i love it!

all tests are preserved and passing; only *the message changed at some places*. In my opinion, these messages are holding the same information.

You make it sound like an accident; i encourage you to have a closer look to make sure that there are no other effects than the ones observed on tests.

Upload the right diff.

Rebased on top of master, instead of D71524.

Clarified example usage, especially in contrast of eg.: debug.TaintTest checker.

• Tests added.
• Clang-format-diff applied.

I genuinely think that in the following case we should warn, since the user already had a chance to express the range assumption using an assert.

I'm convinced that we shouldn't remove taint from expressions used in comparisons.

Thank you guys the responses. I cannot agree more. The sole reason why this option exists is that if you scroll back in the git blame of that line, you would find a commit, which removed this warning for this exact scenario.
Possibly due to some seemingly false positives.
I've added the author of that patch to the reviewers of this change, but did not respond.

I think this patch is ok.

Changes:

• Flag option marked as 'enabled by default'.
• Reformat all the test cases for C, C++ and Obj C.
• Now uses -verify=tags approach.
• Fixes checker documentation.

Fixes for @NoQ's comments.
I will update the patch.

@Szelethus The mispositioned report message was my fault. I used a different version of clang for the analysis and to upload the results, which resulted in some mispositioned reports.
I've fixed the linked CodeChecker instance.

@Szelethus Your catch with the mispositioned report message is kinda strange. I will investigate that, but I think it's probably connected to something deeper, and most likely related to CodeChecker itself.

@NoQ What do you think, should it be under a flag (as it would be now), or enabled by default?
I think these warnings are valuable and we should consider it enabling by default.
An interesting fact is that previously rGf224820b45c6847b91071da8d7ade59f373b96f3 patch disabled this warning saying that it generates too many false-positives without any real benefit.

Fix copy-paste mistake.
This time upload the correct version.

Reformatted using clang-format-diff.py.
Minor fixes which were requested by @Szelethus.

Thank you for your response @Szelethus.
Fixed, updating the patch.

Do you have some time @Szelethus to check this change?
Your experience and comments would help a lot to finish this.

What do you think, what should I improve in this checker?
Your remarks, @lebedev.ri, were really valuable.

In D48866#1527540, @lebedev.ri wrote:

In D48866#1527506, @steakhal wrote:

The problem with the -Wcast-align is that it will only fire for C-style bitcast expressions, not for reinterpret_cast ones. example
Does anyone know why is that behavior?

Because reinterpret_cast is by definition allowed to perform these casts, so it is assumed that no warning should be issued.
This part of the check i look forward to.

• Removed different signess related parts.
• Don't warn for the casts which are already covered by '-Wcast-align' check.
• Improved the diagnostic messages:
  • Now adds notes for the first incompatible members of structs.
  • Added alignment, size and type information to most of the warning messages.

The problem with the -Wcast-align is that it will only fire for C-style bitcast expressions, not for reinterpret_cast ones. example
Does anyone know why is that behavior?

Unfortunately the changes that I've made are not available in a diff because I've moved to the monorepo version.
Although, you can see the changes in detail on my llvm-project github fork.

If you don't mind I will finish the leftover work. This will still be committed under your name.

@hgabii Are you planning on finishing this? If not, I'd happily commandeer if not.