Skip to content

Patch Khalid's infinite-loop2 + translations#74

Draft
dmadisetti wants to merge 2 commits intomainfrom
infinite-loop2
Draft

Patch Khalid's infinite-loop2 + translations#74
dmadisetti wants to merge 2 commits intomainfrom
infinite-loop2

Conversation

@dmadisetti
Copy link
Contributor

Cleaned version of #73

@dmadisetti dmadisetti requested a review from kmsalah August 7, 2022 17:15
@dmadisetti dmadisetti marked this pull request as draft August 7, 2022 17:15
@dmadisetti
Copy link
Contributor Author

For your reference. I think this still needs work though. Once you understand what it's doing, I think it makes sense to rewrite it completely.

I converted the comments over from Chinese, some hopefully that gives a bit more context, and added some comments of my own

I used this game to verify it works: #UucsDSEXrhzETmYu-4

Copy link
Contributor Author

@dmadisetti dmadisetti left a comment

Choose a reason for hiding this comment

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

@kmsalah Thoughts? Hope this helped

//
// this is not a strong regex, but enough to use at the time
let response = codeStr.replace(/for *\(.*\{|while *\(.*\{|do *\{/g, function(loopHead) {
var id = parseInt(Math.random() * Number.MAX_SAFE_INTEGER)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kmsalah
Opposed to random number here, maybe we use the line number and character offset. Need to be careful since we could feasible have multiple loops on one line.

But this would let us create more meaningful errors as well opposed to "Loop running too long"

// and makes sure that the loop doesn't exceed some value of execution time.
//
// this is not a strong regex, but enough to use at the time
let response = codeStr.replace(/for *\(.*\{|while *\(.*\{|do *\{/g, function(loopHead) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol, this regex is such a hack- but works well enough I guess.

@@ -0,0 +1,49 @@
// TODO: Clean up. Not really wriotten for modules and a bit of a hack as is.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a bit of a speil. I think to make this play better with modules, it should be rewritten

return response;
}
infiniteLoopDetector.unwrap = function(codeStr) {
return codeStr.replace(/infiniteLoopDetector\([0-9]*?\);/g, '')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops. Bug. This should be infDectector

But also, maybe this function isn't needed? When are we ever going to call unwrap

// this is not a strong regex, but enough to use at the time
let response = codeStr.replace(/for *\(.*\{|while *\(.*\{|do *\{/g, function(loopHead) {
var id = parseInt(Math.random() * Number.MAX_SAFE_INTEGER)
return `infDetector(${id});${loopHead}infDetector(${id});`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

infDetector matches the var in frame.js. But obviously this is very brittle as is.


function infiniteLoopDetector(id) {
if (id in map) { // Not the first execution, it can be optimized here, the performance is too low
if (Date.now() - map[id] > 1000) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hardcoded 1 second? Maybe we can make this adjustable? Or do you think 1 second is fine?

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.

1 participant