[opensource-dev] Review Request: VWR-24312: Massively duplicated objects (part 2)

[Aleric Inglewood]

> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20110121/e32e1af1/attachment.htm