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.

Sign up for GitHub

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

Jump to bottom

remove deserialize mode MODULE_POSTWORK #18534

Merged
merged 1 commit into from
Sep 19, 2016
Merged

remove deserialize mode MODULE_POSTWORK #18534

merged 1 commit into from
Sep 19, 2016

Conversation

vtjnash
Copy link
Sponsor Member

@vtjnash vtjnash commented Sep 15, 2016

it seems to be unnecessary

fix #18343

@vtjnash
remove deserialize mode MODULE_POSTWORK
f0e6dc1 
it seems to be unnecessary

fix #18343
@vtjnash vtjnash added backport pending 0.5 kind:bugfix This change fixes an existing bug labels Sep 15, 2016
@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Sep 15, 2016

CI failure was unrelated:
From worker 3: * pkg Exception running test pkg :
On worker 3:
LoadError: GitError(Code:ERROR, Class:OS, Failed to receive response: The server returned an invalid or unrecognized response
)

tkelman
tkelman reviewed Sep 16, 2016
View reviewed changes
src/dump.c
jl_tupletype_t *simpletype;
} def[100];
size_t count;
} linkedlist_t;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this file should just be c++ then?

tkelman
tkelman reviewed Sep 16, 2016
View reviewed changes
src/dump.c
@@ -1714,15 +1686,16 @@ static jl_value_t *jl_deserialize_value_(jl_serializer_state *s, jl_value_t *vta
else if (vtag == (jl_value_t*)jl_datatype_type || vtag == (jl_value_t*)SmallDataType_tag) {
int32_t sz = (vtag == (jl_value_t*)SmallDataType_tag ? read_uint8(s->s) : read_int32(s->s));
jl_value_t *v = jl_gc_alloc(ptls, sz, NULL);
jl_set_typeof(v, (void*)(intptr_t)0x40);
Copy link
Contributor

Choose a reason for hiding this comment

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

does 0x40 have a define'd name?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes or no?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

no, it's just a random garbage value (0x4, since it's the 4th one added here), like setting it to NULL, but more likely to fail

Copy link
Contributor

Choose a reason for hiding this comment

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

that should be a comment

Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment here

@tkelman
Copy link
Contributor

tkelman commented Sep 16, 2016 •
edited
Loading

that pkg error is #16555

what was MODE_MODULE_POSTWORK supposed to be for then?

@vtjnash vtjnash mentioned this pull request Sep 19, 2016
Closed
@vtjnash
Copy link
Sponsor Member Author

vtjnash commented Sep 19, 2016

trying to be more efficient (another of my recent patches mostly resolved that though), and to avoid writing the linked list implementation

@vtjnash vtjnash merged commit a6a8946 into master Sep 19, 2016
@vtjnash vtjnash deleted the jn/no-postwork branch September 19, 2016 19:25
@yuyichao yuyichao mentioned this pull request Sep 23, 2016
Closed
@tkelman
Copy link
Contributor

tkelman commented Oct 11, 2016

being backported in #18869

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkelman tkelman tkelman left review comments

Assignees
No one assigned
Labels
kind:bugfix This change fixes an existing bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crash on master with CategoricalArrays
2 participants
@vtjnash @tkelman