-
-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrong line number reported in backtrace in hash creation #4154
Comments
If I put just the hash in a file, we report line 2, the first key/value pair. It would seem that MRI tracks the line number all the way through. Here's IR for a simple hash with one simple key and one that errors:
So basically, we lose all line number information because the hash construction itself happens as a single unit, with each key/value pair evaluated before hand. I suppose we could insert line number instructions around anything that might raise an error (all calls, for example). This case seems like the line number is not a big deal, but imagine having a giant literal hash with many keys that might error...finding the bad one would be troublesome. |
Yes that's exactly our problem. We have a user DSL that centers around Thanks for looking into it. On Thu, 15 Sep 2016, 22:17 Charles Oliver Nutter, notifications@github.com
|
I think we can just emit linenum out of order in this case before each temp var assignment for each expr that builds up the hash. I checked and the values/keys seem to have proper line number info in the ast. For pure-literal hashes we do not need to omit since they cannot raise by themselves. |
@enebo I started looking at this a bit. It does not seem like we have a good method on Node to indicate that a construct can't raise an error. That's basically what we need here. We could do that with another override, like needsDefinitionCheck, or via a visitor that walks a subtree looking for nodes that could potentially raise an exception. |
@headius I do not think we need to know. It is not about the node but more about the operands. For Hash is basically toggle between build and buildWithOrder depending whether we use anything beyond simple operands (e.g. no chance of side-effects). The problem I had when I looked at this is we do not mark newlines in node on hash elements. So we could pass in last elements line into buildWithOrder and emit linenum if it changes and it was an instr which assigns to a temp? It is more complicated than I thought but I think we might be able to use newline boolean on nodes when line changes and only emit in those cases when we pass right boolean to buildWithOrder. Should I run this paragraph on any longer? :) |
Actually maybe we can just look for: preserveOrder && !(value instanceof ImmutableLiteral) and if so emit linenum instr. The main downside is this is a little to overly simplistic in that all assignments (which nearly universally can cause side-effects) might end up emitting a linenum for the same line multiple times (once per use). We could make a buildWithOrder(Node node, boolean preserveOrder, int lastLine) where we pass in previous processed key/value and only emit if the value is different (along with the condition above in buildWithOrder). Hmmm I will try quickly |
Environment
Expected Behavior
Correct line number is reported in backtrace when raising error in function called within hash creation statement.
Example code:
Actual Behavior
Output in MRI 2.3.0
Output in JRuby
Cross referencing another linenum issue: #4211
The text was updated successfully, but these errors were encountered: