fix same-line detection to be source aware#1834
fix same-line detection to be source aware#1834ironm00n wants to merge 1 commit intobrownplt:horizonfrom
Conversation
blerner
left a comment
There was a problem hiding this comment.
This is weird, but I think I get it. I think you have it subtly wrong, though:
student-code1 inserted-code1 student-code2
will cause no distinct-line errors to be emitted. I think you might want to remove this change here, and instead call ensure-distinct-lines on a filtered list of statements, where you remove any instructor-inserted code. Does that meet your needs, or am I misunderstanding the problem?
| cases(Loc) loc: | ||
| | builtin(_) => ensure-distinct-lines(first.l, A.is-s-template(first), rest) | ||
| | srcloc(_, _, _, _, end-line1, _, _) => | ||
| | srcloc(end-source1, _, _, _, end-line1, _, _) => |
There was a problem hiding this comment.
This should just be named source1 -- the fields being extracted here are named end-line and start-line, and the suppliers are numbered 1 and 2. So if you're extracting a new field, just name it source
For our use-case that is fine, since we already rely on a student's file being well-formed before performing any AST manipulations. Our motivation being that if a program has content from multiple different sources, the srcloc information is far less meaningful here.
I'm not sure what this would entail, currently we run the merged code through the entire compiler pipeline, the amount of work to book keep for what segments of the code correspond to who originally wrote it, and then selectively running different compiler passes doesn't seem like a good use of anyone's resources. Perhaps our needs are more along the lines of being able to be more selective about well-formedness checks are run on our resulting code? We just implemented this fix since a few students had hit edge cases where the line numbers of our files and theirs would collide and cause the autograder's REPL to reject their program. |
|
I was just suggesting that at the call site for |
This is only an issue when a program has srclocs from multiple files, but we run into this in the autograder.