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 18 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
6 changes: 3 additions & 3 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>The message that describes the exception.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand Down Expand Up @@ -772,4 +772,4 @@
</Docs>
</Member>
</Members>
</Type>
</Type>
26 changes: 13 additions & 13 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 Expand Up @@ -3915,4 +3915,4 @@
</Docs>
</Member>
</Members>
</Type>
</Type>
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>
28 changes: 23 additions & 5 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>Returns a result string in which arguments are formatted by using the conventions of the current culture.</summary>
<returns>The string that results from formatting the current instance by using the conventions of the current culture.</returns>
<remarks>
<format type="text/markdown">
<![CDATA[

### Remarks

This static method may be imported in C# by:

```csharp
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:

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

]]></format>.</remarks>
</Docs>
</Member>
<Member MemberName="Format">
Expand Down Expand Up @@ -468,4 +486,4 @@ Invariant($"{{ lat = {latitude}; lon = {longitude} }}")
</Docs>
</Member>
</Members>
</Type>
</Type>
112 changes: 74 additions & 38 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:

```csharp
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" />) of a collection.</param>
<summary>Initializes a new <see cref="T:System.Index" /> with a specified index position and a value that indicates if the index is from the start or the end of a collection.</summary>
<remarks>
<format type="text/markdown">
<![CDATA[

### Remarks

If the <see cref="T:System.Index" /> is constructed from the end, an index value of 1 points to the last element, and an index value of 0 points 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>Gets an <see cref="T:System.Index" /> that points beyond the last element.</summary>
<value>an <see cref="T:System.Index" /> that points beyond the 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">The object to compare with this instance.</param>
<summary>Returns a value that indicates whether the current object is equal to another <see cref="T:System.Index" /> object.</summary>
<returns><see langword="true" /> if the current Index object is equal to <paramref name="other" />; <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 instance.</param>
<summary>Indicates whether the current Index object is equal to a specified object.</summary>
<returns><see langword="true" /> if <paramref name="value" /> is of type <see cref="T:System.Index" /> and is equal to the current instance; <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 of a collection.</param>
<summary>Creates an <see cref="T:System.Index" /> from the end of a collection at a specified index position.</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 position from the start of a collection.</param>
<summary>Create an <see cref="T:System.Index" /> from the specified index at the start of a collection.</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,23 @@
<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, 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`.


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 +313,8 @@
<ReturnType>System.Boolean</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Gets a value that indicates whether the index is from the start or the end.</summary>
<value><see langword="true" /> if the Index is from the end; otherwise, <see. langword="false" />.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -305,9 +341,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 +367,8 @@
<ReturnType>System.Index</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Gets an <see cref="T:System.Index" /> that points to the first element of a collection.</summary>
<value>An instance that points to the first element of a collection.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -357,8 +393,8 @@
</ReturnValue>
<Parameters />
<Docs>
<summary>To be added.</summary>
<returns>To be added.</returns>
<summary>Returns the string representation of the current <see cref="T:System.Index" /> instance.</summary>
<returns>The string representation of the <see cref="T:System.Index" />.</returns>
<remarks>To be added.</remarks>
</Docs>
</Member>
Expand All @@ -382,10 +418,10 @@
<ReturnType>System.Int32</ReturnType>
</ReturnValue>
<Docs>
<summary>To be added.</summary>
<value>To be added.</value>
<summary>Gets the index value.</summary>
<value>The index value.</value>
<remarks>To be added.</remarks>
</Docs>
</Member>
</Members>
</Type>
</Type>
3 changes: 2 additions & 1 deletion 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"><paramref name="handler" /> is <see langword="null" /> (<see langword="Nothing" /> in Visual Basic).</exception>
</Docs>
</Member>
<Member MemberName="OnReport">
Expand Down Expand Up @@ -247,4 +248,4 @@
</Docs>
</Member>
</Members>
</Type>
</Type>
Loading