Skip to content

fix and optimize erroneous code paths#58

Merged
c42f merged 2 commits intoJuliaLang:mainfrom
aviatesk:avi/jet
Aug 23, 2022
Merged

fix and optimize erroneous code paths#58
c42f merged 2 commits intoJuliaLang:mainfrom
aviatesk:avi/jet

Conversation

@aviatesk
Copy link
Copy Markdown
Member

No description provided.

Comment thread src/syntax_tree.jl Outdated
Co-authored-by: Sebastian Pfitzner <pfitzseb@gmail.com>
Copy link
Copy Markdown
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

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

This looks great, thanks. Feel free to merge it!

Comment thread src/syntax_tree.jl
else
print(io, node.val isa Symbol ? string(node.val) : repr(node.val))
val = node.val
print(io, val isa Symbol ? string(val) : repr(val))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm curious - is this stylistic or does it do something more?

Ah wait I think I see - this allows inference to prove that val::Symbol inside the first branch.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah currently inference can propagate conditional type information about local variable but not about memory owned by local variable. So we need to extract the field value into a local variable to get that extra type propagation.

Comment thread src/value_parsing.jl
str, sizeof(str), buffer, nwords, options,
@cfunction(utf8proc_custom_func, UInt32, (UInt32, Ptr{Cvoid})), C_NULL)
ret < 0 && utf8proc_error(ret)
ret < 0 && Base.Unicode.utf8proc_error(ret)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oops. Thanks!

Comment thread src/diagnostics.jl
message = !isnothing(error) ? error :
!isnothing(warning) ? warning :
error("No message in diagnostic")
Base.error("No message in diagnostic")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops! Thanks for the fix.

@aviatesk
Copy link
Copy Markdown
Member Author

Feel free to merge it!

Thanks! I don't have a write access to this repository so please merge this👍

@c42f
Copy link
Copy Markdown
Member

c42f commented Aug 23, 2022

Oh, I thought you'd have access as a member of the JuliaLang/committers group. I'd add you to this repo directly but I seem to have lost admin when I transferred this repo (requesting access again now...)

For now I'll just merge this. Thanks @pfitzseb for the review :)

@c42f c42f merged commit 7cfb056 into JuliaLang:main Aug 23, 2022
@aviatesk aviatesk deleted the avi/jet branch August 23, 2022 05:24
c42f pushed a commit to JuliaLang/julia that referenced this pull request Oct 17, 2025
Co-authored-by: Sebastian Pfitzner <pfitzseb@gmail.com>
topolarity pushed a commit to JuliaLang/julia that referenced this pull request Nov 14, 2025
Co-authored-by: Sebastian Pfitzner <pfitzseb@gmail.com>
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.

3 participants