[SYCL] Update validation of annotated_arg/annotated_ptr parameterization

[wangdi4]

Update the validation of annotated_arg/annotated_pointer type parameterization as follows. Given a parameterized annotated_arg<T, PropertyList> or annotated_arg<T, PropertyList>, validate the type from two aspects:

1. For a given T, pointer-specific properties are not allowed if T is not pointer. E.g., buffer_location<N> is not applicable for non pointers.
2. For annotated_arg/annotated_ptr, each of them has its specific properties. E.g. alignment<N> is only supported for anntoated_ptr, while restrict is only supported for annotated_arg. Therefore this PR each property in PropertyList must be supported for annotated_arg (or anntoated_ptr)

[tiwaria1]

I hope in a future update we can make this better? Which property is invalid? Why is it invalid? Not gating on this.

[tiwaria1]

I see better errors are bellow.

[tiwaria1]

Make sure to add some test files.

[steffenlarsen]

Due to the static_assert inside the body, it seems unnecessary to check them in the conditional too. It would make sense if std::conditional didn't evaluate both branches, but it does. However, I believe you can prevent further checks if you move the recursive use of validate_annotated_type to after the static_assert.

That is, I would prefer one of:

Suggested change

	struct validate_annotated_type<
	annotated_arg<T, detail::properties_t<Prop, Props...>>>
	: std::conditional_t<is_valid_property<T, Prop>::value &&
	is_property_value_of_annotated_arg<Prop>::value,
	validate_annotated_type<
	annotated_arg<T, detail::properties_t<Props...>>>,
	std::false_type> {
	static_assert(
	is_valid_property<T, Prop>::value,
	"Property is invalid for the underlying type of annotated_arg.");
	static_assert(is_property_value_of_annotated_arg<Prop>::value,
	"Property is not supported for annotated_arg.");
	};
	struct validate_annotated_type<
	annotated_arg<T, detail::properties_t<Prop, Props...>>>
	: validate_annotated_type<annotated_arg<T, detail::properties_t<Props...>>> {
	static_assert(
	is_valid_property<T, Prop>::value,
	"Property is invalid for the underlying type of annotated_arg.");
	static_assert(is_property_value_of_annotated_arg<Prop>::value,
	"Property is not supported for annotated_arg.");
	};

or

Suggested change

	struct validate_annotated_type<
	annotated_arg<T, detail::properties_t<Prop, Props...>>>
	: std::conditional_t<is_valid_property<T, Prop>::value &&
	is_property_value_of_annotated_arg<Prop>::value,
	validate_annotated_type<
	annotated_arg<T, detail::properties_t<Props...>>>,
	std::false_type> {
	static_assert(
	is_valid_property<T, Prop>::value,
	"Property is invalid for the underlying type of annotated_arg.");
	static_assert(is_property_value_of_annotated_arg<Prop>::value,
	"Property is not supported for annotated_arg.");
	};
	struct validate_annotated_type<
	annotated_arg<T, detail::properties_t<Prop, Props...>>> {
	static_assert(
	is_valid_property<T, Prop>::value,
	"Property is invalid for the underlying type of annotated_arg.");
	static_assert(is_property_value_of_annotated_arg<Prop>::value,
	"Property is not supported for annotated_arg.");
	static constexpr bool value = validate_annotated_type<annotated_arg<T, detail::properties_t<Props...>>>::value;
	};

where (to my knowledge) the latter would stop at the first invalid property.

[steffenlarsen]

Isn't the detail::properties_t<> the only case that hits this? If so, we might as well make that clear in the definition:

template <typename T>
struct validate_annotated_type<annotated_arg<T, detail::properties_t<>>>
    : std::true_type {};

[github-actions]

This pull request is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be automatically closed in 30 days.

[tiwaria1]

Hey @wangdi4 , I think we implemented this later in some other PR, please verify and close this one.