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:

block #2 (out: 3): LBL_1:-1
  0: %self := recv_self
  1:          line_num(lineNumber: 1)
  2:  %v_3 := call_0o(self<%self>, callType: VARIABLE, name: a_func, potentiallyRefined: false)
  3:  %v_4 := copy(hash<sym<>=>fix<1>,sym<>=>%v_3>)
  4:          return(%v_4)

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.

This is an IR item. Pulling in @enebo and @subbuss.

Yes that's exactly our problem. We have a user DSL that centers around
defining a reasonably large hash, with various helper functions that assign
values to elements of that hash. However when there is a syntax error or a
parse error raised in the helper function, the user is clueless as to which
line is the root cause since the backtrace directs them to the first line
of the hash.

Thanks for looking into it.

On Thu, 15 Sep 2016, 22:17 Charles Oliver Nutter, notifications@github.com
wrote:

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:

block #2 (out: 3): LBL_1:-1
0: %self := recv_self
1: line_num(lineNumber: 1)
2: %v_3 := call_0o(self<%self>, callType: VARIABLE, name: a_func, potentiallyRefined: false)
3: %v_4 := copy(hash<sym<>=>fix<1>,sym<>=>%v_3>)
4: return(%v_4)

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.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#4154 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABC5RUFVPkpQEA1xo77VcwJw618CSLlMks5qqVNSgaJpZM4J8XaF
.

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