Further clarify stream class, and align better with C++ ostream in the process #58
Conversation
mkinsner
commented
Nov 1, 2019
mkinsner
commented
- rename constructor members to be more descriptive
- use flush instead of statement-based descriptions. Flush is the C++ approach to describe/define
- endl implicitly flushes, like C++ convention
- no OpenCL interop on stream, since no appropriate object to map
- clarify synchronization and cross-WI guarantees
- improve wording
…, also align it with C++ iostream semantics
There was a problem hiding this comment.
This looks good, thanks @mkinsner, I just have some minor clarifying comments.
latex/programming_interface.tex
Outdated
| { | ||
| endl | ||
| } | ||
| { | ||
| Outputs a new-line character. | ||
| Outputs a new-line character and then generates a flush. |
There was a problem hiding this comment.
Perhaps we should define the flush operation outside of the table, and then we can reference that operation from endl and flush.
There was a problem hiding this comment.
I've defined it near the beginning of the section, before the table, with the latest commit.
There was a problem hiding this comment.
This looks good now thanks, though we can probably simplify the wording of the flush manipulator since it's defined earlier.
There was a problem hiding this comment.
I did try simplifying the flush manipulator description, but then readded because I thought it was too hard to understand with the distributed wording. I don't really mind either way - do you want me to remove the description from the table?
|
|
||
| /* -- common interface members -- */ | ||
|
|
||
| size_t get_size() const; | ||
|
|
||
| size_t get_work_item_buffer_size() const; |
There was a problem hiding this comment.
Is the intention here to deprecate get_work_item_buffer_size?
There was a problem hiding this comment.
No. Intent is to rename get_max_statement_size() to get_work_item_buffer_size() which is aligned with the constructor terminology. It still returns the value passed as the second constructor arg.
There was a problem hiding this comment.
Perhaps that counts as deprecating, but in a weaker way than removal of a feature.
There was a problem hiding this comment.
I think this makes sense, if by renaming it we at some point remove get_max_statement_size then I would consider this to be a deprecation, simply because it is available in SYCL 1.2.1 but would not be available in a later revision.
To clarify I am fine with this, but if we are deprecating an interface I think we should mark it as deprecated in the specification so our intentions are clear.
There was a problem hiding this comment.
I think I clarified appropriately with the latest commit.
mkinsner
changed the title
…ructor terminology
