Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/7053 )
Change subject: KUDU-2191 (2/n): Hive Metastore client ...................................................................... Patch Set 30: (1 comment) http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h File src/kudu/hms/mini_hms.h: http://gerrit.cloudera.org:8080/#/c/7053/29/src/kudu/hms/mini_hms.h@30 PS29, Line 30: #include "kudu/util/subprocess.h" > unique_ptr doesn't work with forward declared classes. You sure about that? I'm looking at https://stackoverflow.com/questions/6012157/is-stdunique-ptrt-required-to-know-the-full-definition-of-t and it suggests that, for most operations incomplete knowledge of the class is OK. I also wrote a small test that's similar to mini_hms to prove this to myself: --bar.cc-- #include "bar.h" #include "foo.h" Bar::~Bar() {} void Bar::CreateFoo() { f.reset(new Foo()); f->a = "hello world"; } --test.cc-- #include "bar.h" #include "foo.h" int main(void) { Bar b; b.CreateFoo(); return 0; } --bar.h-- #include <memory> struct Foo; struct Bar { Bar() = default; ~Bar(); void CreateFoo(); private: std::unique_ptr<Foo> f; }; --foo.h-- #include <string> struct Foo { std::string a; }; This test compiles and links. As you can see, bar.h has only partial knowledge of Foo, but full knowledge of it where needed (bar.cc and test.cc). Also, if I follow Alexey's advice and move Bar's noarg constructor from bar.h to bar.cc, I can remove the inclusion of foo.h from test.cc. -- To view, visit http://gerrit.cloudera.org:8080/7053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee Gerrit-Change-Number: 7053 Gerrit-PatchSet: 30 Gerrit-Owner: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Mon, 23 Oct 2017 19:47:45 +0000 Gerrit-HasComments: Yes
