[PATCH] D86233: [LangRef] Define mustprogress attribute

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/docs/LangRef.rst:1963
+    Standard <http://eel.is/c++draft/intro.progress>`_. This attribute does
+    not apply transitively to callees. This is implied by `willreturn`.
 
----------------
nikic wrote:
> jdoerfert wrote:
> > ```
> > This attribute indicates that the function is required to return, unwind,
> > or interact with the environment in an observable way e.g. via a
> > volatile memory access, I/O, or other synchronization. 
> > The ``mustprogress`` attribute is intended to model the
> > requirements of the first section of `[intro.progress] of the C++
> > Standard <http://eel.is/c++draft/intro.progress>`_. This attribute does
> > not apply transitively to callees, that means a callee might not make progress.
> > Note that `willreturn` implies `mustprogress`.
> > ```
> Some notes from my side:
>  * I'm not sure how to interpret the "does not apply to callees" part. If mustprogress function A calls not-mustprogress function B and B goes into an infinite loop, then A will not return, unwind or interact with the environment, thus violating the mustprogress contract. Now, the callee does not necessarily have to be mustprogress in general, but I believe it needs to progress for any invocation from a mustprogress function -- or not?
>  * I would explicitly state that violating the progress requirement results in undefined behavior.
>  * It may be worthwhile to explicitly mention that a non-interacting loop inside a mustprogress function can thus be assumed to terminate and may be removed. That would help clarify the practical effect setting this attribute has.
> I'm not sure how to interpret the "does not apply to callees" part. If mustprogress function A calls not-mustprogress function B and B goes into an infinite loop, then A will not return, unwind or interact with the environment, thus violating the mustprogress contract. Now, the callee does not necessarily have to be mustprogress in general, but I believe it needs to progress for any invocation from a mustprogress function -- or not?

Yes. That is the intention. If we come from the perspective that C++ has the mustprogress requirement, it has to hold for any statement, including a call to a function that may not progress. However, the statement (=call) has to make progress from the C++ side. This is the same as other attributes, let's say `readonly` (aka. pure). A function can have that attribute and call an arbitrary function. This is all good as long as the arbitrary function is readonly for this call site.

Do you have an idea how to clarify this in the wording? (Even though other attributes implicitly make this assumption as well.)


> I would explicitly state that violating the progress requirement results in undefined behavior.

Sure. @atmnpatel 

> It may be worthwhile to explicitly mention that a non-interacting loop inside a mustprogress function can thus be assumed to terminate and may be removed. That would help clarify the practical effect setting this attribute has.

Sounds good to me. @atmnpatel 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86233/new/

https://reviews.llvm.org/D86233