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

asm2wasm debuginfo #895

Merged
merged 7 commits into from
Feb 7, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/asm2wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,13 @@ struct AstStackHelper {

std::vector<Ref> AstStackHelper::astStack;

static bool startsWith(const char* string, const char *prefix) {
while (1) {
if (*prefix == 0) return true;
if (*string++ != *prefix++) return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should also probably return false if *string == 0 for completeness, e.g. startsWith("foo", "foobar")

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, fixing.

}
};

//
// Asm2WasmPreProcessor - does some initial parsing/processing
// of asm.js code.
Expand Down Expand Up @@ -238,18 +245,11 @@ struct Asm2WasmPreProcessor {
std::string DEBUGINFO_INTRINSIC = EMSCRIPTEN_DEBUGINFO.str;
auto DEBUGINFO_INTRINSIC_SIZE = DEBUGINFO_INTRINSIC.size();
bool seenUseAsm = false;
auto startsWith = [&](const char *s) {
auto* check = input;
while (1) {
if (*s == 0) return true;
if (*check++ != *s++) return false;
}
};
while (input[0]) {
if (out + ADD_FACTOR >= end) {
Fatal() << "error in handling debug info";
}
if (startsWith("//@line")) {
if (startsWith(input, "//@line")) {
char* linePos = input + 8;
char* lineEnd = strchr(input + 8, ' ');
char* filePos = strchr(lineEnd, '"') + 1;
Expand All @@ -276,7 +276,7 @@ struct Asm2WasmPreProcessor {
out += line.size();
*out++ = ')';
*out++ = ';';
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing to do here just a thought/comment/observation:
This interleaving section of strcpy and *out++ = char is pretty verbose and I wish there was a cleaner/more compact way to do it.
Something like outStream << DEBUGINFO_INTRINSIC << '(' << fileIndex << ',' << line << ");", though that would require a very different model of how this is done and probably isn't worth it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, might be a better way. The code is not expected to change much if at all, though, so I think it's maintainable as it is. If it were evolving code definitely it would need to be refactored.

} else if (!seenUseAsm && (startsWith("asm'") || startsWith("asm\""))) {
} else if (!seenUseAsm && (startsWith(input, "asm'") || startsWith(input, "asm\""))) {
// end of "use asm" or "almost asm"
seenUseAsm = true;
memcpy(out, input, 5);
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth pulling the 5s into a constant?
Actually yeah it's not obvious why 5 and not 4, presumably consuming asm\'\n?

Copy link
Member Author

Choose a reason for hiding this comment

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

Heh, it's actually the semicolon at the end, "use asm";. I'll clarify.

Expand Down