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

Improved Map handling: Strings and integers. #413

Merged
merged 3 commits into from Jul 27, 2020

Conversation

lazaroclapp
Copy link
Collaborator

@lazaroclapp lazaroclapp commented Jul 27, 2020

This change improves our handling of map operations such as
containsKey, put, and get, by allowing keys that do
not have an access path directly associated with them.

In particular, it allows literal strings to be understood
as Map keys by NullAway for the purpose of matching
checked and accessed map-paths (fixing #398). It also
handles boxed integers and long integers, both as literals
and as primitive variables (fixing #410).

This change improves our handling of map operations such as
`containsKey`, `put`, and `get`, by allowing keys that do
not have an access path directly associated with them.

In particular, it allows literal strings to be understood
as `Map` keys by NullAway for the purpose of matching
checked and accessed map-paths (fixing #398). It also
handles boxed integers and long integers, both as literals
and as primitive variables (fixing #410).
// No MapKey object can be constructed here
return null;
}
return new AccessPathMapKey(accessPath);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think it would be worthwhile to just have AccessPath implement MapKey instead of introducing this AccessPathMapKey wrapper? I don't know what the common case will be here or what the perf impact would be, but that would save one level of indirection for AccessPaths.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good to me. It reduces runtime indirection and code size :)

package com.uber.nullaway.dataflow;

/** Marker interface to implement a union type of classes that can be used as AccessPath.mapKey */
interface MapKey {}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This needs to be outside AccessPath.java now to avoid circular definition issues.

Copy link
Collaborator

@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.

LGTM

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.

None yet

2 participants