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

Updated Native Models #1396

Merged
merged 19 commits into from
May 14, 2024
Merged

Updated Native Models #1396

merged 19 commits into from
May 14, 2024

Conversation

MadhuNimmo
Copy link
Contributor

Added stubs/models to support more native method calls.

Copy link

github-actions bot commented May 2, 2024

Test Results

  572 files  ±0    572 suites  ±0   2h 49m 27s ⏱️ - 11m 15s
  734 tests ±0    717 ✅ ±0  17 💤 ±0  0 ❌ ±0 
3 554 runs  ±0  3 467 ✅ ±0  87 💤 ±0  0 ❌ ±0 

Results for commit 130031c. ± Comparison against base commit ae27048.

♻️ This comment has been updated with latest results.

Copy link
Member

@msridhar msridhar left a comment

Choose a reason for hiding this comment

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

Looks great! I have a few questions

cast/js/src/main/resources/preamble.js Show resolved Hide resolved
cast/js/src/main/resources/preamble.js Show resolved Hide resolved
}
}

Console = function Console(){
Copy link
Member

Choose a reason for hiding this comment

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

Does this help with calls like console.log(...)? If so, how, given that the variable is named Console?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Sample Program:


function getRandomNumber(min, max) {
        return Math.floor(Math.random() * (max - min + 1)) + min;
}

var randomNumber = getRandomNumber(1, 100);

console.log("Random Number:", randomNumber);

if (randomNumber % 2 === 0) {
        console.debug("The random number is even.");
} else {
        console.info("The random number is odd.");
}
    

Output:

   "test.js@1:0-341": {
    "test.js@12:297-338": [
      "console_info (Native)"
    ],
    "test.js@5:125-148": [
      "test.js@1:0-104"
    ],
    "test.js@7:151-194": [
      "console_log (Native)",
      "Math_log (Native)"
    ],
    "test.js@10:235-278": [
      "console_debug (Native)"
    ]
  }

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, but can you explain why adding a write to the Console variable leads to new information for a variable named console? These are two different variables.

Comment on lines 153 to 154
//Function = function Function() {};

Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were to identify the constructor calls. But, some of the tests fail as of now, due to these constructor calls. I can remove them for now.

Copy link
Member

Choose a reason for hiding this comment

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

Remove the changes? Or remove the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove the changes

Copy link
Member

Choose a reason for hiding this comment

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

Were you planning to remove this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 595 to 596
//Number = function Number() {};

Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as Function.

Copy link
Member

Choose a reason for hiding this comment

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

Same question as above

Copy link
Member

@msridhar msridhar left a comment

Choose a reason for hiding this comment

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

Looks great! Couple more very minor things and then this will be ready to land

@@ -502,7 +555,12 @@ String$proto$__WALA__ = {

replace: function String_prototype_replace(regex, withStr) {
// return new String(primitive("StringReplace", this, regex, withStr));
return this || withStr;
//return this || withStr;
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this commented line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


local_promise.prototype = Promise$proto$__WALA__;
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a newline at the end of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@msridhar msridhar merged commit e39d2d6 into wala:master May 14, 2024
11 checks passed
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