Add some documentation for standard library things. #3540

nmichaels · 2019-10-27T05:01:05Z

Added a bunch of descriptions for array_list.
Added some usage notes for failing_allocator.
Documented some of mem.Allocator.

Added a bunch of descriptions for array_list. Added some usage notes for failing_allocator. Documented some of mem.Allocator.

andrewrk

Thank you for getting the documentation effort started!

andrewrk · 2019-10-27T19:29:40Z

lib/std/array_list.zig

@@ -5,6 +5,10 @@ const testing = std.testing;
 const mem = std.mem;
 const Allocator = mem.Allocator;

+/// Safe, resizable array type.


I think calling this "safe" is misleading, since there are a handful of ways to use ArrayList unsafely.

obtaining a reference to an element and then appending an element or otherwise changing the size.

accessing items directly instead of using toSlice or toSliceConst

That's fair. I wanted to characterize it somewhat like std::vector, since that's what it seems to be. It's safer than just passing an unsized array around. Maybe just leave the "List of elements" part? I feel like this should be where an example for canonical use goes.

andrewrk · 2019-10-27T19:37:31Z

lib/std/array_list.zig

+        /// Extend the list by 1 element, but assuming `self.capacity`
+        /// is sufficient to hold an additional item. Use with care;
+        /// will produce undefined behavior in ReleaseFast and
+        /// ReleaseSmall modes.


See #2402, and consider that "assert" means "invokes detectable illegal behavior if the asserted value is false". I would phrase like this:

/// Extend the list by 1 element, asserting `self.capacity` /// is sufficient to hold an additional item.

The concept of illegal behavior and assertions is pervasive in all zig code. Documentation readers absolutely must understand what "assert" means in documentation.

You can see that the definition of std.debug.assert matches this:

zig/lib/std/debug.zig

Lines 195 to 207 in f756d87

/// This function invokes undefined behavior when `ok` is `false`.

/// In Debug and ReleaseSafe modes, calls to this function are always

/// generated, and the `unreachable` statement triggers a panic.

/// In ReleaseFast and ReleaseSmall modes, calls to this function are

/// optimized away, and in fact the optimizer is able to use the assertion

/// in its heuristics.

/// Inside a test block, it is best to use the `std.testing` module rather

/// than this function, because this function may not detect a test failure

/// in ReleaseFast and ReleaseSmall mode. Outside of a test block, this assert

/// function is the correct function to use.

pub fn assert(ok: bool) void {

if (!ok) unreachable; // assertion failure

}

andrewrk · 2019-10-27T19:39:29Z

lib/std/array_list.zig

        pub fn appendAssumeCapacity(self: *Self, item: T) void {
            const new_item_ptr = self.addOneAssumeCapacity();
            new_item_ptr.* = item;
        }

+        /// Remove the element at index `i` from the list and return
+        /// its value.


As an example from the previous comment, this function also asserts something.

Asserts the array has at least one element.

Better to use the word "assert" here, I think, than repeat the "Use with care..." sentence.

andrewrk · 2019-10-27T19:41:29Z

lib/std/array_list.zig

+        /// Append the slice of items to the list. Copies the contents
+        /// of `items`.


"Copies the contents of items" is a bit misleading here. What is this sentence disambiguating? If the answer is nothing, let's omit it.

I was trying to make it clear that it's not a move, and that if items contains any pointers they're aliased now. Of course, that's true for all the insertion functions, so it probably either belongs in all of them or none of them. Maybe when the top-level doc has examples, it can be made clear there.

andrewrk · 2019-10-27T19:43:21Z

lib/std/array_list.zig

@@ -191,11 +221,13 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type {
            return result;
        }

+        /// Remove and return the last element from the list.


/// Asserts the array has at least one element.

andrewrk · 2019-10-27T19:44:32Z

lib/std/debug/failing_allocator.zig

+///
+/// To use this, first initialize it and get an allocator with
+/// 
+/// `var failing_allocator = &FailingAllocator.init(<allocator>,


Suggested change

/// `var failing_allocator = &FailingAllocator.init(<allocator>,

/// `const failing_allocator = &FailingAllocator.init(<allocator>,

andrewrk · 2019-10-27T19:46:50Z

lib/std/debug/failing_allocator.zig

+    /// fail_index is the number of successful allocations you can
+    /// expect from this allocator. Allocation <fail_index> + 1 will
+    /// fail.


fail_index is the index that will fail. What makes you think it's fail_index + 1 ?

This is a zero-based index vs. 1-based index confusion thing. I'll change it to fail_index and add an example to clarify.

andrewrk · 2019-10-27T19:59:09Z

lib/std/mem.zig

+    /// Returns an uninitialized array of T. Call `free` to free the
+    /// memory.


Not uninitialized - set to undefined. This is meaningful in Zig. Also, free is not always required, for example the purpose of std.heap.ArenaAllocator is to make freeing not required.

Allocates an array of n items of type T and sets all the items to undefined. Depending on the Allocator implementation, it may be required to call free once the memory is no longer needed, to avoid a resource leak. If the Allocator implementation is unknown, then correct code will call free when done.

For allocating a single item, see create.

Note that accepted proposal #2292 would make it possible to determine whether calling free() is necessary.

andrewrk · 2019-10-27T20:04:18Z

lib/std/mem.zig

@@ -218,6 +220,7 @@ pub const Allocator = struct {
        return @bytesToSlice(T, @alignCast(new_alignment, byte_slice));
    }

+    /// Free an array allocated with `alloc`.


Additional docs:

To free a single item, see destroy.

Add some documentation for standard library things.

254d871

Added a bunch of descriptions for array_list. Added some usage notes for failing_allocator. Documented some of mem.Allocator.

daurnimator added docs standard library This issue involves writing Zig code for the standard library. labels Oct 27, 2019

andrewrk requested changes Oct 27, 2019

View reviewed changes

Corrected several function descriptions, clarified others.

3df1fc1

andrewrk merged commit 6fdeaac into ziglang:master Oct 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some documentation for standard library things. #3540

Add some documentation for standard library things. #3540

nmichaels commented Oct 27, 2019

andrewrk left a comment

andrewrk Oct 27, 2019

nmichaels Oct 28, 2019

andrewrk Oct 27, 2019

andrewrk Oct 27, 2019

andrewrk Oct 27, 2019

nmichaels Oct 28, 2019

andrewrk Oct 27, 2019

andrewrk Oct 27, 2019

andrewrk Oct 27, 2019

nmichaels Oct 28, 2019

andrewrk Oct 27, 2019

andrewrk Oct 27, 2019

	/// This function invokes undefined behavior when `ok` is `false`.
	/// In Debug and ReleaseSafe modes, calls to this function are always
	/// generated, and the `unreachable` statement triggers a panic.
	/// In ReleaseFast and ReleaseSmall modes, calls to this function are
	/// optimized away, and in fact the optimizer is able to use the assertion
	/// in its heuristics.
	/// Inside a test block, it is best to use the `std.testing` module rather
	/// than this function, because this function may not detect a test failure
	/// in ReleaseFast and ReleaseSmall mode. Outside of a test block, this assert
	/// function is the correct function to use.
	pub fn assert(ok: bool) void {
	if (!ok) unreachable; // assertion failure
	}

		/// Append the slice of items to the list. Copies the contents
		/// of `items`.

	/// `var failing_allocator = &FailingAllocator.init(<allocator>,
	/// `const failing_allocator = &FailingAllocator.init(<allocator>,

		/// Returns an uninitialized array of T. Call `free` to free the
		/// memory.

Add some documentation for standard library things. #3540

Add some documentation for standard library things. #3540

Conversation

nmichaels commented Oct 27, 2019

andrewrk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment