> On Jan. 14, 2011, 1:31 p.m., Boroondas Gupte wrote: > > indra/llcommon/lllslconstants.h, line 184 > > <http://codereview.secondlife.com/r/81/diff/1/?file=392#file392line184> > > > > Yay for having type modifiers after the core type name. Makes them much > > easier to understand, especially when there are several cascaded ones. :-) > > Aleric Inglewood wrote: > Yeah, I'm strongly convinced that TYPE const is superior in anyway over > const TYPE. > See http://www.xs4all.nl/~carlo17/cpp/const.qualifier.html for the > reasoning. > In one line: all type qualifiers work to the left, it's best to PUT them > on the > left so the whole type is logically uniform in it's construction. The > fact that > you can legally type 'const TYPE' is just a historically grown misfortune. > >
Of course I meant.. "All type qualifiers work to the left, so it's best to PUT them on the _right_ ...", as in: TYPE QUALIFIER : The Qualifier changes the TYPE on it's left, so that what first was TYPE now becomes TYPE QUALIFIER. [Where "QUALIFIER" is not just const, volatile, restrict, but also * and &. The only exception where any qualifier works to the right is where 'const' (volatile and restrict, but NOT * an &) works on a base type. Surely it needs getting used when one changes style, but this one is so logical that already after a single week you don't understand anymore how you were ever able to use to previous format style. While on the topic of coding style and types. The above is a good reason to put * (and &) that are part of types *against* the TYPE they work on. That way one can easily recognize the difference between the unary operator, the binary operator and the type qualifier: A* b = *c * d + e; // The type qualifier has no space on it's left (and // changes the type A), the binary operator (multiplication) // has spaces on both sides, while the unary operator // (dereference) works to it's right side and has no // space on the right.] [[ Ie, #include <iostream> struct A { int i; }; int main() { A const a[7] = { 0, 1, 2, 3, 4, 5, 6 }; int const two = 2; int const* c = &two; int const d = 3; A* const& e = &a[0]; A* b = *c * d + e; std::cout << b->i << std::endl; } ]] - Aleric ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://codereview.secondlife.com/r/81/#review153 ----------------------------------------------------------- On Jan. 16, 2011, 5:53 a.m., Aleric Inglewood wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://codereview.secondlife.com/r/81/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2011, 5:53 a.m.) > > > Review request for Viewer. > > > Summary > ------- > > Turns out that most of my SNOW-800 patch was included in Viewer 2 (albeit > without crediting me). > However, not everything was used and some more cleaning up was possible. > > After this patch, and when compiling with optimization, there are no > duplicates left > anymore that shouldn't be there in the first place: apart from the debug > stream > iostream guard variable, there are several static variables with the same > name (r, r1, > r2, etc) but that indeed actually different symbol objects. Then there are a > few > constant POD arrays that are duplicated a hand full of times because they are > accessed with a variable index (so optimizing them away is not possible). I > left them > like that (although defining those as extern as well would have been more > consistent > and not slower; in fact it would be faster theoretically because those arrays > could > share the same cache page then). > > > This addresses bug VWR-24312. > http://jira.secondlife.com/browse/VWR-24312 > > > Diffs > ----- > > doc/contributions.txt 422f636c3343 > indra/llcharacter/llanimationstates.cpp 422f636c3343 > indra/llcommon/llavatarconstants.h 422f636c3343 > indra/llcommon/lllslconstants.h 422f636c3343 > indra/llcommon/llmetricperformancetester.h 422f636c3343 > indra/llmath/llcamera.h 422f636c3343 > indra/llmath/llcamera.cpp 422f636c3343 > indra/newview/llviewerobject.cpp 422f636c3343 > indra/newview/llvoavatar.cpp 422f636c3343 > indra/newview/llvosky.h 422f636c3343 > indra/newview/llvosky.cpp 422f636c3343 > > Diff: http://codereview.secondlife.com/r/81/diff > > > Testing > ------- > > Compiled with optimization and then running readelf on the executable to find > duplicated symbols. > > > Thanks, > > Aleric > >
_______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges