diff mbox

Add missing includes to gdb_assert.h and gdb_string_view.h

Message ID 20191001183730.214317-1-cbiesinger@google.com
State New, archived
Headers

Commit Message

Terekhov, Mikhail via Gdb-patches Oct. 1, 2019, 6:37 p.m. UTC 
  gdb::string_view uses gdb_assert, so it should include that header.
And gdb_assert uses internal_error, so it should include errors.h.

gdb/ChangeLog:

2019-10-01  Christian Biesinger  <cbiesinger@google.com>

	* gdbsupport/gdb_assert.h: Include errors.h.
	* gdbsupport/gdb_string_view.h: Include gdb_assert.h.
---
 gdb/gdbsupport/gdb_assert.h      | 2 ++
 gdb/gdbsupport/gdb_string_view.h | 1 +
 2 files changed, 3 insertions(+)

Comments

Simon Marchi Oct. 2, 2019, 3:05 a.m. UTC | #1 
On 2019-10-01 2:37 p.m., Christian Biesinger via gdb-patches wrote:
> gdb::string_view uses gdb_assert, so it should include that header.
> And gdb_assert uses internal_error, so it should include errors.h.

It might be that in the past, files like gdb_assert.h just assumed that
they were included as part of defs.h, and that necessary includes were
already included by defs.h.  Not sure.

In any case, I don't really see any downside going with the "include what
you use" route, so this patch would LGTM.  I'm just curious to see if others
have any objection to this, so please wait a bit before merging it.

Simon
Tom Tromey Oct. 2, 2019, 3:56 p.m. UTC | #2 
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> In any case, I don't really see any downside going with the "include what
Simon> you use" route, so this patch would LGTM.  I'm just curious to see if others
Simon> have any objection to this, so please wait a bit before merging it.

It seems fine to me.  A redundant include is pretty harmless overall; I
think the main include problem is a "new" include that isn't needed, as
these add to compilation time.

Tom
Terekhov, Mikhail via Gdb-patches Oct. 2, 2019, 6:34 p.m. UTC | #3 
On Wed, Oct 2, 2019 at 10:56 AM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>
> Simon> In any case, I don't really see any downside going with the "include what
> Simon> you use" route, so this patch would LGTM.  I'm just curious to see if others
> Simon> have any objection to this, so please wait a bit before merging it.
>
> It seems fine to me.  A redundant include is pretty harmless overall; I
> think the main include problem is a "new" include that isn't needed, as
> these add to compilation time.

Thanks, pushed.

I also found https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html
so including the same file twice should be even cheaper than I
thought.

Christian
diff mbox

Patch

diff --git a/gdb/gdbsupport/gdb_assert.h b/gdb/gdbsupport/gdb_assert.h
index a719d87899..6113050320 100644
--- a/gdb/gdbsupport/gdb_assert.h
+++ b/gdb/gdbsupport/gdb_assert.h
@@ -19,6 +19,8 @@ 
 #ifndef COMMON_GDB_ASSERT_H
 #define COMMON_GDB_ASSERT_H
 
+#include "errors.h"
+
 /* A static assertion.  This will cause a compile-time error if EXPR,
    which must be a compile-time constant, is false.  */
 
diff --git a/gdb/gdbsupport/gdb_string_view.h b/gdb/gdbsupport/gdb_string_view.h
index 68f7f7de36..19ae222527 100644
--- a/gdb/gdbsupport/gdb_string_view.h
+++ b/gdb/gdbsupport/gdb_string_view.h
@@ -46,6 +46,7 @@  namespace gdb {
 
 #include <string>
 #include <limits>
+#include "gdb_assert.h"
 
 namespace gdb {