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

Stop Storing Random UUIDs as Stand-In Identifiers in the Slice Compiler #2721

Open
InsertCreativityHere opened this issue Sep 4, 2024 · 0 comments
Assignees
Milestone

Comments

@InsertCreativityHere
Copy link
Member

InsertCreativityHere commented Sep 4, 2024

  1. In certain cases, the Slice parser will gracefully handle missing identifiers (something like struct { ... }).
    When it encounters this, the parser will call Ice::generateUUID to generate a random identifier for this struct.

  2. Other times, the Slice compiler will decide that an existing identifier is 'bad'. Also in these cases, it will replace the user-supplied identifier with a random UUID.

I think that instead of generating and storing random UUIDs for these cases,
we should just store the empty string, and use this as a sentinel value.

A) This is more true to the intent of a forgotten identifier, or one that we want to ignore because it's 'bad'.
B) This is easier than generating UUIDs, and reduces the chance of collision from near-zero to zero.
C) It's detectable. Validation functions could check if the identifier is empty to determine whether they're worth running or not.
Whereas right now, there's no way to tell if an identifier is a UUID or user supplied, leading to bugs like #2662 where we leak UUIDs.


Q: Why not use optional<string> instead of a sentinel value?

A: The only times when this matters is when the user has made an error; either they forgot an identifier, or used a bogus one.
So I don't think it's worth complicating the API just to handle this.
createStruct(optional<string> name, ...) kind of implies that it's 'okay' to not supply a name for a struct, which is not true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant