0
\$\begingroup\$

This site states that correctness in unanticipated situations is explicitly on-topic here.

If you have a working piece of code from your project and are looking for open-ended feedback in the following areas:

  • Application of best practices and design pattern usage
  • Potential security issues
  • Performance
  • Correctness in unanticipated cases

… then you are in the right place!

Undefined behavior (UB) is when the language standard does not require any particular behavior. Therefore, questions about whether your code relies on UB arise when your code seemingly works, but you don't know if it might break in unanticipated situations (a different compiler, options, whatever)

So it seems that UB is explicitly on-topic. In light of this I don't understand why my question was downvoted within seconds (currently -2), or why the comments suggest it's off-topic:

+3 | If the code isn't working as expected then ask the question on stackoverflow.com rather than code review. – pacmaninbw 2020-10-05 18:17:52Z, License: CC BY-SA 4.0

| |
\$\endgroup\$
  • 3
    \$\begingroup\$ Hey does the code work? If you can show an example of you using the code with expected output that'd help me understand where the problem here lies. A simple basic single unit test should suffice for me but more than one test would be greatly appreciated. \$\endgroup\$ – Peilonrayz Oct 5 at 19:53
  • 3
    \$\begingroup\$ Undefined behavior in the title was part of the problem, it indicated that there was a problem in the code. There isn't enough code to review this, you need to show how it is used. The original comment has been removed, but my down vote is locked in until the question is edited. \$\endgroup\$ – pacmaninbw Oct 5 at 20:10
5
\$\begingroup\$

You are kind of missing the point that this whole site Code Review is about code reviews, which is a well-known term for peer review by other programmers. It is not about discussing language features and mechanics.

Anyway, your question is almost a perfect match for https://stackoverflow.com. Here is some migration advice for how you can ask it there instead:

  • Go to Stack Overflow and ask a new question, paste the question there.
  • Leave out the parts about C++ and Java, tag the question C only.
  • Add the tags undefined-behavior and language-lawyer. The latter tag means that you are interested in the formal behavior of the language, rather than a practical use case.
  • Rephrase "best way" with your specific criteria for "best". Most portable? Safest? Most readable? Least memory consumed?

If you are also interested in C++, then copy the question and tag it C++ only, then ask it separately. C++ and C are almost always different in some subtle way, which happens to be the case here too (union type punning rules).


Example of how to make your question well-received on SO:

Title: Is it undefined behavior to use short type bitfields with unions?
Tags:

Does the following declaration contain any form of undefined behavior?

typedef union
{
  typedef struct
  {
     unsigned short a :1;
     unsigned short b :1;
     unsigned short c :2;
     unsigned short d :10;
  } bitfield;

  unsigned short bitmap;
}example_bitfield;

Or are there other forms of poorly-defined behavior present?

In case the above code isn't safe, then what is the safest and most portable way to express this bit-field, using well-defined standard C?

| |
\$\endgroup\$
4
\$\begingroup\$

To me this is a rather clear case of authorship of code; the spirit of that rule is that you're supposed to come here with a piece of code you've written, to get feedback on any/all aspects. This includes unexpected undefined behavior, yes (but then, where are your test cases?), but also your naming conventions and any other nitpick any reviewer might think to point out in your code (i.e. a great CR question purposely open-ended).

But if it's not your code but something you saw online or in a book, then you'll be getting feedback on things you don't care to hear about, and when someone does come up with an edge case that breaks it, and goes on to explain why & how and proceeds to show how to rework the code to make it a nicely handled case... that's great, but was that a peer review?

Go ahead, write stuff, break things, write more stuff and break more things - eventually you'll have a working piece of code of your own for which you'll really want another pair of eyes to look at, and we'll be right here ready to review it and you'll really get the best value out of this site, I'm sure.

IMO questions about stuff in books (ignoring legal aspects, IANAL) can spark very interesting discussions on Quora or Reddit, or even in [chat] - but CR is a bit different.

| |
\$\endgroup\$
2
\$\begingroup\$

The code in the question consists solely of a type definition, not even a variable declaration. Without executable code it is impossible to tell if there will be undefined behavior or not.

Because of the nature of the code (type definition) and the type name given to it, example_bitfield this code seems to be very hypothetical which would be a valid reason for closing the question. Using a real example of executing code that runs as expected would make the question on topic.

| |
\$\endgroup\$
1
\$\begingroup\$

tl;dr

I find your question to potentially be off-topic for Authorship of Code, being obviously broken and 'lacking context'. I don't see much of a link with our rational for AoC as you're not asking us to explain the code. Your code doesn't seem to have glaring issues that'd prevent usage, so isn't broken.

At least one user has enough context to see issues with your code. And so there is enough context for your question to be answered.

The problem is there isn't enough code to tell if there is a logic or alignment problem here.

