workaround wrong paths reported for stdlib functions #32763

KristofferC · 2019-08-02T00:25:24Z

Not really clean to special case it just ofr less and edit but that is where people seems to encounter this issue the most. The workaround could potentially be put in find_source_file but that is a bit more "invasive".

This is also kinda hard to test. I tested it by evaluating this into InteractveUtils in a released julia.

Fixes #26314

fredrikekre · 2019-08-04T10:25:53Z

Maybe add this for @which too?

cossio · 2019-10-14T13:44:16Z

Something holding this from being merged? Would be nice to have this issue fixed.

KristofferC · 2019-10-15T06:58:13Z

Something holding this from being merged? Would be nice to have this issue fixed.

I improved the implementation a bit but should be good to go now.

Tried it locally with the executable built by the buildbot and @which, @edit and @less work well with stdlib functions now.

vtjnash · 2019-10-15T15:27:17Z

Yeah, going with something, anything, is better than being always broken.

vtjnash · 2019-10-15T15:41:25Z

base/methodshow.jl

+# This function does the method location updating
+function updated_methodloc(m)
+    file, line = invokelatest(methodloc_callback[], m)
+    if file !== nothing && isdefined(@__MODULE__, :Sys)


Suggested change

if file !== nothing && isdefined(@__MODULE__, :Sys)

if file !== nothing && Sys.BUILD_STDLIB_PATH != Sys.STDLIB

Is Sys ever not defined?

And should we put this inside default_methodloc, or do we want to do this unconditionally?

It's not defined when this function gets defined and I was worried if there was some error before Sys gets defined then it would crash when trying to print some method.

I think we want to do this unconditionally.

vtjnash · 2019-10-15T15:42:39Z

base/methodshow.jl

            print(io, "[$(n)] ")
            show(io, meth; kwtype=kwtype)
            file, line = meth.file, meth.line
            try


move these try; catch inside updated_methodloc also

vtjnash · 2019-10-15T15:43:28Z

base/methodshow.jl

+        # BUILD_STDLIB_PATH gets defined in sysinfo.jl
+        file = replace(String(file), normpath(Sys.BUILD_STDLIB_PATH) => normpath(Sys.STDLIB))
+    end
+    return Symbol(file), line


Suggested change

return Symbol(file), line

return string(file), line

vtjnash · 2019-10-15T15:44:40Z

base/methodshow.jl


+# This function does the method location updating
+function updated_methodloc(m)
+    file, line = invokelatest(methodloc_callback[], m)


Suggested change

file, line = invokelatest(methodloc_callback[], m)

file, line = try

invokelatest(methodloc_callback[], m)

catch

default_methodloc(m)

end

file === nothing && return (string(m.file), m.line)

file = string(file)

vtjnash · 2019-10-15T15:45:18Z

base/methodshow.jl

 const methodloc_callback = Ref{Function}(default_methodloc)

+# This function does the method location updating
+function updated_methodloc(m)


Suggested change

function updated_methodloc(m)

function updated_methodloc(m)::Tuple{String, Int32}

rapus95 · 2019-10-16T09:20:38Z

Can this still make it into 1.3 or does the feature freeze prohibit it?

KristofferC · 2019-10-16T09:23:14Z

It will not make it into 1.3.0 for sure. Maybe 1.3.1.

rapus95 · 2019-10-16T09:40:36Z

Btw as this request is called workaround, I guess you'd not call it the idiomatic way.
Would it generally make sense to use the pathof(module) function to determine the path of the module where the function was defined and then resolve the path relative to that?

KristofferC · 2019-10-16T10:00:22Z

Btw as this request is called workaround, I guess you'd not call it the idiomatic way.

I dunno... If it works, it works:P

I did some fixups here @vtjnash

base/methodshow.jl

musm · 2019-10-16T17:15:10Z

Does Pkg\src\Operations.jl need an update as well?
I get the wrong paths as well when interacting with the Package Manager.

└ @ Pkg.Operations C:\cygwin\home\Administrator\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.3\Pkg\src\Operations.jl:494

KristofferC · 2019-10-16T17:21:51Z

I guess @info embeds the full path as well at macro expansion time. But that's for another PR.

rapus95 · 2019-10-23T11:09:24Z

Sad that it won't make it into 1.3 even though it is already merged 🙈

KristofferC · 2019-10-23T11:36:29Z

Could be put into 1.3.1 if it doesnt cause any problems on master.

carstenbauer · 2019-11-26T23:10:49Z

Could be put into 1.3.1 if it doesnt cause any problems on master.

