Generally accepting a RAII guard as a function argument by value is wrong. You usually want to pass the guard by reference (as passing it by value requires copying it - which is usually wrong). This could be done by adding a new attribute (MOZ_NO_BYVALUE_ARG_CLASS?) which prevents the type from being passed by-value as an argument, and then adding that attribute to MOZ_RAII. I'm not sure how much code this would break, or if there are valid use-cases for passing RAII guards by value to functions. So this is also part of this bug.

I understand why you wouldn't want to pass an RAII thing by /copy/, but how do you pass one by /move/? (Both of these fall under "by value", right?) Will bug 1153277 take care of this by ensuring there's no copy constructor?

I might be misunderstanding your question, but wouldn't you use a universal (&&) reference to "pass by move"?

(In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #4) > I might be misunderstanding your question, but wouldn't you use a universal > (&&) reference to "pass by move"? Technically an && reference is called an r-value reference, unless the type parameter T is deduced, in which case it is a universal reference, as the type perameyer can be deduced to T&& or T&. (yes c++ is terrible). Anyways, the r-value reference doesn't necessarily move the value, its just a way of indicating that you may move the value. In contrast if you construct the value to pass to the function using a move constructor and use by-value calling. Then you can know at the call site that the object is actually moved. In practice we don't move raii guards very often. And most raii guards don't have an explicit move constructor, which means that moving those values is just as incorrect as copying them. That being said, there are valid cases where you would want move constructors on raii types. In those cases you would also want to allow temporary objects as long as they are used to pass to a constructor or function accepting a rvalue reference, as these types are presumably movable. For now, none of the types marked as MOZ_RAII are passed by value, so this is OK as an analysis I think, bug I would agree that a better analysis may be necessary. Namely, we might want to treat a type differently if it is an explicit move constructor.

So this hasn't caught any code, it seems. What's the motivation behind adding this?

I was talking with fitzgen on IRC, and he was wondering if we had any plans to prevent mistakes around potentially performing incorrect copies of RAII types - this was simply one mechanism for preventing these copies which we came up with, and which would be unlikely to break a lot of code. I agree that it may be unnecessary.

No, it's fine. I was just trying to understand the goals.

Comment on attachment 8662469 [details] [diff] [review] Part 1: Add an analysis to detect passing certain classes by value to functions Clearing review because patch would be of questionable value.