exercism / elixir-analyzer

GNU Affero General Public License v3.0
31 stars 32 forks source link

Remove protocol consolidation to fix warning in valid code #252

Closed jiegillet closed 2 years ago

jiegillet commented 2 years ago

Closes #251.

My initial hypothesis (in the issue) was not quite right. Protocol consolidation is a feature that links protocols to their implementation in order to speed up the protocol dispatch. This can only work when all implementations are known, typically at compile time.

In the case of clock, when we compile the file in ElixirAnalyzer.ExerciseTest.CommonChecks.CompilerWarnings, an extra implementation is defined and the compiler refuses to include it because the "protocol has already been consolidated" at compile time.

The docs only show how to turn it off for tests, so I brought my hammer for this fix and I turned off the consolidation altogether. The tradeoff is supposedly a performance cost, but I didn't notice any difference. I'm of course open to other ideas.

angelikatyborska commented 2 years ago

Initially I was worried about this because I remembered something about protocol consolidation being crucial to one exercise, but thankfully that was in the test runner: https://github.com/exercism/elixir-test-runner/pull/51