Drop c90 compiler support

I think it would be reasonable to add a configure check which complains if all your compiler supports is c90, and add a switch to override that behaviour documenting that if you use it without telling us, Tor will stop working for you with 0.2.7. That way, we don't prevent anyone from building Tor today, we still enfore c90 code for now in 0.2.6 without breaking it accidentally like we certainly would if we just lifted the requirement, and have a smooth way forward. Happy to cook up a patch if people think this is a good idea.

changed milestone to %Tor: 0.2.6.x-final

added component::core tor/tor milestone::Tor: 0.2.6.x-final nickm-patch priority::medium resolution::implemented status::closed type::defect labels

Sounds okay to me. I wish we could drop the c90 requirement right now, and let anybody who doesn't like it complain... but the risk there is that if we guess wrong and find out that there is some C90 compiler that we need to support, we would maybe need to adjust a lot of code. Maybe it wouldn't be too bad, though?

Also, to be clear: The only c99 feature we're planning to require is the ability to declare variables in the middle of a block, right?

Ah, I thought it would be more of a slow migration towards C99, definitely starting with the variable declaration in the middle of blocks, but also more eventually. If it is just that I think we can just switch - if we make a mistake refactorings will be not so hard.

Are there other c99 features you had in mind?

no, not immediately. But my thoughts were that if someone had a patch we'd not reject it because it used c99.

Here's an imperfect coccinelle script to transform our code to use c99-style declare-at-time-of-use. Probably we shouldn't run it for a while, until we're certain we will allow this syntax long-term.

@@
identifier func;
identifier v;
type tp;
expression e, e2;
statement S1;
@@
 func(...) {
 ...
-tp v;
 ... when != v
(
 {
   ... v ...
 }
|
 if (e2) { ... v ... } else S1;
|
 if (e2) S1 else { ... v ... }
|
 if (e2) { ... v ... }
|
+tp
 v = e;
 ...
)
 }

Do you think we should switch like that? I thought we'd just naturally transition over. Also, what about the previous comment?

Replying to Sebastian:

Do you think we should switch like that? I thought we'd just naturally transition over.

It's an option! Naturally transitioning for a while is probably a good idea, though.

Also, what about the previous comment?

You mean the one about other c99 features? I think we should be careful there; the only one we've really wanted so far has been the mid-block declarations. If we find out that all compilers have those, it might not mean that all compilers have designated initializers. Or we might decide that (say) compound literals are silly. So let's go on a case-by-case basis.

I've added a sample branch in my public repository as "require_some_c99". How does it look?

Trac:
Status: new to needs_review
Keywords: N/A deleted, nickm-patch added

Looks good to me. I didn't find a compiler on any of my systems which didn't like it, but I don't keep too many obscure things around.

Merged! Opened another ticket (#13260 (moved)) for code changes later in this series (or later still).

Trac:
Resolution: N/A to implemented
Status: needs_review to closed

Hello, Unfortunately C99 is not supported by Visual Studio 2010.

Is it possible for you to revert this?

For example, the following code from address.c in common dir will not compile:

tor_assert(addr); sa_family_t v_family = tor_addr_family(addr); ^ v_family needs to be declared before function call.

struct in_addr in_tmp = { .s_addr = 0 }; ^ a c89 way to do this is to use { 0 } instead, it is equivalent to a memset but shouldn't cause much problem here.

static char nil_bytes[16] = { [0]=0,0,0,0, 0,0,0,0, 0,0,0,0, 0,0,0,0 }; ^ there are 16 elements in that array, and 16 zeros declared, why using [0]= ?

Regards, NewEraCracker

Trac:
Username: NewEraCracker

Have a look at commit message 0ca83872468af59 -- those C99 features were put into address.c specifically to catch compilers without c99 support, so that we can find out from people whether there's a compelling need not to use the 15-year-old standard.

There's been a thread about this on tor-dev last month. In particular, see https://lists.torproject.org/pipermail/tor-dev/2014-October/007633.html .

Hm. Actually, do all the other modules build with VS 2010 ? If so, maybe we could restrict ourselves to a disappointing but not wholly inadequate subset subset of c99...

Yes! I have even created a newer patches to build Tor with MSVC 2010.

See #13081 (moved) for more details.

Trac:
Username: NewEraCracker

That second patch actually seems to suggest that the other modules don't build with MSVC 2010. It looks like some of the change you're doing are putting declarations be at the start of their own blocks, according to c90 style. That issue is the big one that made me want to shift away from c90 in the first place.

closed

mentioned in issue #13260 (moved)

moved to tpo/core/tor#13233 (closed)