Fix over-indexing of lists and restore string indexing #9388
Conversation
| @@ -1265,39 +1263,6 @@ public void TestDictionaryRegressMAGN619() | |||
| thisTest.Verify("r", 5); | |||
| } | |||
|
|
|||
| [Test, Category("Failure")] | |||
There was a problem hiding this comment.
This test doesn't make any sense.
| @@ -18,7 +18,7 @@ public enum WarningID | |||
| Default, | |||
| AccessViolation, | |||
| AmbiguousMethodDispatch, | |||
| AurgumentIsNotExpected, | |||
| ArgumentIsNotExpected, | |||
There was a problem hiding this comment.
@aparajit-pratap I don't think we can change these now, they are public. 😢 maybe just add a todo?
|
@aparajit-pratap looks good except I don't think we can fix the enum names now. |
|
@mjkkirschner these are changes in ProtoCore. I'm pretty sure no users/clients use any public stuff from ProtoCore (other than AST's). Or are you saying this will cause binary incompatibility issues? |
|
Binary incompatibility is what I’m worried about.
… On Jan 9, 2019, at 11:15 PM, aparajit-pratap ***@***.***> wrote:
@aparajit-pratap these are changes in ProtoCore. I'm pretty sure no users use any public stuff from ProtoCore. Or are you saying this will cause binary incompatibility issues?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
aparajit-pratap
changed the title
| @@ -1663,6 +1663,7 @@ public IEnumerable<string> ReturnKeys | |||
| public string ObsoleteMessage { get; protected set; } | |||
| public bool IsObsolete { get { return !string.IsNullOrEmpty(ObsoleteMessage); } } | |||
| public bool IsLacingDisabled { get; protected set; } | |||
| public bool AllowArrayPromotion { get; protected set; } = true; | |||
There was a problem hiding this comment.
@aparajit-pratap should this also be added to the imperative AST methodAttributes class - does that exist?
There was a problem hiding this comment.
Strictly speaking this class doesn't have anything to do with Associative AST's and should not be in this file but that's a consideration for another day.
|
@aparajit-pratap looks good to me - one question about imperative behavior vs associative? |
|
Thanks @mjkkirschner, merging. |
* fix overindexing of lists * add null checks * fixes * revert assemblysharedinfo * fix tests * reverting unchanged file * turn on previously failing tests that pass after fix * revert public to protected * revert unchanged file * fix warning messages
* fix overindexing of lists * add null checks * fixes * revert assemblysharedinfo * fix tests * reverting unchanged file * turn on previously failing tests that pass after fix * revert public to protected * revert unchanged file * fix warning messages
| using ProtoCore.Properties; | ||
| using ProtoCore.Exceptions; | ||
| using IndexOutOfRangeException = DesignScript.Builtin.IndexOutOfRangeException; | ||
| using KeyNotFoundException = DesignScript.Builtin.KeyNotFoundException; |
There was a problem hiding this comment.
Can you make sure the usings are sorted?
| @@ -1332,7 +1332,7 @@ void core_Dispose(ProtoCore.RuntimeCore sender) | |||
| { | |||
| CLRObjectMarshaler marshaller = null; | |||
| if (!mObjectMarshlers.TryGetValue(sender, out marshaller)) | |||
| throw new KeyNotFoundException(); | |||
| throw new System.Collections.Generic.KeyNotFoundException(); | |||
There was a problem hiding this comment.
hmm This file has System.Collections.Generic using already I believe
There was a problem hiding this comment.
this is to be explicit because we also have a key not found exception.
There was a problem hiding this comment.
@mjkkirschner Then should we rename our own exception in a different task?
There was a problem hiding this comment.
please a different task. ours is
DesignScript.Builtin.KeyNotFoundException
@aparajit-pratap is aliasing them above.
* Cherry-picking #9388 into 2.0.3 * Cherry-picking #9559 into 2.0.3 * Cherry-picking #9578 into 2.0.3 * cherry-picking #9632 into 2.0.3 * cherry-picking #9408 into 2.0.3 * cherry-picking #9441 into 2.0.3 * Adding gradient.png for the Test_PerforationsByImage test. This was missed while cherrypicking #9441 * Removing DSCoreDataTests.cs as this was the test fixture was introduced in a different commit and is not needed here.
Purpose
Fixes #9383
JIRA:
https://jira.autodesk.com/browse/DYN-995
https://jira.autodesk.com/browse/DYN-1056
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@mjkkirschner