Make applyDebug's free variable check take the context into account

[leonschoorl]

Fixes #2623

Still TODO:

• Write a changelog entry (see changelog/README.md)
• Check copyright notices are up to date in edited files

[martijnbastiaan]

LGTM in general! Reformat of comment + question about it in the comments.

[martijnbastiaan]

Suggested change

	-- Since [Give evaluator acces to inscope let-bindings #2571](https://github.com/clash-lang/clash-compiler/pull/2571)
	-- the evaluator can rewrite expressions using let bindings from the 'TransformContext',
	-- these bindings may reference other things bound in the context which weren't
	-- in the expression before, and in doing so introduces new free variables and
	-- fails this check for new free variables.
	-- To prevent this we filter out all variables from bound in the context.
	-- But only during a caseCon transformation, to not weaken this check unnecessarily.
	-- Since [Give evaluator acces to inscope let-bindings #2571](https://github.com/clash-lang/clash-compiler/pull/2571)
	-- the evaluator can rewrite expressions using let bindings from the 'TransformContext'.
	-- These bindings may reference other things bound in the context which weren't
	-- in the expression before, and in doing so introduces new free variables. Consequently,
	-- this fails a check that prevents new free variables from being introduced -
	-- as this is a bug in all other cases. To prevent this we filter out all
	-- variables from bound in the context. We only do this during a 'caseCon' transformation,
	-- to not weaken this check unnecessarily.

I've reformatted / broke up sentences to make it easier to read.

I'm not sure about this though:

We only do this during a 'caseCon' transformation,
to not weaken this check unnecessarily.

Is this because the evaluator is only called in caseCon? In any case, I think we should defend why we think we only need to do it for caseCon in the note.

[martijnbastiaan]

@christiaanb maybe you could comment on this?

[christiaanb]

The evaluator is called in multiple places, including caseCon. But as @leonschoorl points out, the "introduce free variables" check is quite useful to catch bugs in the Clash compiler, so we want to leave it enabled in as many places as possible. So it's only disabled in caseCon as we have a confirmed "false positive" for that transformation.

[martijnbastiaan]

Okay, this sounds like: we're aware this is an issue, and we're going to wait until another transformation calling the evaluator is going to blow up..

What we should do is check the other transformations calling the evaluator, check whether they suffer from the same issue, and add a comment explaining why we do (not) need a similar approach as introduced in this PR.