14 ms·
From the article: > The line should read "if (key_data_.has_value() && !key_data_->label().empty()) {" but instead of "&&"—the C++ version of the "AND" operato
by fouric 5y ago
From the article:
> The line should read "if (key_data_.has_value() && !key_data_->label().empty()) {" but instead of "&&"—the C++ version of the "AND" operator—the bad update used a single ampersand, breaking the second half of the conditional statement.
Someone correct me if I'm wrong, but this seems like it wouldn't be possible in a language that didn't conflate boolean values with bitvectors.
Edit: I was wrong[1].
[1] https://news.ycombinator.com/item?id=27923785 https://news.ycombinator.com/item?id=27923785
- CobrastanJorji 5y agoForgive my poor C++, but has_value() sounds like a bool, and empty() sounds like a bool. Why wouldn't "bool1 & bool2" work correctly?
- kevinmgranger 5y agoIf I had to guess, the second expression depends on the first being true. So if `has_value()` is false, perhaps `key_data_->label()` is undefined behavior.
- dataflow 5y agoDereferencing via key_label_-> has undefined behavior when !key_label_.has_value(), so the compiler treats that case the same as if key_label_.has_value(). Or to put it another way, the compiler reasons that key_label_ must have had a value (since you dereferenced it), and thus optimizes out the check for `has_value()` (since clearly it was unnecessary).
- fouric 5y agoAhhh, so it was a problem with lazy evaluation (and the lack thereof when using &), and not type coercion, like I initially thought.
- dataflow 5y agoYeah it has nothing to do with type coercion. I'm baffled a linter doesn't catch this though. & for bools should just be prohibited altogether. If people really want & for bool, they can just cast to an integer type and use & with that (which generates its own warnings appropriately).
- gpderetta 5y agothe problem is that bitwise & and | are routinely used for their non-shortcircuiting behaviour in boolean expressions, so it is hard for the compiler to flag this as an error. Many linters and static analysis tools do flag them though as these days it is not considered good practice.
- dataflow 5y ago> & and | are routinely used for their non-shortcircuiting behaviour in boolean expressions Really? I can't recall a single case of this with 'bool' in any codebase in my recent memory. I only see it with integers. And even if it's somehow routine for your codebases with bool (why?!), surely it's not routine inside a conditional expression, so at least they can prohibit it there?
- kevin_thibedeau 5y agoWhenever I see it I assume it's a typo. If it was truly intentional I would expect a comment pointing out why it's needed.
- anyfoo 5y agoWhy would you even need non-shortcircuiting behavior in boolean expressions? Because either side of the operator has side effects that you want to happen unconditionally? Please just write it out as an extra statement then instead of hammering it into place with an implicit coercion to an integer type only so you can abuse bitwise AND and OR only to force that side effect to... you can see why this is probably not the most elegant idea.
- myko 5y agoWould this optimization not have been on the beta/canary channel? I assumed they were but perhaps not since this was not caught.
- dataflow 5y agoI don't know how their builds work or where their templates are instantiated but it's possible that the compiler doesn't see this optimization until LTO and that they don't do LTO until stable releases? It would seem strange though, I'd have thought beta would have LTO.
- anyfoo 5y agoYeah that would be a very weird choice. LTO itself can have bugs, or (as we speculate here) unmask existing bugs. Plus, even that aside, you'd hopefully test exactly the build that you will release before you release it. As I wrote in another comment, this could still be the result of a freak accident and not have shown up in testing, though (but since I know nothing about the process or the code there, it could be anything).
- xmprt 5y agoSo instead of short circuiting like that code should when it uses &&, it instead makes an assumption about the first part of the boolean expression because otherwise the code would have undefined behavior?
- dataflow 5y agoYeah. More specifically, it just inserts whatever behavior it likes for the undefined scenario, which in this case is "the same behavior as in the defined scenario".
- ddalex 5y agoI am utterly confused. How would the compiler know that has_value() guarantees that the ->label dereferencing work, since this is a bitwise operation ?
- dataflow 5y agoI would imagine has_value is checking to see if a pointer is NULL, and -> is dereferencing that same pointer. When the optimizer sees the two together it realizes the former is useless.
- khazhoux 5y agoI don't think that's right. It seems far-fetched that the compiler would know that accessing key_data->label() implies that k_d->has_value() == true I believe the explanation is: A && B : compiler will evaluate B only if A==true A & B : compiler will evaluate both A (safe) and B (unsafe), then perform bitwise operation
- dataflow 5y agoNo, it's not far-fetched, this is a pretty well-known optimization. https://blog.regehr.org/archives/970 https://blog.regehr.org/archives/970
- Cthulhu_ 5y agoIn other languages, booleans are their own values (iirc in C/C++ they're aliases for 0/not-0) for which the `&` operand is simply not implemented. There's also linters and other quality control analyzers that will warn on a bitwise operator on booleans.
- Trollmann 5y agoYou can use `and` and its siblings instead of && and similar in C++11. Most people here, and in other boards, will try to convince you that it hurts readability because 'that‘s how we always did it' (read I‘m used to it and don‘t like change).
- fouric 5y agoI am also of the opinion that `and` is more readable than `&&` (and isn't as easy to typo in a catastrophic way) - although my main point was about the weaker type system.
- qalmakka 5y agoC++ definitely hasn't a weaker type system than "newer" languages like Java - if any, it is much more richer and complex than most languages out there. What's happening here is a type conversion that has to be in place due to C not having a boolean type until 1999. C++ attempts to construct a boolean from the argument of an `if()`, and given that bool can be constructed from int, the conversion succeedes. You can define your own conversion operators to boolean, too, which are very useful for stuff like smart pointers and similar classes that may or may not have a value. struct A { std::string value; explicit operator bool() const { return this->value.size(); } }; // ... A a {}; if (!a) { // ... }
- fouric 5y ago"Not as weak as Java" is not a very interesting benchmark, as Java also has a poor type system. C++'s type system is pitiable relative to those of Rust, Haskell, OCaml, and SML. Moreover, in addition to being less expressive than them, C++'s type system is also weak, in formal sense, by allowing many implicit type conversions - which is one of the issues that I was complaining about. The fact that it "has to be in place due to C not having a boolean type until 1999" doesn't make it any less weak.
- jcelerier 5y ago
- dylan604 5y agoThat so sounds like an error I would make and not some superDev working at the Googs making around 4x the amount of money I make.
- myko 5y agoThey're just people. If you want to get in you probably could, with some effort.
- anyfoo 5y agoNot sure why you think a "superDev" would be less susceptible to simple typos?
- SilasX 5y agoThe superDev would be; the linters and test suite (that said devs should be enforcing and maintaining) shouldn’t be.
- deleted 5y ago[deleted]
- gmfawcett 5y agoThis is one of those times when, for all its verbosity, Ada got it right. For boolean conjunction you must choose between writing "A and B", or "A and then B". The "and then" version is short circuiting, while the bare "and" version is not.
- shawnz 5y agoVisual Basic also has this distinction (but the keywords are "And" and "AndAlso")
- gene91 5y agoThis is a typo of missing an ampersand. If the author were writing Ada, it would have been a typo of missing a “then”. How is it different?
- Cthulhu_ 5y agoIt's different because in C it changes a boolean comparison to a bitwise one. Is Ada's `and` a bitwise and?
- gpderetta 5y agoFor bool there is no difference between bitwise and and logical and. There is a difference between short-circuiting and non short-circuiting and, hence the distinct operators.
- thesuperbigfrog 5y agoThe equivalent Ada code does not exist as the Ada boolean type only has a logical AND operator (no bitwise operations exist): http://www.ada-auth.org/standards/12rm/html/RM-A-1.html#I5384 http://www.ada-auth.org/standards/12rm/html/RM-A-1.html#I538...
- grishka 5y agoCertainly won't compile in Java.
- shawnz 5y agoIn fact you can apply the single ampersand & operator to booleans in Java, and just like in the example here it has the same behaviour as && but with no short-circuiting.
- grishka 5y agoHuh. I didn't know that.
- taeric 5y agoOddly... It would? Try: Map<String, Boolean> map = ... if (map.containsKey("hello") & map.get("hello")) { ...
- wyager 5y agoThe most general solution here is to use algebraic types and pattern match on the value being “None” or “Some data”.
- tialaramex 5y agoCertainly I find if (key_data_.has_value() && !key_data_->label().empty()) return key_data_->label(); to be less readable than: if let Some(label) = key_data_ { label } It's notable that people complain Rust adds too many odd punctuation marks, and yet in this example it's C++ with an exclamation mark, question marks, and two different structure member operators (dot and arrow) while Rust just does the pattern match.
- Cthulhu_ 5y agoIt's not, because that would involve re-engineering and re-writing millions of lines of code. The simpler solution is to fix this gap in their automated testing. Another one is to configure a linter to error on bitwise operations on boolean values.
- shawnz 5y agoOther than this particular edge case it's not obvious to me why bitwise operators shouldn't be applicable to boolean values. Aren't they just single-bit vectors? Of course it makes sense that logical operators wouldn't be applicable to numeric types, but why the reverse?
- phkahler 5y ago>> Other than this particular edge case it's not obvious to me why bitwise operators shouldn't be applicable to boolean values. That is a really good question. At some point they decided that true=1 and false=0 but I'm not sure how deep that goes. In particular what does ! actually do in this context?
- gpderetta 5y agoIn what context? Unless overloaded ! coerces its operand to bool and returns the negation.
- qalmakka 5y agoIf you stop integer values from being implicitly convertible to booleans, you break the language and its compatibility with C. The inverse is debatable, though. I honestly don't see why someone would do bitwise operations on boolean, unless they really wish to die a slow death from thousand cuts. Integer promotion rules make even the simplest operations a wild mess, and there's nothing in the standard that prevents bool from having a sizeof equal to int (thus further complicating everything). It might be sensible for newer language revisions to consider deprecating certain operators from being applied to bool, given that there is a strong change that 99% of the time their use is not on purpose, but I guess it depends on how much disruptive such change would turn out to be.
- shawnz 5y agoI don't think the other poster was imagining a language that retained compatibility with C, but rather a new language altogether. Of course it would be impossible to change this aspect of the language if retaining compatibility with C was a requirement. > I honestly don't see why someone would do bitwise operations on boolean For example, you might want the behaviour shown here of not short-circuiting. Or you might want to create a bitwise function that also allows calling on single-bit (i.e. boolean) values.