Using double underscores consistently for internal-only code.

Many of the symbols we define in include/private/*.h are not using the
double-underscore convention, even though they're only used internally
(cross-library, perhaps, but still internal to Subversion).

In http://subversion.tigris.org/hacking.html#other-conventions, we
seem to have decided that double-underscore is the way to go for all
private symbols -- basically, anything not published for third-party
consumers.

I think it's a good convention, and more important than it might
appear at first, for reasons I explain in the IRC transcript below.
We just apparently haven't stuck to it consistently.

I'd like to clean that up. Anyone object?

Here's the IRC conversation, lightly edited for clarity:

------------------------------------------------------------------------
<gstein> kfogel: on double-underscore in a typedef,

<gstein> I had omitted those, but it *does* look like we do that in
          typedefs

<gstein> functions are exported from the lib, so the __ is necessary
          to distinguish. but typedefs are internal to the compilation
          and what headers you have available to you,

<gstein> so I skipped them

<gstein> sounds like I should add double-under to the types?

<rooneg> yeah, it's kind of silly to do it for typdefs. if we are, i
          don't see why we should continue ;-)

<gstein> rooneg: hehe

<kfogel> gstein: I thought wc_db is only going to be used within
          Subversion?

<gstein> the only typedef in include/* and include/private/* with a
          double-under is svn_sort__item_t

<kfogel> And in fact only by libsvn_wc?

<gstein> kfogel: yes

<gstein> correct

<kfogel> Huh? Everything in include/private/ should use double
          under.

<kfogel> I don't know if they do or not, but they sure should. WTF.

* kfogel looks

<rooneg> kfogel: why should typdefs do it? they don't introduce a
          public symbol...

<kfogel> rooneg: hrm? How does a typedef not introduce a public
          symbol?

<kfogel> I mean, it's a symbol like any other symbol...

<kfogel> You mean into the linker?

<rooneg> yes

<kfogel> I'm talking about a reader looking at a type and knowing
          whether or not it can be used by third-party consumers of
          our APIs.

<rooneg> without the header file you can't actually access it, so if
          it's not in a public header file the name doesn't matter

<julianf> perhaps this is about developer sanity rather than the
          linker

* gstein points to rooneg

<kfogel> julianf: exactly

<kfogel> that's what I'm saying

<gstein> hmm

<gstein> dang. he's got a point.

* gstein smirks

<kfogel> sorry :-)

<rooneg> i'd say that the fact that it lives in a private header file
          is indication enough that it's not usable, but i don't care
          enough to argue the point too much ;-)

<kfogel> When reading code, one wants to know (without having to look
          it up and think about it) whether something is internal-only
          or exported.

<julianf> the other side of the debate has a point too... some sort of
          point...

<kfogel> rooneg: sorry, I would have said something ages ago about
          the stuff under private/ if I'd noticed :-(

<rooneg> i'm not actually convinced that's the usual use case for
          knowing if something is public or private...

<rooneg> i mean the only time a developer needs to know is when
          they're changing the name of something, in which case
          they're actually in the header file in question, so they can
          tell from that that it's private.

<kfogel> rooneg: mmm, I don't think so. The best environment is one
          where you find things out not necessarily at the time you
          use them, and where things reinforce each other. Every
          symbol name is a clue to the alert developer (the kind we
          attract, *ahem*) about the overall architecture of
          Subversion. If someone sees svn_wc_db_foo, they may assume
          "Oh, okay, so the DB backend to the WC is actually publicly
          visible..."

<kfogel> And then they'll start making all sorts of inferences based
          on that wrong fact.

<kfogel> It's not a "need to know" kind of thing, IMHO. It's more of
          a "everything you see should confirm what is true, not
          mislead".

<rooneg> i don't know, i don't tend to infer such things from a
          typedef, but then again i know that it doesn't introduce a
          symbol and thus it might as well be called fred for all i
          care ;-)

<kfogel> rooneg: Okay, so maybe you're different. But I know I do :-).

* gstein is sold

<kfogel> I hope Greg got a good price for himself.

<kfogel> If it was in Euros, he should be okay...

<gstein> the typedef is not always in front of you

<rooneg> any typedef that's private will be surrounded by private
          function calls anyway, so why type the extra character for
          the typedef when you're gonna have other hints around it
          anyway

<gstein> so when you see the type floating around, then this tells
          you where/how you can export/use it

<gstein> unfortunately, mexican pesos

<kfogel> mmmm, sorry 'bout that man

<rooneg> anyway, like i said, i haven't written svn code in like
          forever, so it's not like i've got a real vested interest in
          how many extra characters greg and karl will be typing ;-)

<gstein> lol

<julianf> Just a thinking-point: If we were to compile this stuff with
          a C++ compiler, without the 'extern "C"' guards, then the
          names of types would be significant to a C++ linker. Right
          now I'm not going into why people might want to try to do
          that, but I've compiled C as C++ in a previous day-job.

<kfogel> rooneg:
          http://subversion.tigris.org/hacking.html#other-conventions

<kfogel> rooneg: apparently, we already had this discussion, and
          thought double-under was the way to go.

<rooneg> heck, if we really want to get technical, having double
          underscores in symbols AT ALL is bogus and we are in
          violation of the C standard anyway ;-)

<julianf> oh...

<kfogel> rooneg: whoa, really?

<CIA-35> gstein * r32595 /trunk/subversion/libsvn_wc/wc_db.h:
          Continued evolution of the interface. ...

<neels> julianf: one of your(?) new tests is failing, even though I
          find the expected regexp in the output. It doesn't make
          sense. I checked and it also happens on a clean checkout of
          the tree-conflicts branch. it's merge_tests.py 103,
          del_differing_file, it fails to find "Skipped.*tau", even
          though it's there.

<rooneg> technically anything with two underscores in it is reserved
          for the implementation

<rooneg> in practice, implementations only use leading double
          underscores, so it doesn't really matter

<CIA-35> arfrever * r32596 /trunk/configure.ac: Support Neon
          0.28.3. ...

<kfogel> rooneg: huh. Well, we'll just pretend we don't know that.

<kfogel> :-)

<rooneg> can't find the exact reference for the C standard, but it's
          definitely in the C++ standard, section 17.4.3.1.2

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-20 22:29:03 CEST