sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Dead code enhancement in logical expressions

Open bwilkerson opened this issue 4 years ago • 6 comments

Given a file with the following:

void f() {
  print(true || false);
  print(true && true);
}

The first line is flagged as having dead code, but the second isn't.

We should consider whether it's useful to catch code of this form. It was requested on https://youtrack.jetbrains.com/issue/WEB-52774.

bwilkerson avatar Sep 21 '21 16:09 bwilkerson

@srawlins

bwilkerson avatar Sep 21 '21 16:09 bwilkerson

Great suggestion! I wonder if in general we can just perform const evaluation and report dead code as per the result. E.g. if ('hello'.length == 0) { /* dead code */ }

srawlins avatar Sep 22 '21 01:09 srawlins

I think that would work for expressions that are or could be constants, but it would have to be applied to the operands of the && and || operators, not the whole expression. In your example we wouldn't want to flag 'hello'.length == 0 as dead code because that code would always be executed, even though it always produced the same value.

On the other hand, that would be a good candidate for the invariant_booleans lint, and evaluating the expression as a constant expression might be a good way to implement that lint. @pq

bwilkerson avatar Sep 24 '21 14:09 bwilkerson

Great points!

On the other hand, that would be a good candidate for the invariant_booleans lint,

My only hesitation is that invariant_booleans is already quite tricky and arguably over-reporting (26 open issues 😬.) But maybe const evaluation is a sound path forward for addressing some of them? 🤔

pq avatar Sep 24 '21 15:09 pq

Not to digress too far, but yes. We might be able to eliminate a lot of the complexity by evaluating the expressions. It wouldn't cover all the cases, but it would certainly cover some of them. The lint definitely needs to be cleaned up in order to be useful, and I think that might involve simplifying it somewhat.

bwilkerson avatar Sep 24 '21 15:09 bwilkerson