I don't think any more code is needed to be able to explain one of the logic problem or the alignment problem. As an answerer can explain both problems. Closing the question seems like pointless red tape.

  • The asker has a bad user experience.
  • (Non-diamond) moderators have to then spend time closing the question, reopening it and potentially having to deal with the complaint on Meta.
  • Answerers can feel they can't answer it because they'll get downvoted even though it's answerable.

It in all just seems like a net loss for everyone involved. Where if one C / C++ guru just answered it in a couple of minutes it would be a net benefit, rep would be shared amongst everyone involved and the asker would leave happy. Possibly to return and contribute more to Code Review because of a good experience.

Long in-depth explanation of why this happens

Over time (non-diamond) moderators pick up a gut feeling to be able to tell if a question is on or off topic by skimming the question. Think of it like us being an AI we've seen the same off-topic thing time and time again that we can pattern match with our learned data set in seconds.

But just like AI our data sets can be biased. We as a site don't get many "is this correct" questions like yours. And so from the title we go into the question thinking it's off-topic.

It's hard to say what happens at this point in all users. But for me I take the gut feeling and put it into words, pointing to what all the problems are in a comment. This helps prevent me from incorrectly closing an on-topic question.

I've found this has two benefits:

  1. The site doesn't come across as hostile and hammer happy without reason.
  2. I don't make a fool of myself. I've been wrong on meta, and I can say admitting to that is not fun. The comfort of being wrong in my mind is immeasurably superior.

I'll run through this mental collision test so we can all be on the same page.

So is your question off-topic?

First I go through all of our rules. This is a very basic gut comparison with a sprinkling of logic. This filters out most of our rules and leaves us with the ones that seem relevant so we can focus on them.

Unrelated rules:

  • Best Practice
    This doesn't apply as you're not asking for a best practice you're asking if it has UB.
  • Broken - Known
    You say it's not not broken so it's not known.
  • Context - complete code, example code, hypothetical code, pseudo-code
    Your code looks real enough and complete so is none of these. Whilst it does have _example our example reasoning is complicated and it's not a match. It's better to delegate to "Context - context".
  • Missing Description
    You've not really explained what the code is. But a "bit field" is a fairly well known datatype.
  • Duplicates
    These are allowed.
  • Reviewing Design
    Design question's are not off-topic.
  • Explination of Code
    You're not asking us to explain code.
  • Code Inline
    The code is inline in the question.
  • Language
    C and C++ are languages we review.
  • Purpose of Code
    We've not outlawed any questions based on what they do.
  • Size - long
    You were able to post your question so this is fine.
  • Specific Question
    These are allowed, we even have a meta somewhat related to your post.

Rules that look appropriate:

  • Author of Code
    You are not the author.

  • Broken - Unknown
    Not providing a test case doesn't make a question off-topic. However the question you got the code from says the following:

    I am trying to implement a C style bitfield in Java. Here is a rough example (I do not have the actual code in front of me at this moment).

  • Context - Context
    Size - short
    Whilst there is no size limit it looks a bit short can we actually safely review this?

Ok so my gut and I have found a couple of potential problems with your post. Lets look into the rules a little more to see if there is a collision and if your post is off-topic.

  • Author of Code

    • Moral / Polite
      I think this is ok. You've come here not to shame the author but to learn from their code.

    • Practical
      You can't explain why the code is the way it is. You wouldn't be able to tell me why the code has the variable names it has or why it has whitespace I think is odd. (But I don't know C so it could be normal)

      However this doesn't seem too important and you can probably understand the response you'd get.

    • Legal
      IANAL but you may have committed a licence violation when posting it on Code Review. IIRC you have to provide the title, author and licence of the entity. You've provided one of these.

    Overall since the legal aspect is not handled by us, it's a meh from me.

  • Broken - Unknown
    Even though you've said it works fine we have had users post code with syntax errors and other unquestionably broken parts to their code. In this case it seems like it should compile fine, so it's not broken.

  • Context - Context
    I don't know C or C++ but from the little I've talked to someone I think there is enough context for this to be reviewed. There is enough context to see "a logic or alignment problem" and so the question can be reasonably reviewed for correctness.

    We come here to help people write better code. Now that I know of these problems with the code I want to know how they can be manifested and how not to hit these problems if I learn C or C++.

Overall I don't really feel compelled that it's off-topic for AoC or being broken. To close for missing context seems to go against the point of Code Review to me.

I think it's on-topic.

| |
\$\endgroup\$
  • 1
    \$\begingroup\$ It's outlawed per AoC. The code is not theirs. Why do you conclude it's on-topic? I'm not convinced a review is what they're looking for either, it's looking for verification in multiple languages. That doesn't necessarily make it off-topic by itself in this particular case, but it does make it an ill fit. \$\endgroup\$ – Mast Oct 6 at 6:36
  • \$\begingroup\$ @Mast I have already answered that in my answer. Additionally in the past we've just waved AoC away if it's not EoC with pretty crappy reasons like "they're now the maintainer" or "IANAL SE can handle that" \$\endgroup\$ – Peilonrayz Oct 6 at 10:19

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .