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

Enable leak checks #403

Merged
merged 13 commits into from
Apr 28, 2016
Merged

Enable leak checks #403

merged 13 commits into from
Apr 28, 2016

Conversation

kripken
Copy link
Member

@kripken kripken commented Apr 27, 2016

Let's see where things stand now.

@@ -63,7 +63,8 @@ ELSE()
ADD_COMPILE_FLAG("-O0")
ADD_COMPILE_FLAG("-g3")
Copy link
Member

Choose a reason for hiding this comment

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

Hoist out this -g3 instead of duplicating below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, this is just throwaway code for testing on travis. Just makes the stack traces from leaks nicer to read. I'll remove when this PR is done and the leaks are gone.

@kripken kripken force-pushed the leaks branch 5 times, most recently from 73807b3 to c8d4fbe Compare April 27, 2016 20:11
@kripken kripken self-assigned this Apr 27, 2016
@kripken kripken force-pushed the leaks branch 4 times, most recently from ea38248 to 3ba2501 Compare April 27, 2016 22:58
@kripken
Copy link
Member Author

kripken commented Apr 28, 2016

Ok, this should be good to go. I rebased now and removed the debug help stuff, and all leaks are gone.

A bunch of code changed here, but aside from the .travis.yaml change to enable leak checks, it's nothing interesting, just moving to rely on std::unique_ptr etc. to manage memory in more places, and removing some hacks.

std::map<IString, std::vector<CallImport*>> importedFunctionCalls;

void noteImportedFunctionCall(Ref ast, WasmType resultType, AsmData *asmData, CallImport* call) {
assert(ast[0] == CALL && ast[1][0] == NAME);
IString importName = ast[1][1]->getIString();
auto* type = allocator.alloc<FunctionType>();
std::unique_ptr<FunctionType> type = make_unique<FunctionType>();
Copy link
Member

Choose a reason for hiding this comment

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

auto

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed.

@kripken kripken merged commit 6a6bdc1 into master Apr 28, 2016
@kripken kripken deleted the leaks branch April 28, 2016 02:25
@kripken
Copy link
Member Author

kripken commented Apr 28, 2016

Merging as this touches so much code, it's a merge collision hazard. Let's do additional comments as followups. @jfbastien: I already addressed the first of your comments, and had some questions about the others.

@dschuff: about merge vs squash commits, this pr is an example of where i think a merge commit is better. It's lots of tiny separate commits, not worth their own pr, and it's nice to keep them separate for future bisection and readability purposes. Also the merge commit shows the structure (that those commits led up to the commit enabling leak tests now that they pass).

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.

2 participants