Skip to content

coreclr: Automatic port of triple slash from System #2729

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.

Sign up for GitHub

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

Merged
merged 21 commits into from
Jul 24, 2019
Merged

coreclr: Automatic port of triple slash from System #2729

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
aedd552
Automatic port of triple slash from System
carlossanlop Jul 16, 2019
1c78edf
Various fixes
carlossanlop Jul 16, 2019
3b193fc
Update Range.xml
Jul 18, 2019
cc0ec5f
Update SequencePosition.xml
Jul 18, 2019
c994b64
Update Single.xml
Jul 18, 2019
6aefa09
Update Span`1.xml
Jul 18, 2019
731c14c
Update ValueTuple.xml
Jul 18, 2019
ce54d35
Update ValueTuple`2.xml
Jul 18, 2019
0105f1a
Corrected malformed xml.
Jul 18, 2019
95ccebe
corrected malformed xml
Jul 18, 2019
816e6f0
Update xml/System/AggregateException.xml
carlossanlop Jul 19, 2019
ac2c27f
Double.xml
carlossanlop Jul 19, 2019
c8e643b
FormattableString.xml
carlossanlop Jul 19, 2019
b8caade
Index.xml
carlossanlop Jul 19, 2019
2da3c3f
Progress.xml
carlossanlop Jul 19, 2019
cd3d718
ReadOnlySpan.xml
carlossanlop Jul 19, 2019
50f2db2
String.xml
Jul 19, 2019
30dee39
Merge branch 'clr_System' of https://github.com/carlossanlop/dotnet-a…
Jul 19, 2019
8c4741d
varied suggestions
carlossanlop Jul 24, 2019
652987f
Merge branch 'master' into clr_System
Jul 24, 2019
acbc62c
Minor changes.
Jul 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions xml/System/AggregateException.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,8 +728,8 @@
<ReturnType>System.String</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Gets a message that describes the exception.</summary>
<value>A string representing the message that describes the exception.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down
24 changes: 12 additions & 12 deletions xml/System/Double.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,9 +850,9 @@
<Parameter Name="d" Type="System.Double" Index="0" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="d">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="d">A double-precision floating-point number.</param>
<summary>Determines whether the specified value is finite (zero, subnormal, or normal).</summary>
<returns><see langword="true" /> if the value is finite (zero, subnormal or normal), <see langword="false" /> otherwise.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -1021,9 +1021,9 @@
<Parameter Name="d" Type="System.Double" Index="0" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="d">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="d">A double-precision floating-point number.</param>
<summary>Determines whether the specified value is negative.</summary>
<returns><see langword="true" /> if the value is negative, <see langword="false" /> otherwise.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -1122,9 +1122,9 @@
<Parameter Name="d" Type="System.Double" Index="0" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="d">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="d">A double-precision floating-point number.</param>
<summary>Determines whether the specified value is normal.</summary>
<returns><see langword="true" /> if the value is normal, <see langword="false" /> otherwise.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -1217,9 +1217,9 @@
<Parameter Name="d" Type="System.Double" Index="0" FrameworkAlternate="netcore-2.1;netcore-2.2;netcore-3.0;netstandard-2.1" />
</Parameters>
<Docs>
<param name="d">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="d">A double-precision floating-point number.</param>
<summary>Determines whether the specified value is subnormal.</summary>
<returns><see langword="true" /> if the value is subnormal, <see langword="false" /> otherwise.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down
6 changes: 3 additions & 3 deletions xml/System/Environment.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2076,8 +2076,8 @@ The following example creates environment variables for the <xref:System.Environ
<ReturnType>System.Int64</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Gets the number of milliseconds elapsed since the system started.</summary>
<value>The elapsed milliseconds since the system started.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -2360,4 +2360,4 @@ The following example creates environment variables for the <xref:System.Environ
</Docs>
</Member>
</Members>
</Type>
</Type>
26 changes: 22 additions & 4 deletions xml/System/FormattableString.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,28 @@
<Parameter Name="formattable" Type="System.FormattableString" Index="0" FrameworkAlternate="netcore-3.0" />
</Parameters>
<Docs>
<param name="formattable">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<remarks>To be added.</remarks>
<param name="formattable">The string to be formatted.</param>
<summary>Format the given object in the current culture.</summary>
<returns>The formatted string in the current culture.</returns>
<remarks>
<format type="text/markdown">
<![CDATA[

### Remarks

This static method may be imported in C# by:

```cs
using static System.FormattableString;
```

Within the scope of that import directive an interpolated string may be formatted in the current culture by writing, for example:

```cs
CurrentCulture($"{{ lat = {latitude}; lon = {longitude} }}");
```

]]></format>.</remarks>
</Docs>
</Member>
<Member MemberName="Format">
Expand Down
111 changes: 74 additions & 37 deletions xml/System/Index.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,21 @@
</Attribute>
</Attributes>
<Docs>
<summary>To be added.</summary>
<remarks>To be added.</remarks>
<summary>Represent a type can be used to index a collection either from the start or the end.</summary>
<remarks>
<format type="text/markdown"><![CDATA[

### Remarks

Index is used by the C# compiler to support the new index syntax:

```cs
int[] someArray = new int[5] { 1, 2, 3, 4, 5 };
int lastElement = someArray[^1]; // lastElement = 5
```

]]></format>
</remarks>
</Docs>
<Members>
<Member MemberName=".ctor">
Expand All @@ -52,10 +65,20 @@
<Parameter Name="fromEnd" Type="System.Boolean" />
</Parameters>
<Docs>
<param name="value">To be added.</param>
<param name="fromEnd">To be added.</param>
<summary>To be added.</summary>
<remarks>To be added.</remarks>
<param name="value">The index value. It has to be greater or equal than zero.</param>
<param name="fromEnd">A boolean indicating if the index is from the start <see langword="false" /> or from the end <see langword="true" />.</param>
<summary>Construct an Index using a value and indicating if the index is from the start or from the end.</summary>
<remarks>
<format type="text/markdown">
<![CDATA[

### Remarks

If the Index constructed from the end, index value 1 means pointing at the last element and index value 0 means pointing at beyond last element.

]]>
</format>
</remarks>
</Docs>
</Member>
<Member MemberName="End">
Expand All @@ -78,8 +101,8 @@
<ReturnType>System.Index</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Create an Index pointing at beyond last element.</summary>
<value>The Index pointing at beyond last element.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -109,9 +132,9 @@
<Parameter Name="other" Type="System.Index" />
</Parameters>
<Docs>
<param name="other">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="other">An Index object to compare with this Index object.</param>
<summary>Indicates whether the current Index object is equal to another Index object.</summary>
<returns><see langword="true" /> if the current Index object is equal to the passed Index object, <see langword="false" /> otherwise.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -138,9 +161,9 @@
<Parameter Name="value" Type="System.Object" />
</Parameters>
<Docs>
<param name="value">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="value">An object to compare with this Index object.</param>
<summary>Indicates whether the current Index object is equal to the passed object.</summary>
<returns><see langword="true" /> if the current Index object is equal to the passed object, <see langword="false" /> otherwise.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -167,9 +190,9 @@
<Parameter Name="value" Type="System.Int32" />
</Parameters>
<Docs>
<param name="value">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="value">The index value from the end.</param>
<summary>Create an Index from the end at the position indicated by the value.</summary>
<returns>The Index value.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -196,9 +219,9 @@
<Parameter Name="value" Type="System.Int32" />
</Parameters>
<Docs>
<param name="value">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="value">The index value from the start.</param>
<summary>Create an Index from the start at the position indicated by the value.</summary>
<returns>The index value.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -223,8 +246,8 @@
</ReturnValue>
<Parameters />
<Docs>
<summary>To be added.</summary>
<returns>To be added.</returns>
<summary>Returns the hash code for this instance.</summary>
<returns>The hash code.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -251,10 +274,24 @@
<Parameter Name="length" Type="System.Int32" />
</Parameters>
<Docs>
<param name="length">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<remarks>To be added.</remarks>
<param name="length">The length of the collection that the Index will be used with. Must be a positive value.</param>
<summary>Calculate the offset from the start using the giving collection length.</summary>
<returns>The offset.</returns>
<remarks>
<format type="text/markdown">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This remark was a bit difficult to understand, especially the 3rd sentence, so I left it untouched. Can I get some help to properly rewrite it?
There's another similar remark in another API in this same PR.

