82

I was writing some C++ code and mistakenly omitted the name of a function WSASocket. However, my compiler did not raise an error and associated my SOCKET with the integer value 1 instead of a valid socket.

The code in question should have looked like this:

this->listener = WSASocket(address->ai_family, address->ai_socktype, address->ai_protocol, NULL, NULL, WSA_FLAG_OVERLAPPED);

But instead, it looked like this:

this->listener = (address->ai_family, address->ai_socktype, address->ai_protocol, NULL, NULL, WSA_FLAG_OVERLAPPED);

Coming from other languages, this looks like it may be some kind of anonymous type. What is the name of the feature, in the case it is really a feature?

What is its purpose?

It's difficult to search for it, when you don't know where to begin.

sergiol
  • 4,122
  • 4
  • 47
  • 81
Michael J. Gray
  • 9,784
  • 6
  • 38
  • 67
  • 3
    +1, nice question for a pub quiz (or an appalling interview question for firms that like to hire people who are good at brain teasers). – Bathsheba Nov 18 '14 at 10:17
  • 8
    Link to [Wikipedia article on comma operator](http://en.wikipedia.org/wiki/Comma_operator). FWIW, one of the common uses is to get two side effects in a `for` loop *iteration expression`, ala `for (int i = 0, j = 0; i < 10; ++i, --j) ...` – Tony Delroy Nov 18 '14 at 10:34
  • I understand the comma seperation. But What I don't understand is, why does your compiler deletes content and doesn't even warn you about? 0o I thought more for That was you question. – dhein Nov 18 '14 at 12:06
  • @Zaibis Please do not hijack existing questions. If the existing questions do not answer your question, please post a new one. – AStopher Nov 18 '14 at 12:46
  • @cybermonkey It's not hijacking in my opinion. – Michael J. Gray Nov 18 '14 at 14:07
  • @Zaibis I think that it's not deleting content. It's evaluating the statement and it just so happens that each subsequent piece of the statement that is evaluated is overwriting the behavior of the previous one, until you reach the single effective piece that is the last. – Michael J. Gray Nov 18 '14 at 14:08
  • 1
    @MichaelJ.Gray Technically You are wrong. It is not overwriting the behaving. Imagin: `i=0;j=0;x=i++,j=6;` x and j would be 6 and i would be anyway 1. If the behaving of i++ would be overwritten it would remain 0. But each statement gets invoked and after reaching the `,` just all extensions get discarded and the next pseudo sequencepoint gets invoked. So the first `=` just assignes the part after the last `,` but each point gets invoked. Anyway:I dont get why your compiler doesn't warn you on redefinition of function declaration and instead changes your code to something of diferent behavior – dhein Nov 18 '14 at 15:03
  • @Zaibis When I explained my thoughts, I was referring specifically to the exact case I have in my question. It's effectively overwriting the changes as the statement is evaluated and results in a single value that is the last value. As for the compiler warning, I don't know why it would warn for a redefinition of a function declaration as that's not what is going on here. But if you want to know, I suppose you would have to ask Microsoft. – Michael J. Gray Nov 18 '14 at 15:10
  • @MichaelJ.Gray Then I Missunderstood your question. What I read so far is, you defined a own WSASocket prototype and isntead of using one of the both (the known and yours, the compiler just evaluated its parameters and didn't call the fucntion. – dhein Nov 18 '14 at 15:13
  • @Zaibis The `WSASocket` function is a Windows API function that returns a `SOCKET`. I originally had a call to the `WSASocket` function with its parameter list and the result going into a variable. When I was refactoring, I had removed just the letters for "WSASocket" as I was replacing it and got side tracked. Erroneously, I had left "( arg1, arg2, arg3 ... )". The result was that my variable had `arg3` assigned in the end. – Michael J. Gray Nov 18 '14 at 15:18
  • Ah ok, Now I got it. I thought this refactoring was done by the compiler and you were asking why. May I ask anyway, for what purpose you did that? (The refactoring) What were you expected to happen? – dhein Nov 18 '14 at 15:20
  • 1
    @Zaibis I was replacing `WSASocket` with a call to my own adapter for compatibility with another system, but got distracted in the middle of changing that piece. I left my computer and came back and started fixing something else and forgot about my edit. When debugging one of my assertions kept failing for ensuring a valid socket is associated with my variable. – Michael J. Gray Nov 18 '14 at 15:21
  • You say "my compiler" -> Could you tell us which one it is? Would it, by any chance, be `g++`? – Medinoc Nov 19 '14 at 10:01
  • @Medinoc Microsoft Visual C++ 2013 for Microsoft Visual Studio Ultimate 2013 (12.0.30723.00 Update 3) targeting 32-bit and 64-bit for Microsoft Windows 8.1. – Michael J. Gray Nov 20 '14 at 00:46
  • 5
    If you use a proper compiler, with warnings turned on, it will warn about such dodgy code, for example `gcc` and `g++` with the `-Wall` option say: `warning: left-hand operand of comma expression has no effect [-Wunused-value]` – Sam Watkins Nov 20 '14 at 01:27
  • 3
    So, in my opinion the best answer to this question is "always enable warnings" - then the compiler itself will explain that there is an error and tell you what you did; and this answer will solve many other problems for you also. – Sam Watkins Nov 20 '14 at 01:28
  • @Bathsheba *This is not a brain teaser*. This is a question in an interview where they are saying 'what is the bug here?' Fixing bugs is an important skill. – Miles Rout Nov 24 '14 at 20:52
  • @MilesRout This specific question is not an interview question. I genuinely encountered this while developing something and had no clue what was causing it or where to begin searching it. I knew how to fix it, just not what it actually did in the first place. I agree, fixing bugs is an important skill. This question is not about fixing bugs, it is about gaining a further understanding of a language feature. – Michael J. Gray Nov 25 '14 at 02:57

3 Answers3

151

The comma operator† evaluates the left hand side, discards its value, and as a result yields the right hand side. WSA_FLAG_OVERLAPPED is 1, and that is the result of the expression; all the other values are discarded. No socket is ever created.

† Unless overloaded. Yes, it can be overloaded. No, you should not overload it. Step away from the keyboard, right now!

R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
  • 9
    +1, boost spirit very effectively overloads the comma operator, but that is a very lonely exception to the rule: *don't do it*. – Bathsheba Nov 18 '14 at 10:21
  • 2
    @Bathsheba Boost.Assign also overloads the comma operator, but who uses that since C++11? – rubenvb Nov 18 '14 at 10:29
  • 39
    But you did have the best explanation of how dreadful the mighty comma can be. Unforgiving and full of ridicule, it plagued my code for several minutes while staring at me with no shame for its behavior. – Michael J. Gray Nov 18 '14 at 12:22
  • 3
    Related: [When to Overload the Comma Operator?](http://stackoverflow.com/q/5602112/1025391) – moooeeeep Nov 19 '14 at 08:29
  • Isn't the use of the Comma operator within parentheses a GNU extension? – Medinoc Nov 19 '14 at 10:01
  • @Bathsheba or more specifically, Boost phoenix does. And so does Boost lambda – chbaker0 Nov 19 '14 at 10:48
  • @Medinoc The C++ comma operator within parentheses is standard: see section 5.18 of the latest draft. – Thumbnail Nov 19 '14 at 12:41
  • Wow you got 19 upvotes just for musing about how many upvotes your answer got. I think that's postmodern or something. – Ladlestein Nov 19 '14 at 18:08
  • 1
    @R.MartinhoFernandes - Eigen also very effectively overloads the comma operator. See [this SE question](http://stackoverflow.com/questions/23841723/eigen-library-assigning-matrixs-elements) for some sample usage. – David Hammen Nov 20 '14 at 00:32
  • 1
    @Bathsheba Well, I am sure that someday someone will have a legitimate reason to overload `sizeof`. It still doesn't make it anything less terrible as an idea in general. – Joker_vD Nov 20 '14 at 10:25
  • I imagine overloading `sizeof` is almost feasible now we have `constexpr`. What have you unleashed? – Bathsheba Nov 21 '14 at 08:01
22

The comma operator is making sense of your code.

You are effectively setting this->listener = WSA_FLAG_OVERLAPPED; which just happens to be syntatically valid.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
  • 3
    Not only syntactically valid, but since the Windows API is not type safe it's also a valid conversion. – MSalters Nov 19 '14 at 09:46
  • 5
    @MSalters not that POSIX is much better in that respect – chbaker0 Nov 19 '14 at 10:52
  • 1
    Which is why constructors callable with a single argument should be **explicit**, then a compiler error would have occurred and the developer would have had a chance to take a second look at this bizarre construct. – Matthieu M. Nov 20 '14 at 07:23
  • @MSalters Well, `SOCKET` is a typedef for `unsigned int`, and `WSA_FLAG_OVERLAPPED` is an `int` literal. Had the `SOCKET` be a synonym for `int`, it wouldn't even be a *conversion*. – Joker_vD Nov 20 '14 at 10:20
  • 1
    @Joker_vD: Indeed. Had they made it a typedef for `struct __socket*`, we'd have less bugs. But there's just too much code out there which "knows" `SOCKET` is integral. – MSalters Nov 20 '14 at 10:34
21

The compiler is evaluating each sequence point in turn within the parenthesis and the result is the final expression, WSA_FLAG_OVERLAPPED in the expression.

The comma operator , is a sequence point in C++. The expression to the left of the comma is fully evaluated before the expression to the right is. The result is always the value to the right. When you've got an expression of the form (x1, x2, x3, ..., xn) the result of the expression is always xn.

Sean
  • 60,939
  • 11
  • 97
  • 136
  • After you've added your clarification there, it makes more sense. I didn't quite understand the relevance of the sequence point issue at first. – Michael J. Gray Nov 18 '14 at 10:18