70250 - Generated header files should not contain bogus default implementation

Status: NEW

(Core :: XPCOM, defect, P5)

Description

[Simon Fraser [no longer active]]

Currently, header files generated by xpidl contain template implementations of 
that interface, surrounded by an #if 0 block.

This seems bogus to me. Problems are:
1. Addition header file size slows compilation
2. It's rare that people implement an interface in a class that doesn't implement
   other interfaces, so only the method stubs are useful.

I think we should no generate this extra stuff.

Comment 1

[John Bandhauer]

I think a lot of people find these useful.

Do you have evidence that this slows compilation enough to matter?

I don't understand your point '2'. Which parts are you saying should not be 
there?

Would a compilation switch to turn this off make you happy?

Comment 2

[Simon Fraser [no longer active]]

If you implement an interface in an existing class (as is most often th case), 
you don't need any of:

/* Header file */
class _MYCLASS_ : public nsPICommandUpdater
{
public:
  NS_DECL_ISUPPORTS
  NS_DECL_NSPICOMMANDUPDATER

  _MYCLASS_();
  virtual ~_MYCLASS_();
  /* additional members */
};

/* Implementation file */
NS_IMPL_ISUPPORTS1(_MYCLASS_, nsPICommandUpdater)

_MYCLASS_::_MYCLASS_()
{
  NS_INIT_ISUPPORTS();
  /* member initializers and constructor code */
}

_MYCLASS_::~_MYCLASS_()
{
  /* destructor code */
}

I also note that the spacing between the '  NS_INIT_ISUPPORTS();' line (2 space 
indent) is inconsistent with the other method stubs (4 space indent).

Am I the only one who objects to these implementation stubs?

Comment 3

[Brendan Eich [:brendan]]

I agree with sfraser.  xpidl should do one thing well in each mode.  If there is
a "prototype or sketch some code for me" mode, it should be enabled by another
option.

/be

Comment 4

[Phil Schwartau]

Mass-reassigning mccabe's non-JS, non-Rhino bugs to jband (34 total). 

Would like to cc mccabe; but the mass-reassign page does not allow this. 
I'll leave it up to mccabe to decide if he wants to be cc'ed on these - 

Comment 5

[John Bandhauer]

mass reassign of xpidl bugs to [e-mail]

Comment 6

[David Bradley]

Just to summarize so that I don't have to trudge through all the text whenever I 
read this bug. The ideal solution is for XPIDL to have an option to turn on 
generation of the template implementations and default to not generating them. 
Please comment if this is not correct.

Comment 7

[John Bandhauer]

Ideal to whom? I *like* there always being a place from which I can copy and 
paste the *correct* declaration part of method implmentations. I *wish* more 
people used this. I don't support what I think brendan said about adding another 
'mode' if this means another xpidl backend generating into another file in 
another pass.

The argument for not marking this bug WONTFIX would be stronger if someone 
showed timing numbers of builds actually being slowed by this text.

Comment 8

[David Bradley]

I wasn't thinking of another mode, just another command line option that would 
only apply to the header mode.

I agree with you that the templates are valuable, I believe jst said he used 
them, I'm sure there are others. That's why I thought the idea of the command 
line option might satisfy both parties.

I agree with you that it its not worth doing unless, as you stated, someone can 
show the abscence of the templates would have any substantial impact on build 
times.

I was just summarizing a comprimise solution if it was ever proven that it was 
worth doing.

Comment 9

[basic]

Changing platform

Comment 10

[Marek Stępień [:marcoos, inactive]]

*** Bug 166507 has been marked as a duplicate of this bug. ***

Comment 11

[Michael Kohler [:mkohler]]

This is a mass change. Every comment has "assigned-to-new" in it.

I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.