Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18638 )

Change subject: [common] switch to std::optional
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/18638/3/src/kudu/common/column_predicate-test.cc
File src/kudu/common/column_predicate-test.cc:

http://gerrit.cloudera.org:8080/#/c/18638/3/src/kudu/common/column_predicate-test.cc@25
PS3, Line 25: #include <optional>
> do we use optional in this file?
yes: test scenarios in this file are calling ColumnPredicate::InclusiveRange(), 
and the latter returns  optional<ColumnPredicate>


http://gerrit.cloudera.org:8080/#/c/18638/3/src/kudu/common/generic_iterators.cc
File src/kudu/common/generic_iterators.cc:

http://gerrit.cloudera.org:8080/#/c/18638/3/src/kudu/common/generic_iterators.cc@32
PS3, Line 32: #include <type_traits>
> don't we need to include <optional>
Right -- it seems this file needs both <optional> and <type_traits> (I guess 
the latter is needed for things like 'template <class T> struct 
is_reference<T>' that are used by std::optional).


http://gerrit.cloudera.org:8080/#/c/18638/3/src/kudu/common/scan_spec-test.cc
File src/kudu/common/scan_spec-test.cc:

http://gerrit.cloudera.org:8080/#/c/18638/3/src/kudu/common/scan_spec-test.cc@22
PS3, Line 22: #include <optional>
> do we need optional here?
Yep: a test scenario in this file uses ColumnPredicate::InclusiveRange() that 
returns std::optional<ColumnPredicate>.

That's what IWYU outputs when trying to remove the <optional> header:

>>> Fixing #includes in 'src/kudu/common/scan_spec-test.cc'
@@ -19,6 +19,7 @@

 #include <cstdint>
 #include <cstring>
+#include <optional>
 //#include <optional>
 #include <string>
 #include <unordered_map>
IWYU would have edited 1 files on your behalf.



--
To view, visit http://gerrit.cloudera.org:8080/18638
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae320c3ed2487b896539fcbad4425c8480a4b349
Gerrit-Change-Number: 18638
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Thu, 23 Jun 2022 18:57:53 +0000
Gerrit-HasComments: Yes

Reply via email to