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

Minor evaluation issue regarding update operators #97

Open
exists-forall opened this issue May 2, 2013 · 6 comments
Open

Minor evaluation issue regarding update operators #97

exists-forall opened this issue May 2, 2013 · 6 comments

Comments

@exists-forall
Copy link

foo[bar] += baz

compiles to

foo[bar] = foo[bar] + baz

although it should be

do
    local _update_table_0 = foo
    local _update_index_0 = bar
    _update_table_0[_update_index_0] = _update_table_0[_update_index_0] + baz
end

My intuition tells me that, in addition to other "principle of least surprise advantages", this would improve efficiency for situations such as

multidimensional_array[expensive_row_calculation!][expensive_column_calculation!] += some_value
@fillest
Copy link

fillest commented May 2, 2013

Why?

@exists-forall
Copy link
Author

Because expensive_row_calculation! and expensive_column_calculation! would only have to be calculated once, not twice. Also, is the multidimensional_array is sparse, the hashing of expensive_row_calculation! would only have to be performed once to get the column array.

@exists-forall
Copy link
Author

Also, there's a convention within macro-like systems (of which moonscript is one) that expressions should be evaluated only as many times as they are written. For example, python's neat

a < b < c

syntax (which is basically a macro, albeit a language level one, in that it is a syntactic shortcut) is not exactly equivalent to

a < b and b < c

because b is only evaluated once in the first case.

@exists-forall
Copy link
Author

Again, this is very minor. It's a small detail, and it doesn't really matter if it's addressed. I just thought it would be worthwhile to bring it up because there are situations where it makes a noticeable difference which form it the update compiles to.

@fillest
Copy link

fillest commented May 3, 2013

Ah, I posted my question before you edited your post :) Yeah, for multidimensional it's handy. But for single- it can theoretically bring some overhead (copying numbers?), so better use it only for multidimensional

@leafo
Copy link
Owner

leafo commented May 3, 2013

Yeah this is definitely a bug. I'd like to fix this.

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

No branches or pull requests

3 participants