Fix tr srcloc

[capfredf]

supersedes #80

[capfredf]

I built and tested main-distribution with these changes. No errors or failures related to rackunit have been found.

[samth]

John, is there a reason you merged this now? Did you review the patch?

[capfredf]

@jbclements this pr was not ready to be merged.

[jbclements]

Forgive me, I'm clearly misunderstanding the review process that we're using. My interpretation of "ready for review" was "LGTM, ready to merge". If there's a difference in github's workflow between "ready for review" and "ready to merge", I wasn't aware of it.

[jbclements]

I guess I should add that I did skim the changes, but definitely wouldn't consider myself qualified to review them without many hours of background work. I'm happy to revert this change if required. I'd certainly be even happier to have it go in, though.

[sorawee]

IIUC, in GitHub, you can either submit a PR regularly or submit a PR as a draft, where the latter means that no one is expected to look at it yet.

The process where one converts a draft PR to a regular PR is indicated by the system with "marked this pull request as ready for review"

[jbclements]

Well, that suggests to me that it's not the case that I misunderstood the categories associated with GitHub's PR workflow, so that's ... good, I guess? The problem was that (if i now understand correctly) this PR was not submitted as "LGTM, let's merge this" but rather "I think this looks good, but I would appreciate some inspection from someone." Does that sound right?

[samth]

Right, in general I think you should only merge a PR if you have either reviewed the change in detail or talked to the submitter and other maintainers of the relevant component about it.

[samth]

For this particular change, I think we should wait to see if the package build turns up any failures.

[bestlem]

Did this get into Racket v8.4 release? My test from https://github.com/racket/typed-racket/issues/1177 fails on macOS Arm 8.4

Welcome to DrRacket, version 8.4 [cs]. Language: typed/racket/base, with debugging; memory limit: 128 MB.

. ../../../../../Applications/Racket v8.4/share/pkgs/rackunit-typed/rackunit/main.rkt:24:2: FAILURE name: check-equal? location: /Applications/Racket v8.4/share/pkgs/rackunit-typed/rackunit/main.rkt:24:2 actual: 2 expected: 3

[samth]

No, this will be in the 8.5 release.

[capfredf]

@bestlem for the time being, you can try it out with a snapshot build

[jbclements]

Ha! Okay, well, okay, I found a problem with this PR. It's kind of simultaneously my fault for merging it without review, and also vindication because the bug was in fact caught long before the release.

Here's the actual issue: this file

#lang typed/racket/base

(module+ test
  (check-equal? 1234 1234)
  (require typed/rackunit))

now fails, with this error:

ouch.rkt:4:2: Type Checker: No function domains matched in function application:
Domains: Any Any String 
         Any Any 
Arguments: (Listof Any)

  in: (check-equal? 1234 1234)
  location...:
   ouch.rkt:4:2
  context...:
   /Users/clements/racket/racket/share/pkgs/typed-racket-lib/typed-racket/typecheck/tc-toplevel.rkt:414:0: type-check
   /Users/clements/racket/racket/share/pkgs/typed-racket-lib/typed-racket/typecheck/tc-toplevel.rkt:653:0: tc-module
   /Users/clements/racket/racket/share/pkgs/typed-racket-lib/typed-racket/tc-setup.rkt:101:12
   /Users/clements/racket/racket/share/pkgs/typed-racket-lib/typed-racket/typed-racket.rkt:22:4

reverting to the commit prior to this PR fixes this bug. So there is in fact an issue here.

This might be a good reason to revert the whole PR, or to repair this issue.

Opinions?

[capfredf]

This issue is weird. If I move the require expression before the check-equal? or move it outside, the program will type-check. I will repair this problem.

@jbclements can you file a separate issue?

[samth]

The problem is using check-equal? as a function instead of a macro. Try (#%app check-equal? 1 2).

[jbclements]

Okay, filing this as a separate issue.