CheckInternal::checkRedundantNextPrevious(): Fix FN, simplify

[chrchr-github]

No description provided.

[firewave]

The merge is bad - it removes unrelated stuff.

[firewave]

It still changed unrelated code - see TemplateSimplifier::getTemplateInstantiations().

[firewave]

It seems strange we can detect this now. Looks like some underlying false negative.

[firewave]

Looks clean now.

[firewave]

callgrind in CI: 63,252,683,153 -> 63,298,113,389

[chrchr-github]

That's with edf0104 already applied, right?

[firewave]

callgrind in CI: 63,252,683,153 -> 63,298,113,389

I think this is fine.

But I wonder if there is a potential matchcompiler pass here.
If we dereference the result of any of these calls we could compile this to the corresponding previous() and next() calls.

[firewave]

That's with edf0104 already applied, right?

Yes.

[firewave]

Just a side note:

Calling it "redundant" is a bit misleading as using the Token::*At() functions actually changes the behavior as it prevents the potential dereference of a NULL pointer in a call chain. The additional checks obviously also affect the performance.

[firewave]

But I wonder if there is a potential matchcompiler pass here.
If we dereference the result of any of these calls we could compile this to the corresponding previous() and next() calls.

With such a pass we could get rid of some of the manual optimizations we did by storing the result of an *At() call as only the calls which check the result need to be safe. That might slow down debug builds a bit though.

I will file a ticket about this.

[firewave]

I will file a ticket about this.

https://trac.cppcheck.net/ticket/12805