dart-lang / sdk

The Dart SDK, including the VM, dart2js, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
9.85k stars 1.52k forks source link

Dead code enhancement in logical expressions #47262

Open bwilkerson opened 2 years ago

bwilkerson commented 2 years ago

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 commented 2 years ago

@srawlins

srawlins commented 2 years ago

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 */ }

bwilkerson commented 2 years ago

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

pq commented 2 years ago

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? 🤔

bwilkerson commented 2 years ago

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.