Remove using namespace directives in header #1496
Conversation
adamfidel
requested review from
MikeDvorskiy,
dmitriy-sobolev,
SergeyKopienko,
mmichel11 and
danhoeflinger
There was a problem hiding this comment.
LGTM. Thanks for putting this PR up.
@MikeDvorskiy should probably weigh in before merging.
| namespace internal | ||
| { | ||
|
|
||
| enum class search_algorithm |
There was a problem hiding this comment.
Does it really needed to have only one brick struct custom_brick + user visible enum class search_algorithm + parametrizations by their enum values? What about three bricks without enum class search_algorithm at all?
There was a problem hiding this comment.
I'm sorry, but I don't quite follow the question. I have updated the forward declaration of custom_brick to now use a forward declared search_algorithm enum class. Are you suggesting a different approach?
| @@ -24,6 +24,8 @@ | |||
| #ifndef _ONEDPL_SYCL_TRAITS_H | |||
| #define _ONEDPL_SYCL_TRAITS_H | |||
|
|
|||
| #include <oneapi/dpl/internal/binary_search_extension_defs.h> | |||
There was a problem hiding this comment.
I believe it should be a relative patch, via ""
There was a problem hiding this comment.
I have removed this include.
| namespace internal | ||
| { | ||
|
|
||
| enum class search_algorithm |
There was a problem hiding this comment.
Why only a forward declaration is not enough?
Like this?
enum class search_algorithm;
There was a problem hiding this comment.
I am not sure that this enum required at all: see #1496 (comment)
There was a problem hiding this comment.
I did not realize forward declaring enum classes were possible. Thanks for pointing that out. I have removed the changes to the binary search files and just forward declared the enum class.
There was a problem hiding this comment.
OK: forward declaration is enough too.
My idea was to split this brick into three separate bricks and remove this enum.
But I am fine with forward declaration too.
PR #1394 introduced a few
using namespacedirectives in a header, which would pull everything from our internal namespaces to be visible in user code.This PR removes the
usingdirectives by explicitly writing the namespaces and wrapping the forward declarations in their appropriate namespace.