[SYCL] Specific error messages for invalid properties on non pointer types (intel#11843)

This change adds specific error messages for invalid properties that are
specified on non pointer types. This change is directly in the header
annotated_arg.hpp and these checks are done before the
`check_property_list` check which previously accounted for this but had
a generic error message.

This change adds static asserts so that the error message specifies the
invalid property and that it is invalid due to a non pointer type for
the underlying data type.

For example, if the user tries to declare the following variable: 

```cpp
annotated_arg<int, decltype(properties{buffer_location<0>})> a;
```

The following error message is outputted: 

```
error: static assertion failed due to requirement '!has_buffer_location': Property buffer_location cannot be specified for annotated_arg<T> when T is a non pointer type.
  288 |   static_assert(!has_buffer_location,
      |                 ^~~~~~~~~~~~~~~~~~~~
```

Prior to this change, the following generic error message was printed
with multiple notes:

```
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/oneapi/experimental/common_annotated_properties/properties.hpp:59:17: error: static assertion failed due to requirement 'is_valid_property_for_given_type': Property is invalid for the given type.
   59 |   static_assert(is_valid_property_for_given_type,
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:307:7: note: in instantiation of template class 'sycl::ext::oneapi::experimental::check_property_list<int, sycl::ext::oneapi::experimental::property_value<sycl::ext::intel::experimental::buffer_location_key, std::integral_constant<int, 0>>>' requested here
  307 |       check_property_list<T, Props...>::value;
      |       ^
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:308:17: note: in instantiation of static data member 'sycl::ext::oneapi::experimental::annotated_arg<int, sycl::ext::oneapi::experimental::properties<std::tuple<sycl::ext::oneapi::experimental::property_value<sycl::ext::intel::experimental::buffer_location_key, std::integral_constant<int, 0>>>>>::contains_valid_properties' requested here
  308 |   static_assert(contains_valid_properties,
      |                 ^
temp.cpp:15:66: note: in instantiation of template class 'sycl::ext::oneapi::experimental::annotated_arg<int, sycl::ext::oneapi::experimental::properties<std::tuple<sycl::ext::oneapi::experimental::property_value<sycl::ext::intel::experimental::buffer_location_key, std::integral_constant<int, 0>>>>>' requested here
   15 |     annotated_arg<int, decltype(properties{buffer_location<0>})> a;
      |                                                                  ^
```

### Alternate Methods

PR intel#11748 adds the same checks but in
the `fpga_annotated_properties.hpp` file as a separate check. This PR
has the checks directly in the `annotated_arg.hpp` header file so that
there are fewer notes printed below the error message. Here is a
comparison of how the error message is outputted when the checks are in
different files:

With error checks in properties header file and using a macro (PR
intel#11748):

```
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/intel/experimental/fpga_annotated_properties.hpp:396:3: error: static assertion failed due to requirement '!has_buffer_location': Property buffer_location cannot be specified for annotated_arg<T> when T is a non pointer type.
  396 |   CHECK_INVALID_PROPERTY(buffer_location, list)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/intel/experimental/fpga_annotated_properties.hpp:389:17: note: expanded from macro 'CHECK_INVALID_PROPERTY'
  389 |   static_assert(!has_##property,                                               \
      |                 ^~~~~~~~~~~~~~~
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:202:25: note: in instantiation of template class 'sycl::ext::oneapi::experimental::detail::checkPropertiesForNonPointerType<sycl::ext::oneapi::experimental::property_value<sycl::ext::intel::experimental::buffer_location_key, std::integral_constant<int, 0>>>' requested here
  202 |   static_assert(detail::checkPropertiesForNonPointerType<Props...>::value);
      |                         ^
temp.cpp:15:66: note: in instantiation of template class 'sycl::ext::oneapi::experimental::annotated_arg<int, sycl::ext::oneapi::experimental::properties<std::tuple<sycl::ext::oneapi::experimental::property_value<sycl::ext::intel::experimental::buffer_location_key, std::integral_constant<int, 0>>>>>' requested here
   15 |     annotated_arg<int, decltype(properties{buffer_location<0>})> a;
      |                                                                  ^
```

With error check in annotated_arg header and using a macro: 

```
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:292:3: error: static assertion failed due to requirement '!has_buffer_location': Property buffer_location cannot be specified for annotated_arg<T> when T is a non pointer type.
  292 |   CHECK_INVALID_PROPERTY(buffer_location, property_tuple)
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:23:17: note: expanded from macro 'CHECK_INVALID_PROPERTY'
   23 |   static_assert(!has_##property,                                               \
      |                 ^~~~~~~~~~~~~~~
temp.cpp:15:66: note: in instantiation of template class 'sycl::ext::oneapi::experimental::annotated_arg<int, sycl::ext::oneapi::experimental::properties<std::tuple<sycl::ext::oneapi::experimental::property_value<sycl::ext::intel::experimental::buffer_location_key, std::integral_constant<int, 0>>>>>' requested here
   15 |     annotated_arg<int, decltype(properties{buffer_location<0>})> a;
      |                                                                  ^
```

### This Change

With error check in annotated_arg header and not using a macro (This
PR):

```
/p/psg/swip/w/yugteshs/sycl_include/sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:288:17: error: static assertion failed due to requirement '!has_buffer_location': Property buffer_location cannot be specified for annotated_arg<T> when T is a non pointer type.
  288 |   static_assert(!has_buffer_location,
      |                 ^~~~~~~~~~~~~~~~~~~~
temp.cpp:15:66: note: in instantiation of template class 'sycl::ext::oneapi::experimental::annotated_arg<int, sycl::ext::oneapi::experimental::properties<std::tuple<sycl::ext::oneapi::experimental::property_value<sycl::ext::intel::experimental::buffer_location_key, std::integral_constant<int, 0>>>>>' requested here
   15 |     annotated_arg<int, decltype(properties{buffer_location<0>})> a;
      |                                                                  ^
 ```
 
 This version only prints one note and therefore it is more clear for the user.

Author: yug-intel

sycl/include/sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp

@@ -10,6 +10,7 @@
 
 #include <sycl/detail/defines.hpp>
 #include <sycl/ext/intel/experimental/fpga_annotated_properties.hpp>
+#include <sycl/ext/oneapi/experimental/annotated_ptr/annotated_ptr_properties.hpp>
 #include <sycl/ext/oneapi/experimental/common_annotated_properties/properties.hpp>
 #include <sycl/ext/oneapi/properties/properties.hpp>
 
@@ -79,26 +80,6 @@ __SYCL_TYPE(annotated_arg) annotated_arg<T *, detail::properties_t<Props...>> {
 #endif
 
 public:
-  static constexpr bool is_valid_property_list =
-      is_property_list<property_list_t>::value;
-  static_assert(is_valid_property_list, "Property list is invalid.");
-  static constexpr bool contains_valid_properties =
-      check_property_list<T *, Props...>::value;
-  static_assert(contains_valid_properties,
-                "The property list contains invalid property.");
-  // check the set if FPGA specificed properties are used
-  static constexpr bool hasValidFPGAProperties =
-      detail::checkValidFPGAPropertySet<Props...>::value;
-  static_assert(hasValidFPGAProperties,
-                "FPGA Interface properties (i.e. awidth, dwidth, etc.)"
-                "can only be set with BufferLocation together.");
-  // check if conduit and register_map properties are specified together
-  static constexpr bool hasConduitAndRegisterMapProperties =
-      detail::checkHasConduitAndRegisterMap<Props...>::value;
-  static_assert(hasConduitAndRegisterMapProperties,
-                "The properties conduit and register_map cannot be "
-                "specified at the same time.");
-
   annotated_arg() noexcept = default;
   annotated_arg(const annotated_arg &) = default;
   annotated_arg &operator=(annotated_arg &) = default;
@@ -185,6 +166,34 @@ __SYCL_TYPE(annotated_arg) annotated_arg<T *, detail::properties_t<Props...>> {
   template <typename PropertyT> static constexpr auto get_property() {
     return property_list_t::template get_property<PropertyT>();
   }
+
+  // *************************************************************************
+  // All static error checking is added here instead of placing inside neat
+  // functions to minimize the number lines printed out when an assert
+  // is triggered.
+  // static constexprs are used to ensure that the triggered assert prints
+  // a message that is very readable. Without these, the assert will
+  // print out long templated names
+  // *************************************************************************
+  static constexpr bool is_valid_property_list =
+      is_property_list<property_list_t>::value;
+  static_assert(is_valid_property_list, "Property list is invalid.");
+  static constexpr bool contains_valid_properties =
+      check_property_list<T *, Props...>::value;
+  static_assert(contains_valid_properties,
+                "The property list contains invalid property.");
+  // check the set if FPGA specificed properties are used
+  static constexpr bool hasValidFPGAProperties =
+      detail::checkValidFPGAPropertySet<Props...>::value;
+  static_assert(hasValidFPGAProperties,
+                "FPGA Interface properties (i.e. awidth, dwidth, etc.) "
+                "can only be set with BufferLocation together.");
+  // check if conduit and register_map properties are specified together
+  static constexpr bool hasConduitAndRegisterMapProperties =
+      detail::checkHasConduitAndRegisterMap<Props...>::value;
+  static_assert(hasConduitAndRegisterMapProperties,
+                "The properties conduit and register_map cannot be "
+                "specified at the same time.");
 };
 
 // Partial specialization for non-pointer type
@@ -206,28 +215,6 @@ __SYCL_TYPE(annotated_arg) annotated_arg<T, detail::properties_t<Props...>> {
 #endif
 
 public:
-  static constexpr bool is_device_copyable = is_device_copyable_v<T>;
-  static_assert(is_device_copyable, "Type T must be device copyable.");
-  static constexpr bool is_valid_property_list =
-      is_property_list<property_list_t>::value;
-  static_assert(is_valid_property_list, "Property list is invalid.");
-  static constexpr bool contains_valid_properties =
-      check_property_list<T, Props...>::value;
-  static_assert(contains_valid_properties,
-                "The property list contains invalid property.");
-  // check the set if FPGA specificed properties are used
-  static constexpr bool hasValidFPGAProperties =
-      detail::checkValidFPGAPropertySet<Props...>::value;
-  static_assert(hasValidFPGAProperties,
-                "FPGA Interface properties (i.e. awidth, dwidth, etc.)"
-                "can only be set with BufferLocation together.");
-  // check if conduit and register_map properties are specified together
-  static constexpr bool hasConduitAndRegisterMapProperties =
-      detail::checkHasConduitAndRegisterMap<Props...>::value;
-  static_assert(hasConduitAndRegisterMapProperties,
-                "The properties conduit and register_map cannot be "
-                "specified at the same time.");
-
   annotated_arg() noexcept = default;
   annotated_arg(const annotated_arg &) = default;
   annotated_arg &operator=(annotated_arg &) = default;
@@ -377,6 +364,82 @@ __SYCL_TYPE(annotated_arg) annotated_arg<T, detail::properties_t<Props...>> {
   R operator<<(const annotated_arg<T2, PropertyList2> &other) const {
     return obj << other.obj;
   }
+
+  // *************************************************************************
+  // All static error checking is added here instead of placing inside neat
+  // functions to minimize the number lines printed out when an assert
+  // is triggered.
+  // static constexprs are used to ensure that the triggered assert prints
+  // a message that is very readable. Without these, the assert will
+  // print out long templated names
+  // *************************************************************************
+  static constexpr bool is_device_copyable = is_device_copyable_v<T>;
+  static_assert(is_device_copyable, "Type T must be device copyable.");
+
+  // check if invalid properties are specified for non pointer type
+  static constexpr bool has_buffer_location =
+      has_property<buffer_location_key>();
+  static_assert(!has_buffer_location,
+                "Property buffer_location cannot be specified for "
+                "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool has_awidth = has_property<awidth_key>();
+  static_assert(!has_awidth, "Property awidth cannot be specified for "
+                             "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool has_dwidth = has_property<dwidth_key>();
+  static_assert(!has_dwidth, "Property dwidth cannot be specified for "
+                             "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool has_latency = has_property<latency_key>();
+  static_assert(!has_latency, "Property latency cannot be specified for "
+                              "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool has_read_write_mode =
+      has_property<read_write_mode_key>();
+  static_assert(!has_read_write_mode,
+                "Property read_write_mode cannot be specified for "
+                "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool has_maxburst = has_property<maxburst_key>();
+  static_assert(!has_maxburst,
+                "Property maxburst cannot be specified for "
+                "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool has_wait_request = has_property<wait_request_key>();
+  static_assert(!has_wait_request,
+                "Property wait_request cannot be specified for "
+                "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool has_alignment = has_property<alignment_key>();
+  static_assert(!has_alignment,
+                "Property alignment cannot be specified for "
+                "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool has_usm_kind = has_property<usm_kind_key>();
+  static_assert(!has_usm_kind,
+                "Property usm_kind cannot be specified for "
+                "annotated_arg<T> when T is a non pointer type.");
+
+  static constexpr bool is_valid_property_list =
+      is_property_list<property_list_t>::value;
+  static_assert(is_valid_property_list, "Property list is invalid.");
+  static constexpr bool contains_valid_properties =
+      check_property_list<T, Props...>::value;
+  static_assert(contains_valid_properties,
+                "The property list contains invalid property.");
+  // check the set if FPGA specificed properties are used
+  static constexpr bool hasValidFPGAProperties =
+      detail::checkValidFPGAPropertySet<Props...>::value;
+  static_assert(hasValidFPGAProperties,
+                "FPGA Interface properties (i.e. awidth, dwidth, etc.) "
+                "can only be set with BufferLocation together.");
+  // check if conduit and register_map properties are specified together
+  static constexpr bool hasConduitAndRegisterMapProperties =
+      detail::checkHasConduitAndRegisterMap<Props...>::value;
+  static_assert(hasConduitAndRegisterMapProperties,
+                "The properties conduit and register_map cannot be "
+                "specified at the same time.");
 };
 
 template <typename T, typename PropertyList, typename T2,

sycl/include/sycl/ext/oneapi/experimental/annotated_ptr/annotated_ptr.hpp

@@ -215,26 +215,6 @@ __SYCL_TYPE(annotated_ptr) annotated_ptr<T, detail::properties_t<Props...>> {
 #endif
 
 public:
-  static constexpr bool is_valid_property_list =
-      is_property_list<property_list_t>::value;
-  static_assert(is_valid_property_list, "Property list is invalid.");
-  static constexpr bool contains_valid_properties =
-      check_property_list<T *, Props...>::value;
-  static_assert(contains_valid_properties,
-                "The property list contains invalid property.");
-  // check the set if FPGA specificed properties are used
-  static constexpr bool hasValidFPGAProperties =
-      detail::checkValidFPGAPropertySet<Props...>::value;
-  static_assert(hasValidFPGAProperties,
-                "FPGA Interface properties (i.e. awidth, dwidth, etc.)"
-                "can only be set with BufferLocation together.");
-  // check if conduit and register_map properties are specified together
-  static constexpr bool hasConduitAndRegisterMapProperties =
-      detail::checkHasConduitAndRegisterMap<Props...>::value;
-  static_assert(hasConduitAndRegisterMapProperties,
-                "The properties conduit and register_map cannot be "
-                "specified at the same time.");
-
   annotated_ptr() noexcept = default;
   annotated_ptr(const annotated_ptr &) = default;
   annotated_ptr &operator=(const annotated_ptr &) = default;
@@ -356,6 +336,34 @@ __SYCL_TYPE(annotated_ptr) annotated_ptr<T, detail::properties_t<Props...>> {
   template <typename PropertyT> static constexpr auto get_property() {
     return property_list_t::template get_property<PropertyT>();
   }
+
+  // *************************************************************************
+  // All static error checking is added here instead of placing inside neat
+  // functions to minimize the number lines printed out when an assert
+  // is triggered.
+  // static constexprs are used to ensure that the triggered assert prints
+  // a message that is very readable. Without these, the assert will
+  // print out long templated names
+  // *************************************************************************
+  static constexpr bool is_valid_property_list =
+      is_property_list<property_list_t>::value;
+  static_assert(is_valid_property_list, "Property list is invalid.");
+  static constexpr bool contains_valid_properties =
+      check_property_list<T *, Props...>::value;
+  static_assert(contains_valid_properties,
+                "The property list contains invalid property.");
+  // check the set if FPGA specificed properties are used
+  static constexpr bool hasValidFPGAProperties =
+      detail::checkValidFPGAPropertySet<Props...>::value;
+  static_assert(hasValidFPGAProperties,
+                "FPGA Interface properties (i.e. awidth, dwidth, etc.) "
+                "can only be set with BufferLocation together.");
+  // check if conduit and register_map properties are specified together
+  static constexpr bool hasConduitAndRegisterMapProperties =
+      detail::checkHasConduitAndRegisterMap<Props...>::value;
+  static_assert(hasConduitAndRegisterMapProperties,
+                "The properties conduit and register_map cannot be "
+                "specified at the same time.");
 };
 
 } // namespace experimental

sycl/include/sycl/ext/oneapi/experimental/annotated_ptr/annotated_ptr_properties.hpp

@@ -9,6 +9,7 @@
 
 #pragma once
 
+#include <sycl/ext/oneapi/experimental/common_annotated_properties/properties.hpp>
 #include <sycl/ext/oneapi/properties/properties.hpp> // for properties_t
 #include <sycl/usm/usm_enums.hpp>
 

sycl/test/extensions/annotated_arg/annotated_arg_negative.cpp

@@ -1,4 +1,4 @@
-// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple -fsyntax-only -Xclang -verify -Xclang -verify-ignore-unexpected=note,warning %s
+// RUN: %clangxx -fsycl -ferror-limit=0 -fsycl-targets=%sycl_triple -fsyntax-only -Xclang -verify -Xclang -verify-ignore-unexpected=note,warning %s
 
 #include "sycl/sycl.hpp"
 #include <sycl/ext/intel/fpga_extensions.hpp>
@@ -17,7 +17,42 @@ void check_conduit_and_register_map_properties() {
   annotated_ptr<int, decltype(properties{conduit, register_map})> c;
 }
 
+void check_invalid_properties_on_non_pointer_types() {
+  // check buffer location property specified on non pointer type
+  // expected-error@sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:* {{Property buffer_location cannot be specified for annotated_arg<T> when T is a non pointer type.}}
+  annotated_arg<int, decltype(properties{buffer_location<0>})> a;
+
+  // check awidth property specified on non pointer type
+  // expected-error@sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:* {{Property awidth cannot be specified for annotated_arg<T> when T is a non pointer type.}}
+  annotated_arg<int, decltype(properties{awidth<32>})> b;
+
+  // check dwidth property specified on non pointer type
+  // expected-error@sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:* {{Property dwidth cannot be specified for annotated_arg<T> when T is a non pointer type.}}
+  annotated_arg<int, decltype(properties{dwidth<32>})> c;
+
+  // check latency property specified on non pointer type
+  // expected-error@sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:* {{Property latency cannot be specified for annotated_arg<T> when T is a non pointer type.}}
+  annotated_arg<int, decltype(properties{latency<1>})> d;
+
+  // check read_write_mode property specified on non pointer type
+  // expected-error@sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:* {{Property read_write_mode cannot be specified for annotated_arg<T> when T is a non pointer type.}}
+  annotated_arg<int, decltype(properties{read_write_mode_readwrite})> e;
+
+  // check maxburst property specified on non pointer type
+  // expected-error@sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:* {{Property maxburst cannot be specified for annotated_arg<T> when T is a non pointer type.}}
+  annotated_arg<int, decltype(properties{maxburst<1>})> f;
+
+  // check wait_request property specified on non pointer type
+  // expected-error@sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:* {{Property wait_request cannot be specified for annotated_arg<T> when T is a non pointer type.}}
+  annotated_arg<int, decltype(properties{wait_request_requested})> g;
+
+  // check alignment property specified on non pointer type
+  // expected-error@sycl/ext/oneapi/experimental/annotated_arg/annotated_arg.hpp:* {{Property alignment cannot be specified for annotated_arg<T> when T is a non pointer type.}}
+  annotated_arg<int, decltype(properties{alignment<256>})> h;
+}
+
 int main() {
+  check_invalid_properties_on_non_pointer_types();
   check_conduit_and_register_map_properties();
   return 0;
 }

sycl/test/extensions/annotated_arg/annotated_arg_properties.cpp

@@ -46,15 +46,6 @@ template <typename T> void checkIsPropertyOf() {
       is_property_value_of<decltype(wait_request_requested), T>::value);
 }
 
-// Checks is_property_key_of and is_property_value_of are false for non-pointer
-// type T.
-template <typename T> void checkIsValidPropertyOfNonPtr() {
-  static_assert(
-      is_valid_property<T, decltype(wait_request_not_requested)>::value ==
-      false);
-  static_assert(is_valid_property<T, decltype(latency<8>)>::value == false);
-}
-
 int main() {
   static_assert(is_property_key<register_map_key>::value);
   static_assert(is_property_key<buffer_location_key>::value);
@@ -90,7 +81,5 @@ int main() {
   static_assert(AnnotatedArg4.get_property<read_write_mode_key>() ==
                 read_write_mode_read);
 
-  // Check if a property is valid for a given type
-  checkIsValidPropertyOfNonPtr<A>();
   return 0;
 }