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

Fix compiled locks #1561

Merged
merged 1 commit into from
Mar 30, 2016
Merged

Conversation

hvacengi
Copy link
Member

Fixes #1557
Fixes #1253
Fixes #691

The program builder now ignores duplicate label errors for default user functions (labels that end in "-default"). In this instance, an already existing default function exists, and that instruction pointer is used instead of the new function.

A side effect of the previous logic was also that creating a new lock in a new program would redefine the lock's instruction pointer to refer to the default value. That means that an already existing lock object would no longer be evaluated. To fix this, we check to see if the lock's pointer is already defined. If it is defined, we know that it either points to the existing default function or to the existing lock function. In either case, we can skip setting the pointer to the default function.

Features:

  • Don't throw exceptions for duplicate default function labels.
  • Don't overwrite the function pointer if it already exists.
  • Check documentation for references to this limitation and remove them if necessary.

Question:
Should we also not throw an exception if a label matches the function's hash? I believe that the hash now includes the file name, in which case the hashes will not match if two files simply define an identical lock, unless the hash of their file names is an exact match as well.

ProgramBuilder.cs
* Ignore duplicate labels if they are default values for user functions.
This prevents an error when running a compiled function that defines a
lock identifier already defined in the existing program.
Compiler.cs
* Add opcodes to the initialization logic that will only set the lock
pointer to the new default function if there is not already a pointer
defined.
@hvacengi hvacengi added this to the v0.19.3 milestone Mar 24, 2016
@hvacengi hvacengi added Pending Documentation Author is warning Reviewer not to merge this PR yet. User doc changes aren't in the PR yet. and removed Pending Documentation Author is warning Reviewer not to merge this PR yet. User doc changes aren't in the PR yet. labels Mar 24, 2016
@hvacengi
Copy link
Member Author

@Dunbaratu I just ran through the docs and cannot find any point where we referenced the failure of replicated lock identifiers in compiled scripts. As such, there are no doc changes needed for this PR and it should be ready.

@Dunbaratu Dunbaratu merged commit 17c8ecc into KSP-KOS:develop Mar 30, 2016
@hvacengi hvacengi deleted the issue-1557-compiled_locks branch April 30, 2016 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants