Skip to content

Conversation

@vtjnash
Copy link
Member

@vtjnash vtjnash commented Aug 3, 2017

ensures the return type of literal_pointer_val is always just a simple pointer, and always of the same type (T_pjlvalue)

ensures the return type of literal_pointer_val is always just a simple pointer,
and always of the same type
@vtjnash vtjnash requested a review from Keno August 3, 2017 15:50
@vtjnash vtjnash added compiler:codegen Generation of LLVM IR and native code bugfix This change fixes an existing bug labels Aug 3, 2017
jl_errorf("macro definition not allowed inside a local scope");
name = literal_pointer_val(ctx, mn);
bnd = jl_get_binding_for_method_def(mod, (jl_sym_t*)mn);
JL_TRY {
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated change?

Copy link
Member Author

Choose a reason for hiding this comment

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

julia> begin
         for i = 1:1 end
         macro g() end
       end
ERROR: error compiling anonymous: macro definition not allowed inside a local scope

vs.

julia> begin
         for i = 1:1 end
         macro g() end
       end
ERROR: macro definition not allowed inside a local scope
Stacktrace:
 [1] macro expansion at ./REPL[1]:3 [inlined]
 [2] anonymous at ./<missing>:?

Both results are wrong, but at least it now usually won't corrupt the C++ stack.

Copy link
Member

@Keno Keno left a comment

Choose a reason for hiding this comment

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

Looks semantically valid to me. If this fixes an identified bug, would be good to have a test to that extent.

@tkelman
Copy link
Contributor

tkelman commented Aug 3, 2017

did this get put into #22984 accidentally? agreed on a test if possible, how'd you find this or what is it fixing?

@vtjnash
Copy link
Member Author

vtjnash commented Aug 3, 2017

It wasn't accidental - the case that triggered my review and fixes here were from writing tests for that PR.

@vtjnash vtjnash merged commit 1d7e8b2 into master Aug 4, 2017
@vtjnash vtjnash deleted the jn/codegen-bugs branch August 4, 2017 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This change fixes an existing bug compiler:codegen Generation of LLVM IR and native code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants