<div dir="ltr"><div>Summary</div><div>=======</div><div><br></div><div>To sum up the discussion below so far for everyone who is not Patrick and me:</div><div>    * it sounds like there is some discussion to be had on how strong a prohibition should be.</div><div>    * it seems that Patrick and I have general agreement on (just not necessarily exact</div><div>      strength of rule):</div><div>        - <span style="font-size:12.8px">Variables of class type with static storage duration (including constants),</span></div><div><span style="font-size:12.8px">          plain old data types (POD) are allowed</span></div><div><span style="font-size:12.8px">        - </span><span style="font-size:12.8px">Implicit conversion constructors and operators</span></div><div><span style="font-size:12.8px">        - </span><span style="font-size:12.8px">Multiple Inheritance (I made a typo that I think got construed that I did not want any</span></div><div><span style="font-size:12.8px">          multiple inheritance, including interfaces, when I wanted to only allow multiple</span></div><div><span style="font-size:12.8px">          inheritance of interfaces)</span></div><div><span style="font-size:12.8px">    * issues still under discussion</span></div><div><span style="font-size:12.8px">        - Non-const-non-r-value references</span></div><div><span style="font-size:12.8px">        - Exceptions</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">This is not to be construed that any of the above items are closed or discussion has concluded; this is only intended as a convenience for others who are following the conversation that I think Patrick and I had come to an agreement on something or that we are continuing to discuss it. I certainly have no formal authority in the project and only wish to make it as easy as possible for people to follow an important discussion. I would encourage anyone who feels strongly about anything discussed so far or not discussed so far to share his or her view points on the matter.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Intro</span></div><div><span style="font-size:12.8px">------</span></div><div><br></div>With regards to what you said about philosophy about style guides and the Google style guide in particular; I do not disagree.<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">I strongly feel that if we simply leap to a style guide targeting a</span><br style="font-size:12.8px"><span style="font-size:12.8px">monolithic N-MLOC codebase, we are taking on solutions that are not</span><br style="font-size:12.8px"><span style="font-size:12.8px">relevant to our own problems.</span></blockquote><div><br></div><div>I agree in principle; we should not blindly reuse a style guide that was intended for a completely different type of project. However, I do think that some sort of style guide is essential. The intention of the project is to have as many people as possible contributing; thus it should be easy for them to contribute and not get disheartened; I think a style guide is essential in creating this environment because it sets up expectations for what code should look like. Code review is inevitably the place where a newcomers learn a great deal of how a project works and how to contribute happens, but having a set of basic expectations reduces the pain on all sides for the first few code reviews; this is important for not burning out first time contributors.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">My expectation is that OpenBMC is a</span><br style="font-size:12.8px"><span style="font-size:12.8px">collection of small, self-contained repositories on the order of 5 -</span><br style="font-size:12.8px"><span style="font-size:12.8px">50kLOC each.  The dependencies between repositories (interfaces) are</span><br style="font-size:12.8px"><span style="font-size:12.8px">purposefully segmented via the DBus interfaces, which represent a very</span><br style="font-size:12.8px"><span style="font-size:12.8px">different problem from a monolitic codebase where a new dependency is</span><br style="font-size:12.8px"><span style="font-size:12.8px">simply an #include away.  With small repositories, the code itself can</span><br style="font-size:12.8px"><span style="font-size:12.8px">"fit in the head" of a maintainer or active contributor fairly easily[1].</span></blockquote><div><br></div><div>I do not want to go too far into this because I do not want to have an extended conversation about microservices as it would be off topic; however, I will say a microservice architecture does not protect against bad coding practices and is not intended to do so. Being able to come up with a good mental model for a project comes solely from good abstractions and thus clean code. The goal should not be to make it possible for a small number of people to specialize and understand a handful of repos, but to allow for anyone to jump in and understand any part of the project with only a modicum of effort; consistency is required to achieve this and consistency is built on rules.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">I do suspect that the majority of our "rules" will evolve around the</span><br style="font-size:12.8px"><span style="font-size:12.8px">DBus interfaces, which are fairly unique to our project.</span></blockquote><div><br></div><div>Again, I agree in principle, but I would caution against relying too heavily on concepts particular to a library or framework. I see nothing wrong with a microservice architecture, but I do not really see that as unique and would hope to avoid making it overly unique. In the sense that these ideas are not new and we should try our best to keep it that way. Also, yes I fully expect the rules by which we govern the project to change over time, but that does not mean we should not make a style guide.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">My typical philosophy regarding coding practices tends to be fairly</span><br style="font-size:12.8px"><span style="font-size:12.8px">pragmatic.  I don't tend to see practices as "black" and "white" but</span><br style="font-size:12.8px"><span style="font-size:12.8px">more along a scale of how much justification you need to have to use a</span><br style="font-size:12.8px"><span style="font-size:12.8px">practice (more on this in the specifics, maybe). </span></blockquote><div><br></div><div>I agree entirely. I made a response to this effect to the email sent by Brad (I forwarded the contents of his email to the list in my email this morning). I used the terms whitelist and blacklist entirely as a convention to refer to what we are currently discussing: the stronger rules that will require more discussion and likely solicit stronger opinions; although these rules are stronger than the other rules; I do not expect them to be without exception (except for maybe exceptions ;-), I will get to that later).</div><div><br></div><div>Re: General Code Philosophy</div><div>---------------------------------------</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">    1) Conciseness:</span><br style="font-size:12.8px"><span style="font-size:12.8px">        - Did you use far more lines of code to solve the problem than</span><br style="font-size:12.8px"><span style="font-size:12.8px">          necessary?  Those are lines we have to maintain.</span></blockquote><div><br></div><div>I tend to disagree that this should be the most important thing. I think "easy to read" is far more important than conciseness; it is true that it may create more lines of code to maintain, but if they are easy to understand, they will take less effort to maintain. Still, conciseness is something that does have value; I would agree that if two solutions are equally easy to read than the shorter one is probably better assuming there are no other detractors to it, but I would also argue that it is actually easier to read in that case.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">    2) Modern, widely-accepted practices:</span><br style="font-size:12.8px"><span style="font-size:12.8px">        - This is already spelled out in the 'contributing' doc, having</span><br style="font-size:12.8px"><span style="font-size:12.8px">          priority over any specific style statements. [2]</span></blockquote><div><br></div><div>I certainly cannot argue with that. Keeping concepts and practices generally consistent with the greater programming community has a lot of value.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">    3) Performance:</span><br style="font-size:12.8px"><span style="font-size:12.8px">        - Favoring in order { Code size, CPU utilization, memory size }</span><br style="font-size:12.8px"><span style="font-size:12.8px">          due to the unique concerns of the OpenBMC project and current</span><br style="font-size:12.8px"><span style="font-size:12.8px">          technologies used.</span><br style="font-size:12.8px"><span style="font-size:12.8px">        - Macro-level optimization that does not violate conciseness and</span><br style="font-size:12.8px"><span style="font-size:12.8px">          clarity:</span><br style="font-size:12.8px"><span style="font-size:12.8px">            - Are the algorithms used of the appropriate algorithmic</span><br style="font-size:12.8px"><span style="font-size:12.8px">              complexity?</span><br style="font-size:12.8px"><span style="font-size:12.8px">            - Are you avoiding practices that would contribute to large</span><br style="font-size:12.8px"><span style="font-size:12.8px">              generated code sizes?</span></blockquote><div><br></div><div>This is a pretty tough one; although no one can dispute that performance at some level is always critically important especially when resources are scarce, it is also true that writing good code in any context is always a balance of performance and development cost.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">Quantifying "code smell" into a set of non-ambiguous rules is a hard</span><br style="font-size:12.8px"><span style="font-size:12.8px">problem</span></blockquote><div><br></div><div>Absolutely, and that is not my aim. Style guides are tools to make it easier to have conversations about code. They set the base expectations and provide a structure for people to discuss code in the context of. If one disagrees with a practice, she has a document that she can make arguments against (it is hard to argue with a loose set of practices and thus hard to change them).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">I would</span><br style="font-size:12.8px"><span style="font-size:12.8px">prefer we start with industry recognized experts opinion[3] and then</span><br style="font-size:12.8px"><span style="font-size:12.8px">document deviations specific to OpenBMC as they come up.</span></blockquote><div><br></div><div>I agree with trying to follow generally accepted practices and it seems like the CppCoreGuidelines seem to encode that pretty well, but it is not really the same as a style guide and I see no reason why that precludes us from writing one. I would, however, caution against citing the opinions of "industry recognized experts" as sole justification for something. If the opinion itself is cited with good reasoning/good supported evidence that's fine, but it is important not to discount someone because he disagrees with someone important. I am not saying you did that anywhere, all of the arguments you made below either make clear arguments that stand on their own or cite well thought out self-contained arguments, just something to keep in mind.</div><div><br></div><div><div>Re: <span style="font-size:12.8px">Variables of class type with static storage duration (including constants)</span></div><div><span style="font-size:12.8px">-----------------------------------------------------------------------------------------------------</span></div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">This proposal is a key example on why it is hard to quantify these types</span><br style="font-size:12.8px"><span style="font-size:12.8px">of rules.  Everyone agrees with a general principle of "reducing</span><br style="font-size:12.8px"><span style="font-size:12.8px">globals", but as spelled out this proposal would prohibit</span><br style="font-size:12.8px"><span style="font-size:12.8px">initializer-list construction, std::string, and the pattern within our</span><br style="font-size:12.8px"><span style="font-size:12.8px">IPMI plugins of "register on load".  These are all reasonable exceptions</span><br style="font-size:12.8px"><span style="font-size:12.8px">that require little justification in my mind.</span></blockquote><div><br></div><div>It sounds like we agree in principle. My proposal would prohibit std::string constants, but it would not prevent their general use. Initializer list construction would not be prohibited for c-style arrays and not its use in general; you would not be able to use it for constructing variables of class type with static storage, but it outright prohibits that anyway. As written currently, yes the ipmid plugin registration code would not conform, but it would be possible to write it in a way that does conform; this is actually something I am working on with my changes to ipmid, but that is a discussion for elsewhere. (I would like to be clear that this does not discourage global pointers, but in general these also would require a good justification).</div><div><br></div><div>Re: <span style="font-size:12.8px">Implicit conversion constructors and operators</span></div><div><span style="font-size:12.8px">-------------------------------------------------------------------</span></div><div><span style="font-size:12.8px"><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">This would prohibit 'unique_ptr<T>::operator bool()' and</span><br style="font-size:12.8px"><span style="font-size:12.8px">'string::string(const char*)', which are part of the STL and exist</span><br style="font-size:12.8px"><span style="font-size:12.8px">for concise syntax.  I do agree with this in most cases but there are</span><br style="font-size:12.8px"><span style="font-size:12.8px">cases where they both obvious and crisper.  A non-obvious conversion</span><br style="font-size:12.8px"><span style="font-size:12.8px">should be prohibited.</span></blockquote><div><br></div><div>Maybe I should have called out more clearly that I do not plan on prohibiting widely used parts of the styleguide that do these types of things. In anycase, the smart pointers are special in that they maintain the API that exists for raw pointers and it is common practice for people to check for a nullptr by simply treating it as a boolean value. Nevertheless, the vast majority of custom APIs should not use this feature, and you seem to agree with this point in general anyway.</div><div><br></div><div>Re: <span style="font-size:12.8px">Multiple Inheritance</span></div><div><span style="font-size:12.8px">--------------------------------</span></div><div><span style="font-size:12.8px"><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">Composition is fairly common practice in C++ which uses multiple</span><br style="font-size:12.8px"><span style="font-size:12.8px">inheritance.  The sdbusplus bindings, as an example, have a specific</span><br style="font-size:12.8px"><span style="font-size:12.8px">template to aid in the composition of N dbus interfaces into a single</span><br style="font-size:12.8px"><span style="font-size:12.8px">C++ class. [4]</span></blockquote><div><br></div><div>Your argument seems to be arguing for allowing multiple inheritance *of interfaces*, which I meant to allow. I said "unless at most one of the parent classes is an interface i.e. only has pure virtual methods", it should have read "unless at most one of the parent classes is *not* an interface i.e. only has pure virtual methods". You can inherit from as many interfaces as you want, but only one (or no) non-interface. Technically, your sdbusplus bindings are not interfaces, but I could see an exception for that, because they are fairly special constructs. *It quacks like an interface*, from the standpoint of an implementor. Also, the CCG sections you cited agree with this standpoint. C.135 specifically calls out interfaces and C.136 seemed to want to discourage the behaviour in the reasoning section.</div><div><br></div><div>Re: <span style="font-size:12.8px">Non-const-non-r-value references</span></div><div><span style="font-size:12.8px">---------------------------------------------------</span></div><div><span style="font-size:12.8px"><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">I find this especially unnecessary, not recommended anywhere outside of</span><br style="font-size:12.8px"><span style="font-size:12.8px">the GSG, and having a crippling effect to the language.  C++ is not a</span><br style="font-size:12.8px"><span style="font-size:12.8px">procedural / OO programming language; it is also a functional</span><br style="font-size:12.8px"><span style="font-size:12.8px">programming language.  The bulk of the STL is to facilitate a</span><br style="font-size:12.8px"><span style="font-size:12.8px">functional programming style and many of the C++11 features (lambdas,</span><br style="font-size:12.8px"><span style="font-size:12.8px">constexpr functions, etc.) take large inspiration from FP.  Eliminating</span><br style="font-size:12.8px"><span style="font-size:12.8px">l-value reference makes FP techniques exceptionally harder and</span><br style="font-size:12.8px"><span style="font-size:12.8px">syntactically clumsier.</span></blockquote><div><br></div><div>Again, I explicitly allowed use where necessary to maintain consistency our utilize the STL. But in general, const references, r-value references, and pointers together fulfill the same purposes and make it easier to understand what the code is doing. Const reference implies nothing will happen to the data, it will not (outside of exceptional circumstances) store that reference and will not modify it; basically it stays in the function. No special semantics are required when calling the function because there is nothing to consider. R-value reference implies that the data will be consumed, you do not own it anymore and it does it in a beautifully explicit way, std::move, which means that you know the data does not belong to you anymore when you call the function. Providing a pointer requires you to either be working with a pointer which is clear because it has different semantics, or you have to pass a pointer explicitly; either way, you know that the function is free to edit what you passed in; it should not store it because it does not own it (otherwise use a smart pointer), but is free to edit it in the context of the function call. I do not see it standing in the way of FP at all, in FP you are usually dealing with immutable data types which you should use as much as possible i.e. const references, in the case that is expensive you have r-value references and pointers. Although, the CCG does say it is allowed, it admits that non-const references can lead to bugs see the note under F.17.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">A very common recommendation in "Exceptional Modern C++" is to define a</span><br style="font-size:12.8px"><span style="font-size:12.8px">function like:</span><br style="font-size:12.8px"><span style="font-size:12.8px">    template <typename T> foo(T&&)</span><br style="font-size:12.8px"><span style="font-size:12.8px">This function operates on both l-value and r-value references</span><br style="font-size:12.8px"><span style="font-size:12.8px">("universal references" as Meyers calls them).</span></blockquote><div><br></div><div>I think I am missing something here. l-value and r-value references are two different things and are treated differently by the compiler. I tried the following:</div><div><br></div><div>int add(int&& l, int&& r) {</div><div>  return l + r;</div><div>}</div><div><br></div><div>int main() {</div><div>  int l = 1;</div><div>  int r = 2;</div><div>  std::cout << "add(1, 2) = " << add(l, r) << std::endl;</div><div>  return 0;</div><div>}</div><div><br></div><div>and got the following compile error:</div><div><br></div><div><div>test.cc:10:42: error: cannot bind ‘int’ lvalue to ‘int&&’</div><div>   std::cout << "add(1, 2) = " << add(l, r) << std::endl;</div></div><div><br></div><div>which is precisely what I expected to happen. I think I am missing part of your example.</div><div><br></div><div>Re: <span style="font-size:12.8px">Exceptions</span></div><div><span style="font-size:12.8px">--------------------</span></div><div><span style="font-size:12.8px"><br></span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">This was the "old way" in C++ and the standards body specifically</span><br style="font-size:12.8px"><span style="font-size:12.8px">deprecated what you are suggesting.  I'm sure you could read their</span><br style="font-size:12.8px"><span style="font-size:12.8px">rationale.</span></blockquote><div><br></div><div>Sure, I would be happy to. I tried googling around and the only recent update I could find relating to how the compiler treats exceptions was the addition of noexcept. Could you share this?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">In GCC, at least, function attributes can only be defined as part of</span><br style="font-size:12.8px"><span style="font-size:12.8px">declaration and not definition.  That means you would have to forward</span><br style="font-size:12.8px"><span style="font-size:12.8px">declare every function that returns your error type.</span></blockquote><div><br></div><div>That's true. Clang allows you to put the attribute on the type definition, I forgot gcc does not allow that. Still, if the function was part of a public interface, you would have to forward declare anyway. In anycase, my point was that there are alternatives to exceptions where the compiler will actually bug you if you ignore it.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">In every large project I have been involved with, an integer quickly</span><br style="font-size:12.8px"><span style="font-size:12.8px">becomes insufficient to express "an error".  The projects all make their</span><br style="font-size:12.8px"><span style="font-size:12.8px">own "error object".  According to Titus' talk, Google has one of these</span><br style="font-size:12.8px"><span style="font-size:12.8px">internally as well.  So that means we're now talking about passing</span><br style="font-size:12.8px"><span style="font-size:12.8px">around a heavy object by value or a pointer to one (which is always at</span><br style="font-size:12.8px"><span style="font-size:12.8px">risk of leaking)?  What does this error look like?</span></blockquote><div><br></div><div>Also true, hypothetically we would use some sort of error type. Probably an error enum and an optional string message. Google has an open source one we could use, or we could write our own. In any case, it does not need to be heavy weight, as long as the field list is small the object will be small. For user defined types that fit inside of a few machine types, gcc and clang will frequently pass them via register instead of stack. In any case, I would expect the error object to be returned so it would be subject to return value optimization.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">Having a error return type likely eliminates any RVO and again plays a</span><br style="font-size:12.8px"><span style="font-size:12.8px">huge diminishing role in using functional programming methods.  The two</span><br style="font-size:12.8px"><span style="font-size:12.8px">approaches for output variables become:</span><br style="font-size:12.8px"><span style="font-size:12.8px">    1) Fill-in out parameter reference.  [anti-FP]</span><br style="font-size:12.8px"><span style="font-size:12.8px">    2) Return a tuple<error, real_return>.</span><br></blockquote><div><br></div><div>The approach we use in this case is called StatusOr<T>; it wraps the type being returned and the associated Status object (what we call the error object). This is actually a really common pattern outside of C++ and parallels the error monad type found in a lot of functional programming languages (we do not actually support the fmap operation, but the usage pattern is semantically equivalent).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-im" style="font-size:12.8px"><br>>         - Exceptions are not typed<br><br></span><span style="font-size:12.8px">Exceptions are strongly typed in the exact same type system as any</span><br style="font-size:12.8px"><span style="font-size:12.8px">reference.  I don't know what this statement means.</span></blockquote><div><br></div><div>Sorry, what I meant by this is exceptions are unchecked in C++ so you can throw any type; hence, there is no way to enforce what kinds of exceptions are thrown and that those exceptions are caught in functions that do not forward them as in Java.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Again this is the "lowest common denominator" argument, which I do not</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">accept.  We could make the same argument with our legacy codebase, which</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I can assure you in this problem domain is much bigger than yours.  We</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">are making a concerted effort to not directly port any of our code we</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">are contributing to OpenBMC but to make sure it is modernized and</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">follows the practices with the rest of the codebase.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Any code you are planning to port would either need to go though the</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">same vetting process as a clean contribution (for acceptance into an</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">openbmc tree) or would be maintained in your own repositories.  If it is</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">something that is maintained in your own repositories, the only place</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">exceptions could be introduced is when you call base OpenBMC utilities,</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">like sdbusplus.  It would not be unreasonable to "catch" all exceptions</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">at the API level into base utilities and turn them into a return code.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">(And in fact, I highly suspect you already have these abstracted out due</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">to your Google Mock test framework).</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Since the majority of the repositories have self-contained programs that</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">expose DBus interfaces, what those programs do internally has no impact</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">on your ported programs that are interacting with those DBus interfaces.</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">DBus itself already has a DBus-Error type, which is distinct from method</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">return values, that you are free to turn into a return code instead of</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">an exception.</blockquote><div> </div><div>Fair enough, this is admittedly not a great argument, but in the case of an even split, it is of minor benefit.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">CppCoreGuidelines C.44 and C.50 cover this sufficiently?  Some of your</span><br style="font-size:12.8px"><span style="font-size:12.8px">proposal seems to be a fallout of not allowing exceptions, in my</span><br style="font-size:12.8px"><span style="font-size:12.8px">opinion.  We should always use RAII (E.6) in any case.</span></blockquote><div><br></div><div>Fair enough.</div><div><br></div><div><div>Re: Virtual Functions</div><div>----------------------------</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">I was not proposing an elimination or prohibition on virtual functions,</span><br style="font-size:12.8px"><span style="font-size:12.8px">but a "justification required".  As in, you need to be prepared to</span><br style="font-size:12.8px"><span style="font-size:12.8px">answer how this is better and why compile-time polymorphism would be</span><br style="font-size:12.8px"><span style="font-size:12.8px">insufficient.</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">CppCoreGuidelines C.10 suggests effectively the same thing: "You need a</span><br style="font-size:12.8px"><span style="font-size:12.8px">reason for using a hierarchy."</span></blockquote><div><br></div><div>I would agree that class hierarchies should be well designed and thought out, but I could say the same thing about every bit of code that is written, especially when it is part of the core structure or the externally exposed API. I would, however, argue that "justification required" does imply that the behavior is discouraged; at least my definition of discouraged in a style guide means that a justification is required to use it. Interfaces, and thus virtual functions are required for good unit testing and mocking, so as long as that is not being restricted in that sense I do not have any complaints, I am not sure where that falls on the spectrum of "requires justification".</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span style="font-size:12.8px">Again, this was not meant to be a total prohibition, unless it causes a</span><br style="font-size:12.8px"><span style="font-size:12.8px">systemic concern.  Using streams for a one-off serial / deserial</span><br style="font-size:12.8px"><span style="font-size:12.8px">function might be entirely reasonable (though we should use a</span><br style="font-size:12.8px"><span style="font-size:12.8px">serialization library instead).  The concern was that logging is a</span><br style="font-size:12.8px"><span style="font-size:12.8px">widely-used operation and the stream-style operators cause a big</span><br style="font-size:12.8px"><span style="font-size:12.8px">number of function call locations which result in much larger code than</span><br style="font-size:12.8px"><span style="font-size:12.8px">a direct call to a printf-style function.</span></blockquote><div><br></div><div>Fair enough, I tried to avoid characterizing your argument because I felt it was unfair and that you should be allowed to present it yourself.</div><div><br></div><div>Cheers </div></div>