Generated header files should not contain bogus default implementation

NEW
Unassigned

Status

()

Core
XPCOM
P5
normal
17 years ago
8 years ago

People

(Reporter: Simon Fraser, Unassigned)

Tracking

Trunk
Future
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

17 years ago
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

17 years ago
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?
(Reporter)

Comment 2

17 years ago
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?
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

17 years ago
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 - 
Assignee: mike+mozilla → jband

Comment 5

17 years ago
mass reassign of xpidl bugs to dbradley@netscape.com
Assignee: jband → dbradley

Updated

17 years ago
Target Milestone: --- → Future

Updated

17 years ago
Status: NEW → ASSIGNED
Priority: -- → P5

Comment 6

17 years ago
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

17 years ago
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

17 years ago
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

17 years ago
Changing platform
Hardware: All → Macintosh

Updated

17 years ago
OS: Mac System 8.5 → All
Hardware: Macintosh → All
*** Bug 166507 has been marked as a duplicate of this bug. ***

Updated

9 years ago
Component: xpidl → XPCOM
QA Contact: jband_mozilla → xpcom

Updated

8 years ago
Assignee: dbradley → nobody
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.
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.