Copy link
Contributor Author

@carlossanlop carlossanlop Jul 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Ok I gave it a try. Hopefully it looks a bit better.

Copy link

Choose a reason for hiding this comment

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

I still don't think it's clear. There's nothing that ties length to the length of a collection. And the source ignores the length argument if the index is from the start and just returns the original index:

        public int GetOffset(int length)
        {
            int offset = _value;
            if (IsFromEnd)
            {
                // offset = length - (~value)
                // offset = length + (~(~value) + 1)
                // offset = length + value + 1
 
                offset += length + 1;
            }
            return offset;
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Help us, @tarekgh, you're our only hope.
Can you help clarify this remark?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What I understood from Tarek's explanation is that length is only considered when retrieving the offset from the end. You are correct, the method ignores the length if it's from the start.

<![CDATA[

### Remarks

For performance reasons, we don't validate the input length parameter and the returned offset value against negative values.
Copy link

Choose a reason for hiding this comment

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

Suggested change
For performance reasons, we don't validate the input length parameter and the returned offset value against negative values.
For performance reasons, this method does not test whether `length` or the return value is negative. It also doesn't test whether the return value is greater than `length`.


We also don't validate if the returned offset is greater than the input length.
Copy link

Choose a reason for hiding this comment

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

Suggested change
We also don't validate if the returned offset is greater than the input length.


It is expected that Index will always be used with collections have non negative length/count. If the returned offset is negative and then used to index, a collection will throw range exception which will be the same effect as the validation.
Copy link

Choose a reason for hiding this comment

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

Suggested change
It is expected that Index will always be used with collections have non negative length/count. If the returned offset is negative and then used to index, a collection will throw range exception which will be the same effect as the validation.
If the returned offset is negative and is then used as an index into a collection, the operation throws an <xref:System.ArgumentOutOfRangeException>.

I don't understand this original comment. Is it even possible to have a collection or an array with a negative count or length? It's possible for an array to have an arbitrary starting index, but that doesn't affect its length.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tarekgh can you also help explain this comment a bit more?

Copy link
Member

@tarekgh tarekgh Jul 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Here is some clarification:

before I explain the comments, I want to clarify what is the purpose of this method

Index can be created not tied to any collection (think about it as int value). The index can be from the beginning or from the end. The callers of GetOffset is going to pass collection length to calculate the offset which the Index point to. If the index from start, then we just return whatever stored offset. If the index from end, we'll use the input length to calculate the offset. Here is some examples:

imagine you have index from start with value 1, and then calling GetOffset(10), this will return 1 because this is the offset from the beginning of the collection.
If you have Index with value 1 but from the end, and calling GetOffset(10) this will return 9. which is the offset from the beginning of the collection.

Now let's explain the remark written there:

#1
What happen if someone pass negative length value (i.e. GetOffset(-10)), this should be invalid. For performance reason we don't validate the input length and we don't even fail but we'll return some invalid result too. It is the caller responsibility to pass correct logical values.

#2
If you have Index with value 10 and then calling GetOffset(4), this also is invalid input because the used index is outside the range of the passed collection length. again for performance reason we don't validate that and we'll return invalid value. It is the caller responsibility to pass correct logical values.

#3
Is like #2, imagine you have a collection with length 4 and then have Index with value 10, calling GetOffset(4) and Index is from start, then this will return 10 and when you use 10 to access the collection you'll get out of range exception.
If the index is from the end, GetOffset(4) will return negative value (-6) and when you use this value to access the collection will get OutOfRangeException.

We are trying to say even we are not validating the input and the result of GetOffset, in most common cases when operating over a collection, will get OutOfRangeException if used invalid input to GetOffset.

Let me know if this is still not clear.

In reply to: 305165870 [](ancestors = 305165870)

Copy link
Contributor Author

@carlossanlop carlossanlop Jul 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Ok I understand now, thanks for the explanation. I'll reword it a bit:

Suggested change
It is expected that Index will always be used with collections have non negative length/count. If the returned offset is negative and then used to index, a collection will throw range exception which will be the same effect as the validation.
Collections are not expected to have a non-negative length/count. If this method's returned offset is negative and then used to index a collection, the collection will throw <xref:System.ArgumentOutOfRangeException>, which will have the same effect as validation.


]]>
</format>
</remarks>
</Docs>
</Member>
<Member MemberName="IsFromEnd">
Expand All @@ -277,8 +314,8 @@
<ReturnType>System.Boolean</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Indicates whether the index is from the start or the end.</summary>
<value><see langword="true" /> if the Index is from the end, <see. langword="false" /> if it's not.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -305,9 +342,9 @@
<Parameter Name="value" Type="System.Int32" />
</Parameters>
<Docs>
<param name="value">To be added.</param>
<summary>To be added.</summary>
<returns>To be added.</returns>
<param name="value">The integer to convert.</param>
<summary>Converts integer number to an Index.</summary>
<returns>An Index representing the integer.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -331,8 +368,8 @@
<ReturnType>System.Index</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Creates an Index pointing at the first element.</summary>
<value>An Index of the first element.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -357,8 +394,8 @@
</ReturnValue>
<Parameters />
<Docs>
<summary>To be added.</summary>
<returns>To be added.</returns>
<summary>Converts the value of the current Index object to its equivalent string representation.</summary>
<returns>The string representation of the Index.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -382,8 +419,8 @@
<ReturnType>System.Int32</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Returns the index value.</summary>
<value>The integer value of the Index.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down
1 change: 1 addition & 0 deletions xml/System/Progress`1.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
<param name="handler">A handler to invoke for each reported progress value. This handler will be invoked in addition to any delegates registered with the <see cref="E:System.Progress`1.ProgressChanged" /> event. Depending on the <see cref="T:System.Threading.SynchronizationContext" /> instance captured by the <see cref="T:System.Progress`1" /> at construction, it is possible that this handler instance could be invoked concurrently with itself.</param>
<summary>Initializes the <see cref="T:System.Progress`1" /> object with the specified callback.</summary>
<remarks>To be added.</remarks>
<exception cref="T:System.ArgumentNullException">The <paramref name="handler" /> is <see langword="null" /> (Nothing in Visual Basic).</exception>
</Docs>
</Member>
<Member MemberName="OnReport">
Expand Down
Loading