Skip to content

Conversation

@Fznamznon
Copy link
Contributor

No description provided.

@Fznamznon Fznamznon requested a review from asavonic August 30, 2019 09:40
bader
bader previously approved these changes Sep 13, 2019
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow this comment.
Can we define type alias as using type = const SYCL_GLOBAL_AS ElementType;?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think there is something to improve in the specification?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow this comment.
Can we define type alias as using type = const SYCL_GLOBAL_AS ElementType;?

No. I didn't add the const qualifier here because multi_ptr interface contains methods and operators which take/return pure ElementType parameter without qualifiers and address spaces. If I use the const qualifier here, I will be forced to add const_casts to these methods to remove const qualifier from underlying pointer type. So adding const qualifier here seems unnecessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think there is something to improve in the specification?

Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
@Fznamznon Fznamznon force-pushed the private/mpodchis/constas branch 2 times, most recently from 0e5acba to 814fdd3 Compare September 16, 2019 08:32
@Fznamznon
Copy link
Contributor Author

Rebased, applied suggestion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants