-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Literal symbols are not accepted as const map keys. #15445
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
Comments
I believe Symbol does override Object.operator==, so it's not a valid key in a const Map (per spec). |
The class is defined here: https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/sdk/lib/core/symbol.dart Clearly, it doesn't override ==. The rest is implementation details. |
"It is a compile-time error if the key of an entry in a constant map literal is an instance of a class that implements the operator == unless the key is a string or integer." The keys are not instances of dart.core.Symbol. They are instances of _collection.dev.Symbol, which does define ==. We should treat symbols like strings and include them in the exceptions. (Also, presumably the spec meant to restrict when a class's implementation of == is not the one inherited from Object. As it stands, if I interpret "implements" as "defines", then I could put == in a superclass to sidestep the restriction. If I interpret "implements" as "defines or inherits", then the restriction applies to all classes.) cc @gbracha. |
As I said: _collection.dev.Symbol is an implementation detail. |
As Lasse points out on misc@, the implementation details are leaking through. I've filed issue #15455 to update the library spec. |
It's not an implementation detail if We have not done so yet, or agreed that it must be done. The user can create arbitrarily many Symbol objects from strings, and when faced with a similar problem for Type, we decided to not canonicalize to avoid unlimited memory bloat from caching the canonicalized versions. We might be able to do it, using weak references for the cache to avoid keeping more instances alive than what the program can detect - but I don't know if dart2js has that option. |
As Ryan said, we should simply add symbols to the set of excluded types allowed as keys. I'll file a spec bug to his effect and inform TC52 of our intentions. I don't expect any issues. |
Gilad, I'm losing track of the negations :-) Are you suggesting that symbols can be used as keys in constant maps? If so, sounds good to me. |
Peter, yes, const symbols should be allowed as keys in maps and also in |
Allowing symbols in switch cases and const maps has still not made it into the specification for switch. It only special-cases int and String. Removed Area-VM label. |
Assigning to Gilad to double check that the spec is (or gets) updated to reflect our decision to allow Type objects in const maps and switches. Set owner to @gbracha. |
Confused by this last comment. This bug is about Symbols, not Type. The situation with Type is under review AFIK (see issue #21553). As far as this issue is concerned - if this is a language issue, I can close it as fixed right now. The question is whether it is implemented. Please clarify. |
Sorry, I seem to have messed this up. Please disregard my comment about Type. Is the spec updated for symbols? If so, we should just turn this back into a Area-VM issue. Not sure why Lasse changed it in the first place. |
The spec has been fixed and this is in the TC52 approved version awaiting ECMA ratification in December. Reclassifying as a VM bug. Removed the owner. |
The VM rejects the program reported in issue #15442 (which is an analyzer bug):
class A {}
class B {}
main() {
var map = const <Symbol, int>{#A:0, #B:1};
print(map);
}
The text was updated successfully, but these errors were encountered: