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

add warning when incremental compilation will probably fail #12400

Merged
merged 1 commit into from
Aug 3, 2015

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Jul 31, 2015

this makes incremental compilation noisy when it is pretty sure your code won't work with incremental compilation (due to use of eval(othermodule, code)

so far, I have not seen any false-positives with this, but it was able to find two real errors (with Images/Colors and GeometryTypes/FixedSizeArrays)

jl_lambda_info_t *li = (jl_lambda_info_t*)jl_expand_in(m->func->linfo->module, (jl_value_t*)ex);
assert(jl_is_lambda_info(li));
li->name = m->func->linfo->name;
func = jl_new_closure(NULL, (jl_value_t*)jl_emptysvec, li);
Copy link
Member Author

Choose a reason for hiding this comment

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

@JeffBezanson i think this is what you were suggesting for replacing the eval? what does the second argument to jl_new_closure mean in this case (i copied this from interpreter.c)?

@tkelman
Copy link
Contributor

tkelman commented Jul 31, 2015

eval(othermodule, code)

Down with monkey-patching! :pitchfork:

@vtjnash vtjnash force-pushed the jn/static_compile_warn_eval branch from 48a45fc to 2149008 Compare July 31, 2015 06:56
@timholy
Copy link
Member

timholy commented Jul 31, 2015

The Images/Color issue is already fixed: JuliaAttic/Color.jl#100 and JuliaImages/Images.jl#321 (both tagged).

But this is very good, thanks!

…ntal compilation (close #12352)

calling eval into another module during compile-time is almost guaranteed to break incremental compilation

the usual symptom would be a segfault during deserialization due to the
existance of a type, module, or other data structure in the 'other'
module that will not be persisted.
it is also possible to 'lose' method definitions because their
lambda_info->module field will be assigned incorrectly
@vtjnash vtjnash force-pushed the jn/static_compile_warn_eval branch from 2149008 to 8a1d443 Compare July 31, 2015 20:12
vtjnash added a commit that referenced this pull request Aug 3, 2015
add warning when incremental compilation will probably fail
@vtjnash vtjnash merged commit ddbcd73 into master Aug 3, 2015
@vtjnash vtjnash deleted the jn/static_compile_warn_eval branch August 3, 2015 17:16
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