[unwind]

Very cool, congratulations!

I took a 2-second peek at the code, and just wanted to offer a small piece of advice that I think makes it better. Or two, really. Counting is hard.

Instead of (this is from parser.c):

    apply_result *ret = (apply_result*)malloc(sizeof(apply_result));

apply the two common principles of DRY [1] and "don't cast the return value of malloc()" [2] and you get:

    apply_result *ret = malloc(sizeof *ret);

I think the latter is way better.

[1]: https://en.wikipedia.org/wiki/Don%27t_repeat_yourself

[2]: https://stackoverflow.com/a/605858

[keyvank]

thanks man!

yes that makes sense. unfortunately 30cc is not able to compile that syntax, and will probably give type-checker errors when passing a void* to pointer of another type. but will implement it sometime soon!

[KerrAvon]

I disagree with that advice, FWIW -- (void *) is the cause of a lot of bugs in C programs, and much stricter type checking was often the norm for compilers like CodeWarrior on platforms like classic Mac OS, where (a) if you got something wrong you'd corrupt app memory at best and the filesystem at worst and (b) many developers were used to Pascal, which was stricter. (Moving to GCC on Mac OS X and unexpectedly getting much more lenient type checking was a big surprise.)

[cataphract]

It doesn't make it any safer because the cast is unchecked. It could be argued it's discouraged[1] for a reason that doesn't really apply to modern C versions where implicit-int isn't allowed anymore.

But while the cast is a matter of style (or C++ compatibility), sizeof(*ret) is definitely superior to sizeof(thing_t). The reason is that people sometimes change the type of ret but forget the change the size of the allocation, especially if ret is assigned to on a different line it's declared.

[1]: https://c-faq.com/malloc/mallocnocast.html

[munch117]

But adding the cast doesn't make type checking stricter, quite the opposite - it destroys any hope of the compiler checking the assignment.

C++ shot itself in the foot by removing automatic conversion from void* to T*, because it forces programmers to add casts that weaken type safety.

[unwind]

Aha, so it doesn't know about void* being compatible with all (object) pointers, but so far just requires the pointers to match exactly? I see.

