`CoyieldExpr` doesn't contain the original AST `-ast-print` returns invalid source.

[hanickadot]

Following code when parsed and emitted with -ast-print returns wrong result:

simple_coro<int> coro_test() {
  co_yield 2001;
  co_return 2010;
  co_await my_awaitable{2061};
}

Result printed by clang is: (with fixed formatting

simple_coro<int> coro_test() {
  co_yield __promise.yield_value(2001); // <-- ERROR HERE
  co_return 2010;
  co_await my_awaitable{2061};
}

Expected result:

simple_coro<int> coro_test() {
  co_yield 2001;
  co_return 2010;
  co_await my_awaitable{2061};
}

Notice the co_yield __promise.yield_value(1997); it contains call to __promise.yield_value function, this helper AST is generated in SemaCoroutine.cpp in Sema::ActOnCoyieldExpr:

  // Build yield_value call.
  ExprResult Awaitable = buildPromiseCall(
      *this, getCurFunction()->CoroutinePromise, Loc, "yield_value", E);
  if (Awaitable.isInvalid())
    return ExprError();

  // Build 'operator co_await' call.
  Awaitable = buildOperatorCoawaitCall(*this, S, Loc, Awaitable.get());
  if (Awaitable.isInvalid())
    return ExprError();

There is Operand slot inside CoroutineSuspendExpr which should contain original expression, but the function BuildCoyieldExpr is already getting modified AST.

Full source code to repeat the problem:

#include <coroutine>

struct my_awaitable {
  int value;
  bool await_ready() const {
    return true;
  }
  void await_suspend(std::coroutine_handle<void>) {
    
  }
  int await_resume() const {
    return value;
  }
};

template <typename T> struct simple_coro {
	struct promise_type {
		T value{0};

		constexpr auto initial_suspend() const noexcept {
			return std::suspend_never{};
		}
		constexpr auto final_suspend() const noexcept {
			return std::suspend_never{};
		}
		constexpr void return_value(int v) noexcept {
      value = v;
		}
		constexpr auto yield_value(T v) noexcept {
      value = v;
			return std::suspend_always{};
		}
		std::coroutine_handle<promise_type> get_return_object() {
			return std::coroutine_handle<promise_type>::from_promise(*this);
		}
		constexpr void unhandled_exception() const noexcept {
		}
	};

private:
	std::coroutine_handle<promise_type> handle;

public:
	constexpr simple_coro(std::coroutine_handle<promise_type> h) noexcept: handle{h} { }
	constexpr ~simple_coro() noexcept {
		handle.destroy();
	}
	constexpr int get() {
		return handle.promise().value;
	}
};

simple_coro<int> coro_test() {
  co_yield 2001;
  co_return 2010;
  co_await my_awaitable{2061};
}

Maybe this is fine, but AFAIK clang is trying to represent the AST so the original source code is recoverable and this breaks it.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Hana Dusíková (hanickadot)

Following code when parsed and emitted with `-ast-print` returns wrong result:

simple_coro<int> coro_test() {
  co_yield 2001;
  co_return 2010;
  co_await my_awaitable{2061};
}

Result printed by clang is: (with fixed formatting

simple_coro<int> coro_test() {
  co_yield __promise.yield_value(2001); // <-- ERROR HERE
  co_return 2010;
  co_await my_awaitable{2061};
}

Expected result:

simple_coro<int> coro_test() {
  co_yield 2001;
  co_return 2010;
  co_await my_awaitable{2061};
}

Notice the co_yield __promise.yield_value(1997); it contains call to __promise.yield_value function, this helper AST is generated in SemaCoroutine.cpp in Sema::ActOnCoyieldExpr:

  // Build yield_value call.
  ExprResult Awaitable = buildPromiseCall(
      *this, getCurFunction()->CoroutinePromise, Loc, "yield_value", E);
  if (Awaitable.isInvalid())
    return ExprError();

  // Build 'operator co_await' call.
  Awaitable = buildOperatorCoawaitCall(*this, S, Loc, Awaitable.get());
  if (Awaitable.isInvalid())
    return ExprError();

There is Operand slot inside CoroutineSuspendExpr which should contain original expression, but the function BuildCoyieldExpr is already getting modified AST.

Full source code to repeat the problem:

#include <coroutine>

struct my_awaitable {
  int value;
  bool await_ready() const {
    return true;
  }
  void await_suspend(std::coroutine_handle<void>) {
    
  }
  int await_resume() const {
    return value;
  }
};

template <typename T> struct simple_coro {
	struct promise_type {
		T value{0};

		constexpr auto initial_suspend() const noexcept {
			return std::suspend_never{};
		}
		constexpr auto final_suspend() const noexcept {
			return std::suspend_never{};
		}
		constexpr void return_value(int v) noexcept {
      value = v;
		}
		constexpr auto yield_value(T v) noexcept {
      value = v;
			return std::suspend_always{};
		}
		std::coroutine_handle<promise_type> get_return_object() {
			return std::coroutine_handle<promise_type>::from_promise(*this);
		}
		constexpr void unhandled_exception() const noexcept {
		}
	};

private:
	std::coroutine_handle<promise_type> handle;

public:
	constexpr simple_coro(std::coroutine_handle<promise_type> h) noexcept: handle{h} { }
	constexpr ~simple_coro() noexcept {
		handle.destroy();
	}
	constexpr int get() {
		return handle.promise().value;
	}
};

simple_coro<int> coro_test() {
  co_yield 2001;
  co_return 2010;
  co_await my_awaitable{2061};
}

Maybe this is fine, but AFAIK clang is trying to represent the AST so the original source code is recoverable and this breaks it.

[shafik]

We don't make any guarantees about being able to recover the original code from the -ast-print but I don't know if this result was intended or not.

[hanickadot]

It doesn't feel like a right result. If that was an intentent co_await in presence of .await_transform should be seen as co_await __promise.await_transform(<expr>) and co_return would be __promise.return_value(<expr>).

But what's worse, if I change promise_type::yield_value to:

constexpr auto yield_value(T v) noexcept {
  value = v;
  return packed_value<T>{v};
}

Where packed_value has operator co_await:

template <typename T> struct lazy_awaitable {
  const T & value;
  bool await_ready() const {
    return true;
  }
  void await_suspend(std::coroutine_handle<void>) {
    
  }
  T await_resume() const {
    return value;
  }
};

template <typename T> struct packed_value {
  T value;
  lazy_awaitable<T> operator co_await() const noexcept {
    return lazy_awaitable<T>{value};
  }
};

Then the result of -ast-print (or -ast-dump's equivalent) is:

simple_coro<int> coro_test(int &&value) {
  co_yield co_await __promise.yield_value(value);
}

for this original code:

simple_coro<int> coro_test(int && value) {
  co_yield value;
}

That's definitely not right as co_yield.

[hanickadot]

co_await and co_yield expressions are based on type CoroutineSuspendExpr which has 5 children:

    SubExprs[SubExpr::Operand] = Operand;
    SubExprs[SubExpr::Common] = Common;
    SubExprs[SubExpr::Ready] = Ready;
    SubExprs[SubExpr::Suspend] = Suspend;
    SubExprs[SubExpr::Resume] = Resume;

With purposes:

• Common = how to get awaitable object, on which is then...
• Ready = Common + .await_ready() on the result
• Suspend = Common + .await_suspend(std::coroutine_handle) call
• Resume = Common + .await_resume() to obtain result
• Operand = original expression, which is in case of co_await

but in case of co_yield Common and Operand are the same subtree with Common sometimes wrapped into MaterializeTemporaryExpr node.