After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 346647 - add option to make glib-genmarshal generate internal functions
add option to make glib-genmarshal generate internal functions
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2006-07-05 12:57 UTC by Sven Neumann
Modified: 2011-02-18 16:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to implement and document the suggested feature (7.39 KB, patch)
2006-07-10 15:52 UTC, Sven Neumann
reviewed Details | Review

Description Sven Neumann 2006-07-05 12:57:15 UTC
glib-genmarshal should have a command-line option that makes it mark the generated functions as G_GNUC_INTERNAL. That would allow marshallers that are used in libraries, but not exported, to be marked as internal functions using the visibility(hidden) attribute. This would be cleaner and also slightly more efficient.

If there is interest, I could write a patch that adds this feature to glib-genmarshal.
Comment 1 Matthias Clasen 2006-07-05 16:45:22 UTC
sounds like a good idea to me; in fact, there may even be a bug about it already.
Or maybe I'm misremembering
Comment 2 Sven Neumann 2006-07-10 15:52:09 UTC
Created attachment 68724 [details] [review]
patch to implement and document the suggested feature
Comment 3 Tim Janik 2006-07-13 09:19:06 UTC
(In reply to comment #2)
> Created an attachment (id=68724) [edit]
> patch to implement and document the suggested feature

thanks, the patch looks mostly good to me. it contains a distracting amount of
empty line replacements though like:
-
+
which don't really help in reading it, but other than that, i just think the global variable "internal" should be renamed to "gen_internal" to match in with the rest of the generation flags and then the patch can be comitted as soon as we have created a new development branch.
Comment 4 Sven Neumann 2006-07-13 09:22:40 UTC
The empty lines are because I removed trailing whitespace while editing the code. I will try to avoid that in the future.

But perhaps it would be a good idea to avoid such problems by removing all trailing whitespace from glib (and gtk+) and making sure that changes don't introduce any new trailing whitespace.
Comment 5 Sven Neumann 2006-07-13 09:24:13 UTC
I will take care of committing the change with the suggested variable name as soon as glib has branched.
Comment 6 Tim Janik 2006-07-13 09:24:56 UTC
(In reply to comment #5)
> I will take care of committing the change with the suggested variable name as
> soon as glib has branched.

did you just mean to volounteer to put up such a patch?
Comment 7 Tim Janik 2006-07-13 09:29:12 UTC
foget my last (comment #6) it contains wrong pastage.

(In reply to comment #4)

> But perhaps it would be a good idea to avoid such problems by removing all
> trailing whitespace from glib (and gtk+) and making sure that changes don't
> introduce any new trailing whitespace.

did you just mean to volounteer to put up such a patch?

one problem i see here is that differente editors of different
developers will fight each other over the spaces. e.g. iirc, 
my emacs settings put no extra trailing spaces at line ends,
but preserve indentation spaces like this:
{
  bar;
__
  baz;
}

"__" indicating two sapces before line end.

your editor may or may not remove those automatically.
Comment 8 Sven Neumann 2006-08-23 06:49:09 UTC
I have committed a variant of this to the HEAD branch:

2006-08-23  Sven Neumann  <sven@gimp.org>

	* gobject/glib-genmarshal.[c1]: added new command-line option
	"--internal" that can be used to let glib-genmarshal generate
	internal functions using the G_GNUC_INTERNAL attribute (bug #346647).

The difference to the discussed patch is that it now puts the G_GNUC_INTERNAL attribute at the beginning of the line. This is to avoid problems as outlined in bug #352268.

I have tried the change but of course further review would be appreciated.
Comment 9 Tim Janik 2006-08-23 08:34:29 UTC
i reviewed the diff that went into CVS, which looked ok.