Not sure why I was down-voted, I think that is ... not nice. :(

[dataflow]

I downvoted just because I thought it wasn't great advice. It makes the code incompatible with C++ compilers, and being able to compile C with C++ compiler is often useful.

Also... it felt kind of... unnecessary? to tell someone who is making a compiler how to write basic code in the language they're writing a compiler for. It almost felt like telling an overweight medical student/doctor that he should avoid eating fatty foods.

[Gibbon1]

Writing C code so won't compile with a C++ compiler is gods work.

Because C++ is why C can't have nice things.

[nar001]

I don't think you can be downvoted, there's no downvote button

[quuxplusone]

A beautiful example of Cunningham's Law! :)

[hollerith]

Users with enough karma can downvote comments.

[KerrAvon]

If you're implementing C23 compatibility, you could do

  auto p = (thing_t *)malloc(sizeof(thing_t));

and you get both DRY and avoid (void *) casting.

[dcuthbertson]

> and you get both DRY and avoid (void *) casting.

While that line of code is DRY, it doesn't avoid casting "void *" to "thing_t *".

[gizmo]

This kind of condescending advice doesn't help anybody. Redundant casts, like redundant braces, white space, and a plethora of similar choices are stylistic more than anything else.

If you really feel the need to give unsolicited coding style feedback maybe at least spend more than a few seconds looking at the code before you do so. Otherwise it's just rude.

[dominicrose]

Giving feedback - solicited or not - is part of an engineer's job. Wether it's cosmetic feedback or not doesn't matter as cosmetics are also part of the job, although not everybody thinks so.

[unwind]

Really? Wow. I had no idea, and I really tried to sound non-condescending.

I simply don't agree that redundant casts are something to be ignored, since they can be harmful and hide actual errors (the cast is a bit like `sudo make me a sandwich`, it makes the compiler do what you say which is not always a good idea).

I guess I believe that publicly posting code, and even writing a compiler for the language in question, kind of automatically marks you as interested in the language and open for ideas of usage and so on.

My apologies to the OP if you were offended. Thanks for the feedback, @gizmo.

[solarengineer]

FWIW, I liked your suggestion, the author’s explanation, And your follow on written understanding.

If someone’s technical article is submitted and they are themselves participating in a discussion out feeling insulted, then accusations of unsolicited advice are unfounded.

[williamdclt]

I didn't find your tone condescending at all, it was a perfectly fine comment (esp with sources)

[cschep]

I enjoyed reading the feedback, and their response, and felt no negativity whatsoever. Keep honestly interacting it's good for everyone!

[tetha]

> If you really feel the need to give unsolicited coding style feedback maybe at least spend more than a few seconds looking at the code before you do so. Otherwise it's just rude.

Mh, at the same time, people who know their craft can give solid feedback within a very short timeframe. It's very impressive how good sound engineers can just pickup your mixed recording, listen to some 5 second batches, 15 - 20 seconds in total and already give very solid advice for improvement. Or when I was playing foosball more actively with league players. I could hold myself, but after 2-3 exchanges they could usually point out some technique issues.

I found GPs feedback (and the resulting discussion, heh) useful and the resulting code cleaner (in case I ever have to touch C-Code again, brr), and then learned that the compiler can't do that yet.

[peterfirefly]

Redundant casts are really, really good at hiding errors...

(And few people know that sizeof is a unary operator so the parentheses are rarely necessary. Dropping them reduces the visual noise => improves readability.)

[dcuthbertson]

> sizeof is a unary operator so the parentheses are rarely necessary

I thought parentheses where still required for types, and are optional for variables. Even if that's not so, I still use them for types and avoid them for variables just to add one more cue about what's being sized.

[quuxplusone]

Not "variables," but "expressions." In both C and C++, `sizeof` is a unary operator that applies to a postfix expression, so e.g. `sizeof a[0]` and `sizeof p->x` are both OK. This can theoretically be confusing, although the only examples I know are silly and/or easily fixed:

In C++, `p->pmf` has surprisingly low precedence, so `sizeof p->pmf` means `sizeof(p) ->* pmf`, which is ill-formed.

https://gcc.gnu.org/onlinedocs/cpp/Operator-Precedence-Probl... shows that "insufficiently hygienic" macros can also cause trouble, e.g. `-DINC(x)=x+1` means that `sizeof INC(1)` is `sizeof 1+1` which is `5`, not, as you might have naïvely expected, `sizeof (1+1)` which is `4`.

Extra parentheses can be used to deceive, too: `sizeof(0[p])` is the same as `sizeof(p[0])`, but `sizeof(0)[p]` is... also the same as `sizeof(p[0])` — not `p[sizeof(0)]` — because the postfix `[]` operator binds tighter than the prefix `sizeof` operator.

Oddly enough, the `alignof` operator officially applies only to types, as in `alignof(T)`; Clang, GCC, and EDG all do support `alignof expr`, but only as a non-standard extension.

[teo_zero]

Surprisingly, the sizeof operator applies to prefix expressions, too, except casting. So sizeof !x works, but sizeof (int)x doesn't, despite ! and casting having the same precedence.

The presence of (int) sort of promotes sizeof from "operator that applies to expressions" to "function that takes a type". I think this is a side effect of reusing the same keyword for two syntactically different constructs.

[dcuthbertson]

Thank you very much for that clarification. I knew about using sizeof with expressions, but I so rarely used it that way that I never gave it much thought.

[_proofs]

this reads more like you're anxiously anticipating criticism bc you've been given feedback in a bad manner before. i interpreted nothing in OPs post, or condescending.

as someone who works largely in C and C++ codebases, these kinds of comments are gems, especially when i learn something.

what about it seems condescending? we are on a public forum, literally designed for discussion -- unsolicited perspectives, factual or otherwise -- are the norm, not some off-beaten, micro-transgression.

to reciprocate using a similar tone to the one you used with OP: i just do not understand how people find this offenses -- what are you so sensitive about?