Skip to content
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

remove filename from line number nodes #15000

Closed
wants to merge 1 commit into from
Closed

Conversation

JeffBezanson
Copy link
Member

@carnaval is this what you want?

currently breaks the backtrace test

[ci skip]
@@ -921,6 +921,8 @@ JL_DLLEXPORT Type *julia_type_to_llvm(jl_value_t *jt, bool *isboxed)
return PointerType::get(lt, 0);
}
if (jl_is_bitstype(jt)) {
if (jt == (jl_value_t*)jl_long_type)
return T_size;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could probably also just generally generically cache these in jst->struct_decl

@toivoh
Copy link
Contributor

toivoh commented Feb 9, 2016

I use the filenames (and line numbers) in line number nodes in Debug.jl to keep track of source location. How would I do that going forward?

@JeffBezanson
Copy link
Member Author

File names are available in LambdaInfo, and also (in theory, hopefully soon to be in practice) via backtraces.

@toivoh
Copy link
Contributor

toivoh commented Feb 9, 2016

But LambdaInfo is for compiled methods, right? There will be no way for a macro to determine which source files went into creating the AST that was given to it?

@carnaval
Copy link
Contributor

carnaval commented Feb 9, 2016

Ah. That may be a problem. The frontend could insert a '(meta push_lambda ,linfo) around blocks that are passed to macros (like we are trying to do for inline functions) ?
or maybe a meta push/pop_file because we don't have the linfo yet.

Or we could still have those filenames in there as before, just seemed weird to me to have the same information duplicated everywhere.

@JeffBezanson
Copy link
Member Author

We could put (meta file x.jl) in surface ASTs I suppose. Macros are a difficult case though because they can expand to a mix of code from different locations.

@toivoh
Copy link
Contributor

toivoh commented Feb 9, 2016

Also, different lines in functions can come from different files, after macro expansion. Would that information still be representable?

@carnaval
Copy link
Contributor

carnaval commented Feb 9, 2016

yes I think you would need to have such a pair of node in every "toplevel" block, meaning the outer most block of a quote, including implicit quotes such as the argument to a macro. Feels a little better to me than having the file name everywhere but might not be worth the effort.

As an added benefit, those nodes could specify that the code is being quoted/macroed so we could display that in the backtrace maybe. Just thinking out loud.

@JeffBezanson
Copy link
Member Author

Filename push/pop per outer quote block sounds pretty reasonable to me.

@toivoh
Copy link
Contributor

toivoh commented Feb 9, 2016

Sure, that would be fine.

@JeffBezanson
Copy link
Member Author

Hmm, adding "pop" at the end of blocks is actually tricky, since the last expression in a block determines its value. We might be able to handle this later in lowering, e.g. inserting "pop" only when merging one block into another in a non-final position.

@tkelman tkelman deleted the jb/nofilenames branch March 17, 2016 05:19
@JeffBezanson JeffBezanson mentioned this pull request Mar 22, 2016
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants