Skip to content
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.

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

some documentation bugs/quesitons #100

Closed
1 of 6 tasks
akrzemi1 opened this issue Sep 2, 2023 · 7 comments
Closed
1 of 6 tasks

some documentation bugs/quesitons #100

akrzemi1 opened this issue Sep 2, 2023 · 7 comments

Comments

@akrzemi1
Copy link
Member

akrzemi1 commented Sep 2, 2023

This is a feedback from the review of the documentaiotn.

(1) . Section "Motivation" says:

It takes a collection of concepts from other languages and provides them based on C++20 coroutines.

  • easy asynchronous base functions, such as an async main & threads

What does this "threads" mean here? A link in the docs would be useful.

(2). Section "Coroutine Premier" > "Coroutines" gives example of a value consuming generator that does not compile:

async::generator<int, bool> iota()
{
  int i = 0;
  bool more = false;
  do
  {
    more = co_yield i++;
  }
  while(more);
  co_return ;
}

Apparently, one has to yield and return values of the same type. But a general problem is that this example is unrelaistic. If I no longer require new vales, I just abandon the coroutine (and trigger the call to .destroy. Why would I inform the coroutine body that I want to break the loop. And this brings me to an even more general concern. I have seen no realistic motivating example for a value-consuming generator. Nor can I imagine one. Is it possible that this feature has no application?

(3). Section "Coroutine Premier" > "Awaitables":

  • "Awaitables are types that can be used in a co_await expression." -> "Awaitables are types that can be used as an operand in a co_await expression."
  • "In a co_await statement" -> "In a co_await expression"
  • "The return_type is the result type of the coroutine" -> "The return_type is the type of the co_await expression"
  • Regarding the term "immediate completion", maybe you want to offer a section "Concepts" or "Definitions" that would list and describe the concepts used by this library. You could then make every ocurrence of "immediate completion" a link to the place where it is defined.

(4). Regarding "async uses an asio::io_context as its default event loop.":

  • Could you make asio::io_context link to the corresponding seciton of Boost.Asio docs?
  • "default event loop" implies that this library can work on other event loops. Is this the case? If so, could the docs show an example of this, and link to it from where statement "async uses an asio::io_context as its default event loop" appears?
@akrzemi1
Copy link
Member Author

akrzemi1 commented Sep 2, 2023

(5). In section "Tutorial" > "Delay" we have a comment:

The co_main function defines an implicit main when defined and is the easiest way to enter asynchronous code.

This is imprecise, and therefore confusing. Maybe you want to say:

(0) Header <boost/async/main.hpp> defines function main which performs setup required for aynchronous single-threaded computations, such as starting the default event loop and registering signal processing, and then calls co_main as an entry point to the application.

Next, we have:

(2) Take the executor from the current coroutine promise.

If this is an immediate competion, and I think it is, please indicate this.

@akrzemi1 akrzemi1 changed the title minor documentation bugs some documentation bugs/quesitons Sep 2, 2023
@akrzemi1
Copy link
Member Author

akrzemi1 commented Sep 2, 2023

(6) The echo server tutorial has the following sentence in the description of the listener:

Since this is a tutorial, we’re using a generator.

This seems to imply that a non-tutorial server implementation would not use generator. Is it true?

@akrzemi1
Copy link
Member Author

akrzemi1 commented Sep 2, 2023

(7). In the same echo server example:

This is a coroutine that can be co_awaited multiple times, until a co_return statement is reached.

There is no co_return in the coroutine body. Maybe change it to something like:

This is a coroutine that can be co_awaited multiple times, until the entire body of the coroutine has been executed.

@akrzemi1
Copy link
Member Author

akrzemi1 commented Sep 2, 2023

In the same echo server example we have:

The wait_group is used to manage the running echo functions. This class will cancel & await the running echo coroutines.

Do you mean, "This class will cancel & await the running echo coroutines when wait_group is destroyed"? Or that the interface of wait_group allows user to cancel & await the promises?

Next sentence:

We do not need to do the same for the listener, because it will just stop on its own, when l gets destroyed. > The destructor of a generator will cancel it.

I think that, unlike for the acceptor, the goal of the wait_group is to extend the lifetime of async::promise created my echo() in order not to incur an unnecessary synchronization upon each loop iteration. Am I right? If so, maybe the docs should state this explicitly. Otherwise it looks like saying that for std::promise we forgot to implement the stoppage of the coroutine in the destructor.

Also "I gets" -> "it gets".

@akrzemi1
Copy link
Member Author

akrzemi1 commented Sep 2, 2023

(9). use_op exmple has:

We can however implement our own ops, that can also utilize the async_ready optimization.

Do you mean the "immediate completion"? If so, you should probably use this defined term.

@akrzemi1
Copy link
Member Author

akrzemi1 commented Sep 2, 2023

(10). When describing the contents of <async/this_thread.hpp>, please wrap all the declarations into a namespace this_thread:

namespace async::this_thread {
  pmr::memory_resource* get_default_resource() noexcept; 
  pmr::memory_resource* set_default_resource(pmr::memory_resource* r) noexcept; 
  pmr::polymorphic_allocator<void> get_allocator(); 

  typename asio::io_context::executor_type & get_executor(); 
  void set_executor(asio::io_context::executor_type exec) noexcept; 
}

Otherwise, there is no indication that they are in a dedicated namespace. It is the more difficult as we also have std::this_thread namespace.

@akrzemi1
Copy link
Member Author

akrzemi1 commented Sep 2, 2023

(11). The documentation provides only section "Outline" for each header, but is missing the detailed description of individual functions. Because of this we do not know the contract of the functions: their preconditions, behavior in corner cases (such as empty vector passed to select), exception safety, and any other relevant information.

klemens-morgenstern added a commit that referenced this issue Sep 5, 2023
Incorporates much of #100.
klemens-morgenstern added a commit that referenced this issue Oct 5, 2023
klemens-morgenstern added a commit that referenced this issue Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant