fix sgen if/else issue #25562
fix sgen if/else issue #25562
Conversation
| @@ -2197,6 +2200,8 @@ private void WriteMemberElementsIf(Member[] members, Member anyElement, string e | |||
| Writer.WriteLine("switch (state) {"); | |||
| } | |||
| int cases = 0; | |||
| bool largeNumberOfMembers = (members.Length > 1000); | |||
There was a problem hiding this comment.
Why is the number set to 1000?
Besides, I think it'd be better to extract the value into a readonly variable.
There was a problem hiding this comment.
Unless there are perf concerns, I would not bother to generate different code.
| @@ -2197,6 +2200,8 @@ private void WriteMemberElementsIf(Member[] members, Member anyElement, string e | |||
| Writer.WriteLine("switch (state) {"); | |||
| } | |||
| int cases = 0; | |||
| bool largeNumberOfMembers = (members.Length > 1000); | |||
| bool useDoWhile = largeNumberOfMembers && !isSequence; | |||
There was a problem hiding this comment.
We might want to add comments here explaining why we want to use do...while.
| @@ -2217,7 +2220,8 @@ private void WriteMemberElementsIf(Member[] members, Member anyElement, string e | |||
| ElementAccessor e = elements[j]; | |||
| string ns = e.Form == XmlSchemaForm.Qualified ? e.Namespace : ""; | |||
| if (!isSequence && e.Any && (e.Name == null || e.Name.Length == 0)) continue; | |||
| if (!firstElement || (!isSequence && count > 0)) | |||
|
|
|||
| if(!firstElement && isSequence) | |||
There was a problem hiding this comment.
I don't think there's any case we want to write else. And when would this condition be true?
There was a problem hiding this comment.
All the changes are only for the case when isSequence = false.
| @@ -2344,7 +2353,10 @@ private void WriteMemberElementsIf(Member[] members, Member anyElement, string e | |||
| if (isSequence) | |||
| Writer.WriteLine("default:"); | |||
| else | |||
| Writer.WriteLine("else {"); | |||
| { | |||
| Writer.WriteLine("Finish:"); | |||
There was a problem hiding this comment.
Is this the right place for Finish:? This seems to be the line of original else. I guessed the Finish label should be after the original else block.
There was a problem hiding this comment.
You are right. So the Finish tag has to put outside of this method. Seems goto is not the right way in this case and I will switch back to do while(false).
| } | ||
| else | ||
| { | ||
| Writer.Indent--; |
There was a problem hiding this comment.
The Finish label should probably be here.
…n write member elements only when the total number of members is bigger than 1000
* Port the if/else fix from SG to corefx. Fix another if/else issue when write member elements only when the total number of members is bigger than 1000 * Use goto instead of do while. * Use do while. * Add missing Indent++ when IsSequence=true.
* Port the if/else fix from SG to corefx. Fix another if/else issue when write member elements only when the total number of members is bigger than 1000 * Use goto instead of do while. * Use do while. * Add missing Indent++ when IsSequence=true.
* Port the if/else fix from SG to corefx. Fix another if/else issue when write member elements only when the total number of members is bigger than 1000 * Use goto instead of do while. * Use do while. * Add missing Indent++ when IsSequence=true. Commit migrated from dotnet/corefx@c867915
…x#26285) Commit migrated from dotnet/corefx@a8d34fe
Port the if/else fix from SG to corefx. Fix another if/else issue when write member elements only when the total number of members is bigger than 1000.
Fix #25248
@shmao @zhenlan @mconnew