[05/40] command.h: Include scoped_restore_command.h

Message ID 1496406158-12663-6-git-send-email-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves June 2, 2017, 12:22 p.m. UTC
  This file depends on scoped_restore:

  extern scoped_restore_tmpl<int> prevent_dont_repeat (void);

But doesn't include the corresponding header.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* command.h: Include "scoped_restore_command.h".
---
 gdb/command.h | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Yao Qi June 27, 2017, 11:30 a.m. UTC | #1
Pedro Alves <palves@redhat.com> writes:

> This file depends on scoped_restore:
>
>   extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
>
> But doesn't include the corresponding header.

It is a function declaration, so why does it need scoped_restore?

$ make check-headers CHECK_HEADERS="command.h"

The command above doesn't complain anything.  I tried gcc-5, 6, and 7.
  
Pedro Alves June 27, 2017, 11:45 a.m. UTC | #2
On 06/27/2017 12:30 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> This file depends on scoped_restore:
>>
>>   extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
>>
>> But doesn't include the corresponding header.
> 
> It is a function declaration, so why does it need scoped_restore?

I don't understand.  Why wouldn't it?  If the compiler doesn't
know what scoped_restore_tmpl is, how can that compile?

> 
> $ make check-headers CHECK_HEADERS="command.h"
> 
> The command above doesn't complain anything.  I tried gcc-5, 6, and 7.

Odd.  Let me take another look.

I ran into this because at some point I added "#include "command.h"
somewhere, which then failed to compile.

Thanks,
Pedro Alves
  
Pedro Alves June 27, 2017, 11:52 a.m. UTC | #3
On 06/27/2017 12:45 PM, Pedro Alves wrote:
> 
> On 06/27/2017 12:30 PM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>>
>>> This file depends on scoped_restore:
>>>
>>>   extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
>>>
>>> But doesn't include the corresponding header.
>>
>> It is a function declaration, so why does it need scoped_restore?
> 
> I don't understand.  Why wouldn't it?  If the compiler doesn't
> know what scoped_restore_tmpl is, how can that compile?
> 
>>
>> $ make check-headers CHECK_HEADERS="command.h"
>>
>> The command above doesn't complain anything.  I tried gcc-5, 6, and 7.
> 
> Odd.  Let me take another look.

OK, so defs.h includes utils.h, which includes scoped_restore.h,
as found via this hack:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 diff --git c/gdb/command.h w/gdb/command.h
 index aa179e9..f587ebd 100644
 --- c/gdb/command.h
 +++ w/gdb/command.h
 @@ -415,6 +415,8 @@ extern void error_no_arg (const char *) ATTRIBUTE_NORETURN;
 
  extern void dont_repeat (void);
 
 +template <typename T> union scoped_restore_tmpl;
 +
  extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
 
  /* Used to mark commands that don't do anything.  If we just leave the

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/gdb/command.h:418:29: error: ‘union’ tag used in naming ‘struct scoped_restore_tmpl<T>’ [-fpermissive]
 template <typename T> union scoped_restore_tmpl;
                             ^
In file included from src/gdb/utils.h:25:0,
                 from src/gdb/defs.h:785,
                 from <command-line>:0:
src/gdb/common/scoped_restore.h:50:7: note: ‘struct scoped_restore_tmpl<T>’ was previously declared here
 class scoped_restore_tmpl : public scoped_restore_base
       ^
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> I ran into this because at some point I added "#include "command.h"
> somewhere, which then failed to compile.

I don't remember exactly where I added the #include "command.h" that
triggered this.  Must have been somewhere that caused "command.h" to
be included via "defs.h", before the "#include utils.h" line.

Let me try removing it from the series, see if I can still reproduce it.

Thanks,
Pedro Alves
  
Pedro Alves June 27, 2017, 12:02 p.m. UTC | #4
On 06/27/2017 12:52 PM, Pedro Alves wrote:

> I don't remember exactly where I added the #include "command.h" that
> triggered this.  Must have been somewhere that caused "command.h" to
> be included via "defs.h", before the "#include utils.h" line.
> 
> Let me try removing it from the series, see if I can still reproduce it.

Found it.  Patch #11 (https://sourceware.org/ml/gdb-patches/2017-06/msg00023.html)
later does:

 diff --git c/gdb/symtab.h w/gdb/symtab.h
 index 5901cf9..fb08479 100644
 --- c/gdb/symtab.h
 +++ w/gdb/symtab.h
 @@ -25,6 +25,7 @@
  #include "gdbtypes.h"
  #include "common/enum-flags.h"
  #include "common/function-view.h"
 +#include "completer.h"

which without this fix would break building all .o files like this:

 In file included from src/gdb/completer.h:21:0,
                  from src/gdb/symtab.h:28,
                  from src/gdb/language.h:26,
                  from src/gdb/frame.h:72,
                  from src/gdb/gdbarch.h:39,
                  from src/gdb/defs.h:636,
                  from src/gdb/top.c:20:
 src/gdb/command.h:434:8: error: ‘scoped_restore_tmpl’ does not name a type
  extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
         ^
 Makefile:1911: recipe for target 'top.o' failed


because completer.h includes command.h, and that include chain
happens before utils.h is included at the end of defs.h.
I think it'd be good if defs.h didn't pull in so much,
but I think that's orthogonal, and the current fix stands on
its own.

I should have included this info in the original submission.
Sorry about that.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/command.h b/gdb/command.h
index aa179e9..4a56a51 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -19,6 +19,7 @@ 
 #define COMMAND_H 1
 
 #include "gdb_vecs.h"
+#include "common/scoped_restore.h"
 
 /* This file defines the public interface for any code wanting to
    create commands.  */