I wouldn’t say that this is an incompatibility of note because Lua 5.3, if memory serves (or maybe 5.2?), had the same behavior. We will note this optimization once it ships outside of Studio on the performance page. (it’s part of the release notes for the release today which is why it didn’t make it into the recap)
1 Like
From reading the 5.1 manual:
2.5.2 states Objects (tables, userdata, threads, and functions) are compared by reference: two objects are considered equal only if they are the same object. Every time you create a new object (a table, userdata, thread, or function), this new object is different from any previously existing object., and 2.5.9 states A function definition is an executable expression, whose value has type function. When Lua pre-compiles a chunk, all its function bodies are pre-compiled too. Then, whenever Lua executes the function definition, the function is instantiated (or closed). This function instance (or closure) is the final value of the expression. Different instances of the same function can refer to different external local variables and can have different environment tables..
These make it clear that every time a function expression is encountered, a new function instance is created and it compares unequal to other instances of the same function.
I also noticed that the incompatibilities mentions order of table assignment in a table constructor, when there is no guarantee of it happening in a specific order.
Sure, that’s fair - we can list it there as well. For us it’s conditional on fenv overrides so we can’t promise ref equality - it’s going to be equal “sometimes”.
A question from a beginner scripting learner, is this a replacement for LUA or something entirely different like an addition to it
Edit: after reading over the linked page, basically this is just an auto complete for lua? I’m still not 100% sure. If someone can clarify that would be great.
Since I understand some of the basics of roblox lua etc, I just want to make sure I can learn and apply it to this in case it actually is some new replacement in the future.
How ever if it’s just auto completion, that’s super helpful for someone like me
Not sure what you mean by LUA, but any Lua 5.1 code written before Luau is still valid, as the latter is intentionally backwards compatible with the former, so you could still use Luau even without using the new syntax.
2 Likes
This might not be as clean a change as you’re expecting.
For instance, even this recent post of mine contains code that relies on the fact function() end ~= function() end (in the “RoboxSignal” implementation) to generate unique tokens that can be passed through a BindableEvent without being serialized: Lua Signal Class Comparison & Optimal `GoodSignal` Class.
I know I’ve written code in the past that also makes that uses that approach, though I don’t think any of it is still important / in libraries of mine people might be using.
1 Like
Sure, but I maintain that having definitions and implementations be linked only by a linter and the good will of developers is flimsy. Marrying the definition and implementation seemed like a good way to link them. Conceptually, the definition is allowed to be wrong and neglected while the implementation can’t because if a developer neglects it… they can’t be working on that type’s implementation. That is what I consider to be the issue, and in practice it did prove to be enough of an issue that we don’t write code based on type definitions.
For what it’s worth, we already don’t have many problems managing (relative) type safety with just the annotations. It’s very rare for us to have random type related bugs. But I’m glad to hear there are alternatives coming down the road!
2 Likes
Another issue arises if setfenv is brought into the scenario (backport _ENV please 
)
local t = {}
for i = 1, 2 do table.insert(t, function() print("Hello, World!") end) end
setfenv(t[1], {})
t[2]()
This should be mentioned as the latest update notes say unless setfenv is used, but that doesn’t seem to be the case here. Are the release notes wrong or is this a different issue?
1 Like
The release notes don’t say that if setfenv is used after the functions are created it’s too late. Try using setfenv (1) before creating each function and you will see unique instances with unique tables.
In our internal tests this was sufficiently powerful to not break interesting examples of sandboxing so that’s the compromise for now - the only way to know if it’s safe is to try.
We also have a more heavy handed de optimization for this that we aren’t enabling yet - we’ll see if we observe issues in practice with setfenv interaction
3 Likes
We’re very aware that this is a complex optimization, especially due to interactions with setfenv, which is why we will run it in Studio only for a while. Like any optimization that elides allocations it’s important enough to try.
Any change results in observable behavior under Hyrum’s law; some are fine in practice, some aren’t. We’ll see!
This probably isn’t as big of an issue as it’s only present in this edge case which I’m pretty confident isn’t used commonly by people (only noticed in obfuscation). A bigger issue that was mentioned by someone else is storing similar functions as keys in tables and accessing them later. Is there something that can be added to disable this optimization for specific functions that won’t de optimize the entire script?
If there’s a wide spread use of using functions as tokens (it’s funny that @stravant’s example literally says “Abuse the fact that function refs can be passed through BindableEvents intact” 
), I think we’ll need to simply disable the optimization. It will be unfortunate. I don’t think there’s a great reason to rely on this behavior, especially if you include interactions with our reflection system, but if it breaks code that’s in widespread use there’s not going to be a clean way for us to unbreak that code without just giving up on the idea.
1 Like
Is there not some alternative to getfenv/setfenv that can be created that makes an entirely new environment for the function? Sort of like setfenv(f, getfenv()), but instead of a call to getfenv that triggers deoptimization it sets it as a cloned default environment of some sort? I don’t know if I’m explaining it well enough but hopefully you can partially understand what I’m asking for.
What I mean is that I’m actually expecting that the setfenv-based code will work just fine with the new optimizations, because while you can construct examples that break it’s going to be very artificial. The problematic examples are likely to exclude setfenv entirely, like the code @stravant linked. That code isn’t relying on the presence of function environments, and indeed it would have been broken in Lua 5.3 (and fixed in Lua 5.4) despite the fact that Lua 5.2 removed setfenv.
We wouldn’t add a whole new primitive for such a small corner case - if we don’t break code in practice, then it’s already okay; if we do, we already need to disable this optimization, there’s not a whole lot of in-between.
1 Like
FWIW I think that the change is worth attempting, I was just raising a potential pattern that could be problematic with it.
2 Likes
I understand now. I honestly don’t think it should be too big of an issue and if it really makes a reasonable impact to optimizations maybe its better off making it intended functionality. Though it should definitely be listed as an incompatibility in it’s current state.
Yeah I agree, when we were planning this optimization the mental model was “setfenv breaks this but we don’t like setfenv so we will try anyway and it’s not a huge deal either way”. The examples we’re discussing definitely convinced me that we need to document this as an incompatibility (and we will note that it actually mirrors Lua 5.3 behavior somewhat), if the optimization actually stays enabled Stay tuned for the next recap where we will find out!
Yeah, understood. FWIW the future introductions of new ways to store objects will resolve this, I think, although I still wouldn’t recommend to write type-safe code without a type checker in the loop but that problem hopefully we’ll get to solving earlier.
Ignoring the issue that stravant pointed out, do you ever think there will be a time where you decide to remove getfenv/setfenv entirely in favor of backporting _ENV? It creates a huge backwards compatibility issue but seems to be a huge brick wall preventing a lot of good optimizations.
1 Like
Oh believe me we would LOVE to. setfenv is a cool concept but it’s been nothing but trouble so far :-/ Unfortunately we did a survey of the various uses of getfenv/setfenv and there’s no clear path for us to remove these without, well, breaking a bunch of games and upsetting a bunch of developers, which would be unfortunate – this is even if we added _ENV support. For example many developers [used to] use module systems where your first line in your script is
_G.InitModules()
And then you can proceed to use a bunch of globals injected through setfenv. This is not something that _ENV can replicate, so these developers would have to migrate to require which not everyone is happy to do.
So maybe this is possible but it would be a big effort, would break a bunch of code and honestly since we did more or less find a way to have our cake and eat it too, we don’t want to spend time on this right now.
6 Likes