milessabin / shapeless

Generic programming for Scala
Apache License 2.0
3.4k stars 533 forks source link

illTyped does not display proper message if compiler crashes #789

Closed soronpo closed 7 years ago

soronpo commented 7 years ago

I recently hit a situation that illTyped(""" < Compiler_Crashing_Code > """) causes a compiler crash for the type check. It caused a compiler error that there was no type error. I can reproduce the problem, but not in a minimal code (dependent on a library that can cause the crash). It would be better if the implementation of illTyped can somehow understand the compiler crashed and return the appropriate error message.

milessabin commented 7 years ago

Does the compiler crash manifest itself as an exception thrown out of c.typeCheck?

soronpo commented 7 years ago

If I understand the question correctly, I don't know. I did not try to debug the illTyped macro.

milessabin commented 7 years ago

If the crash is due to a c.typeCheck call which exercises a buggy macro in another library, then there's not a lot that illTyped can do unless the crash is an exception which is thrown through it, in which case it could report that. If there's no exception, just a bogus tree which causes the compiler to crash later, then I don't see that there's anything that illTyped can reasonably do to detect that.

soronpo commented 7 years ago

Looking at the implementation, what happens if an unhandled exception is raised? Currently the macro explicitly handles only two specific exceptions.

milessabin commented 7 years ago

Any exceptions not handled by illTyped will be propagated to the caller (ie. the typechecker invoking the illTyped macro).

soronpo commented 7 years ago

But isn't c.error(c.enclosingPosition, "Type-checking succeeded unexpectedly.\n"+expMsg) superceding them?

milessabin commented 7 years ago

If c.typecheck throws an exception then c.error won't be called.

Is an exception other than TypeCheckException or ParseException being thrown out of c.typecheck?

soronpo commented 7 years ago

I guess not, since c.error is called. So indeed there is nothing that can be done without fixing c.typecheck. Maybe this behavior should be documented at illTyped, that mentions that in case a the expression being typechecked is causing a compiler-crash, then illTyped may report that type-checking succeeded unexpectedly.

milessabin commented 7 years ago

Why do you say "fixing c.typecheck"? Isn't the failure in the other library? This is singleton-ops right? Are you sure it's not generating bogus trees?

soronpo commented 7 years ago

The error occurs in Scalac 2.11.11, but is resolved in 2.12. The compiler is crashing in the following case: illTyped("""val ret : W.`2`.T + W.`1`.T = implicitly[W.`4`.T - W.`2`.T]""") But not in this workaround: illTyped("""val impl = implicitly[W.`4`.T - W.`2`.T]; val ret : W.`2`.T + W.`1`.T = impl""")

soronpo commented 7 years ago

And I mean 'fixing' c.typecheck because it does not raise an exception in case of a crash. It doesn't matter where is the fault for the crash, but I would have expected it to raise an an exception.

milessabin commented 7 years ago

I don't think it's quite that straightforward. c.typecheck has produced an invalid tree which is causing a compiler crash later ... arguably c.typecheck should be able to validate all trees that it returns, but that's never been the case and I think there's really very little prospect that it ever will.

The only possible fix is to solve the problem at the source by not creating invalid trees in the first place.

milessabin commented 7 years ago

I think we've established that this problem lies elsewhere? Shall I close this now?

soronpo commented 7 years ago

Running val ret : W.`2`.T + W.`1`.T = implicitly[W.`4`.T - W.`2`.T] causes a compiler crash, while val impl = implicitly[W.`4`.T - W.`2`.T]; val ret : W.`2`.T + W.`1`.T = impl does not. At 2.12 there is no crash at all. Are you saying singleton-ops might still be producing invalid trees, and somehow the workaround or scala 2.12 manage to mask this?

milessabin commented 7 years ago

Perhaps. Or perhaps you're seeing a bug in 2.11.x which was fixed in 2.12.x.

milessabin commented 7 years ago

Why not open a ticket for this on the singleton-ops issue tracker with the stack trace from the 2.11.x compiler crash and I'll try and find some time to take a look.

soronpo commented 7 years ago

I think we've established that this problem lies elsewhere? Shall I close this now?

Yes, just did :smile: . I think that illTyped can benefit if the compiler crash behavior is documented.

Why not open a ticket for this on the singleton-ops issue tracker with the stack trace from the 2.11.x compiler crash and I'll try and find some time to take a look.

OK. I will open a ticket for singleton-ops. I can't expect you to dive into the implementation, but would love to get your feedback.

soronpo commented 7 years ago

FYI, the issue opened for singleton-ops: https://github.com/fthomas/singleton-ops/issues/43