SYCL header are not "parsable"

[TApplencourt]

So our header is not "parsable" (sorry, I don't know what the correct term is. "give no error when using -fsyntax-only" is quite long).

For example, using stream.h (#304 )

tapplencourt@jlselogin7:~/Specification/adoc/headers> g++ -fsyntax-only stream.h
stream.h:49:1: error: ‘__precision_manipulator__’ does not name a type
   49 | __precision_manipulator__ setprecision(int precision);
      | ^~~~~~~~~~~~~~~~~~~~~~~~~
stream.h:51:1: error: ‘__width_manipulator__’ does not name a type
   51 | __width_manipulator__ setw(int width);
      | ^~~~~~~~~~~~~~~~~~~~~
stream.h:56:16: error: expected ‘)’ before ‘totalBufferSize’
   56 |   stream(size_t totalBufferSize, size_t workItemBufferSize, handler& cgh,
      |         ~      ^~~~~~~~~~~~~~~~
      |                )
stream.h:57:44: error: expected unqualified-id before ‘)’ token
   57 |          const property_list &propList = {});
      |                                            ^
stream.h:63:3: error: ‘size_t’ does not name a type
   63 |   size_t size() const noexcept;
      |   ^~~~~~
stream.h:1:1: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
  +++ |+#include <cstddef>
    1 | // Copyright (c) 2011-2021 The Khronos Group, Inc.
stream.h:66:3: error: ‘size_t’ does not name a type
   66 |   size_t get_size() const;
      |   ^~~~~~
stream.h:66:3: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
stream.h:68:3: error: ‘size_t’ does not name a type
   68 |   size_t get_work_item_buffer_size() const;
      |   ^~~~~~
stream.h:68:3: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?
stream.h:73:3: error: ‘size_t’ does not name a type
   73 |   size_t get_max_statement_size() const;
      |   ^~~~~~
stream.h:73:3: note: ‘size_t’ is defined in header ‘<cstddef>’; did you forget to ‘#include <cstddef>’?

I think we may want to fix that. It makes tools that use the header to generate stuff harder to use. For example, at ALCF, we develop a tool that parses header files to generate tracepoints. And the 'header -> ast' transformation fails if not everything is well defined.

PS: __precision_manipulator__ and __width_manipulator__ doesn't seem to be defined in the spec or by C++

[Pennycook]

PS: precision_manipulator and width_manipulator doesn't seem to be defined in the spec or by C++

I think the intent is that anything named __foo__ is an implementation detail. Most of the other instances in the specification use __unspecified__ instead of providing a name, but there are some providing a name (e.g. __value_type__) and some using both conventions (e.g. __unspecified_callable__).

If the specification consistently used __unspecified__ everywhere, would that be easier? Maybe then you could just define __unspecified__ to some dummy type in an include file you pass to the compiler:

// make_sycl_headers_parseable.h
struct __unspecified__ {};

g++ --include make_sycl_headers_parsable.h stream.h --fsyntax-only

[gmlueck]

FWIW, I've been using a comment of the form /*unspecified*/ to indicate a type that is unspecified, so there is yet another style in our headers. I choose that style because it is the same way that cppreference does it (one example here).

[TApplencourt]

I think the intent is that anything named foo is an implementation detail.

Oh, thanks, I didn't realize!

@nliber Pointed me to http://eel.is/c++draft/map.overview. The c++ spec uses italics. (aka /* */ in markdown). So I tend to agree with @gmlueck that we should use the /*foo*/. But that may clash with some comments in the headers if we want to have tool who parse the spec

If the specification consistently used unspecified everywhere, would that be easier?

Not really. Doing a sed on /*\w+/* or __\w+__ is not hard as a preprocessing step (minus distinguishing comments versus SYCL type). And we may need multiple types. Indeed some of the __foo__ have template arguments like insert-return-type<iterator, node_type>; (in Nevin's example).

So the dummy struct trick will need to be generalized.

If you think it's useful (at least it should cache any typo...), I can try carving a little time to see if we can make the header "parseable" and unify our coding style.

[gmlueck]

we should use the /*foo*/. But that may clash with some comments in the headers...

Why do we need to say anything other than /*unspecified*/? I don't think we need a general form of /*foo*/ where "foo" can be any word.

[TApplencourt]

Why do we need to say anything other than /unspecified/? I don't think we need a general form of /foo/ where "foo" can be any word.

Oh, then it works for me. The C++ spec uses multiple specifiers, and we are doing the same right now. But if we can go away with one, it may make stuff easier (at least for the parsing)

[TApplencourt]

Some quick "grep" results ( and I learned about -o):

$ grep -r -h "* unspecified"  * | wc -l # Counting the /* unspecified */
30
$ grep -r -h "*unspecified"  * | wc -l
2
$ grep -r -h -o  __[A-Za-z_]*__ * | wc -l  # Counting  the `__foo__`
68
$ grep -r -h -o  __[A-Za-z_]*__ * | sort -u
__pointer_class__
__precision_manipulator__
__swizzled_vec__
__SYCL_DEVICE_ONLY__
___unspecified___
__unspecified__
__unspecified_callable__
__unspecified_iterator__
__value_type__
__width_manipulator__

So Moving to /* unspecified */ may improve the style coherency of your header. I can make a PR to replace all of those (except __SYCL_DEVICE_ONLY__ of course).

Now regard to making the header parseable, I did some tests.

• It Sounds doable.
• I found some possible double definitions of some classes (for example, kernel_bundle.h and kernelClass.h both define class kernel ), so it may be a worthwhile effort to pursue.