I'm begging for it! This has been bugging me for way too long now. 😊

KristofferC · 2020-01-02T20:41:50Z

I have my script set to ignore backport labels that also has a triage label because we said we would triage backports at some point. This doesn't seem to happen though so the result is that PRs (like this) just get forgotten. I'll just backport stuff that includes the triage label as well.

timholy mentioned this pull request Aug 24, 2019

Stacktrace gripes #33065

Open

KristofferC force-pushed the kc/stdlib_path_less branch 2 times, most recently from 758a381 to 8bd5da3 Compare October 14, 2019 19:28

KristofferC requested a review from timholy October 14, 2019 19:29

KristofferC force-pushed the kc/stdlib_path_less branch 3 times, most recently from 2cda8ba to 748d69c Compare October 14, 2019 20:06

KristofferC changed the title ~~workaround wrong paths reported for stdlib functions in less and edit~~ workaround wrong paths reported for stdlib functions Oct 14, 2019

fredrikekre approved these changes Oct 15, 2019

View reviewed changes

vtjnash reviewed Oct 15, 2019

View reviewed changes

KristofferC added 2 commits October 16, 2019 11:47

workaround wrong paths reported for stdlib functions in less and edit

ab65312

improve stdlib path rewriting

4c98ae0

KristofferC force-pushed the kc/stdlib_path_less branch 2 times, most recently from 29dac94 to cf492f1 Compare October 16, 2019 10:00

KristofferC force-pushed the kc/stdlib_path_less branch from cf492f1 to 9e9b84c Compare October 16, 2019 11:06

rapus95 reviewed Oct 16, 2019

View reviewed changes

base/methodshow.jl Outdated Show resolved Hide resolved

KristofferC force-pushed the kc/stdlib_path_less branch from 9e9b84c to fffe94f Compare October 16, 2019 11:49

fixups

caa35be

KristofferC force-pushed the kc/stdlib_path_less branch from fffe94f to caa35be Compare October 16, 2019 14:23

KristofferC merged commit f9975d1 into master Oct 23, 2019

KristofferC deleted the kc/stdlib_path_less branch October 23, 2019 09:37

fredrikekre added backport 1.3 triage This should be discussed on a triage call labels Nov 26, 2019

KristofferC removed the triage This should be discussed on a triage call label Jan 2, 2020

fredrikekre mentioned this pull request Jan 8, 2020

[1.3.0] @edit edit(+, (Int, Int)) is a SystemError #34301

Closed

non-Jedi mentioned this pull request Mar 20, 2020

Path to stdlib files is incorrect in stacktraces #35191

Closed

	if file !== nothing && isdefined(@__MODULE__, :Sys)
	if file !== nothing && Sys.BUILD_STDLIB_PATH != Sys.STDLIB

-    file, line = invokelatest(methodloc_callback[], m)
+    file, line = try
+            invokelatest(methodloc_callback[], m)
+        catch
+            default_methodloc(m)
+        end
+    file === nothing && return (string(m.file), m.line)
+    file = string(file)

	function updated_methodloc(m)
	function updated_methodloc(m)::Tuple{String, Int32}

Uh oh!

Uh oh!

workaround wrong paths reported for stdlib functions #32763

workaround wrong paths reported for stdlib functions #32763

Uh oh!

Conversation

KristofferC commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fredrikekre commented Aug 4, 2019

Uh oh!

cossio commented Oct 14, 2019

Uh oh!

KristofferC commented Oct 15, 2019

Uh oh!

vtjnash commented Oct 15, 2019

Uh oh!

vtjnash Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

KristofferC Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

KristofferC Oct 16, 2019

Choose a reason for hiding this comment

Uh oh!

vtjnash Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

vtjnash Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

vtjnash Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

vtjnash Oct 15, 2019

Choose a reason for hiding this comment

Uh oh!

rapus95 commented Oct 16, 2019

Uh oh!

KristofferC commented Oct 16, 2019

Uh oh!

rapus95 commented Oct 16, 2019

Uh oh!

KristofferC commented Oct 16, 2019

Uh oh!

Uh oh!

musm commented Oct 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KristofferC commented Oct 16, 2019

Uh oh!

rapus95 commented Oct 23, 2019

Uh oh!

KristofferC commented Oct 23, 2019

Uh oh!

carstenbauer commented Nov 26, 2019

Uh oh!

KristofferC commented Jan 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

KristofferC commented Aug 2, 2019 •

edited

Loading

musm commented Oct 16, 2019 •

edited

Loading