453491 - trunk calendar link failure looking for calDateTime::PostCreatePrototype

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: linkage fix, v1

This is the result of some XPConnect changes on trunk.  I haven't actually tested the calendar functionality with the attached patch, but it un-breaks the build, which is a significant step forward.

Comment 1

[Mark Banner (:standard8)]

If you want to commit this so it works for comm-central and leaves cvs alone, then you could change the ifdef to COMM_BUILD, and in the Makefile.in add a section:

ifdef COMM_BUILD
DEFINES += COMM_BUILD
endif

The Sunbird cvs trunk would still compile, as would TB + lightning from comm-central.

Comment 2

[Daniel Boelzle [:dbo]]

Comment on attachment 336681 [details] [diff] [review]
linkage fix, v1

stealing review, r=dbo

You could avoid a warning omitting the (unused) parameter names, but since we're not doing that throughout the whole file and are not compiling with -Wall...

BTW: Anybody knows whether warning-free code is any goal for the mozilla platform? IMO it pays off in better code quality. We've found quite some bugs while making OpenOffice warning-free and meanwhile compile with -Werror to keep it clean.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Personally, I think we always need to start somewhere and should avoid the warning now, even though the rest of the file doesn't do so. Maybe we can make it a goal at least for calendar code.

Comment 4

[Robert Kaiser]

AFAIK, there is some bug around to eliminate warnings and convert more and more code to using -Werror (some cookie code already does). This also means that compilation usually breaks with every new gcc version, but there seems to be a strong wish to get rid of warnings incrementally across our source.

Comment 5

[Daniel Boelzle [:dbo]]

(In reply to comment #4)
> AFAIK, there is some bug around to eliminate warnings and convert more and more
> code to using -Werror (some cookie code already does). This also means that
Interesting, because this requires the core XPCOM headers and macros to be already warning-free(?)

> compilation usually breaks with every new gcc version, but there seems to be a
> strong wish to get rid of warnings incrementally across our source.
Don't be too pessimistic ;-) My experience doesn't show that, at least w.r.t. gcc3.3/3.4/4.*.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: linkage fix, v2

Tweaked as per Standard8's suggestion.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

In general, I agree with the goal of getting rid of warnings.  W.r.t. this particular patch, does gcc really warn about unused method parameters?  Omitting the params seems like a weird way to make such a warning go away, since it's kind of lying about what the stack looks like.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Since this is a build-bustage fix, I think going ahead and landing this without going through another review cycle is the right thing to do.

Comment 9

[Daniel Boelzle [:dbo]]

(In reply to comment #7)
> In general, I agree with the goal of getting rid of warnings.  W.r.t. this
> particular patch, does gcc really warn about unused method parameters? 
Yes it does, but depends on the used warning level.

> Omitting the params seems like a weird way to make such a warning go away,
> since it's kind of lying about what the stack looks like.
...but which is irrelevant, because the variables are actually not used. At times I opted to just comment out the names to have them once I need them again.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Checked in on 1.8 branch and 1.9 trunk; resolving.