[llvm] r176408 - recommit r172363 & r171325 (reverted in r172756)

Shuxin Yang shuxin.llvm at gmail.com
Mon Mar 4 10:29:34 PST 2013


Partially true.

It is not completely about name, it is about mixing different stuff in a 
single function, and the function
is meaning less in come context, but still magically "works", in the 
sense it dose not expose bug.

  Often time I see following situations happen. Mapping to what is 
happening today,
the class we are talking about  was in stage 1. With this change, it 
involves into 2.

    1) a file/class starts with a clean design and implementation, and 
It is working for one instance at a time.
    2) later on, an engineer change a function, which is supposed to 
work with a single instance,
        to be a aggregate function. Say min/max of a particular aspect 
of the set of instances.
        The function name no longer precisely reflect the original meaning.

    3) this function further involve into aggregation function which 
care some common aspects
        a set of instances are involved instead of min/max.

        The name deviate from its original meaning further.

    4) couple years later, people show interest in another set of 
aspects of these instances,
      and the function  are heavily changed. At the end of day, this 
function/class become a load
      of crap, nobody dare to touch.



On 3/4/13 10:02 AM, Arnold Schwaighofer wrote:
> On Mar 4, 2013, at 11:27 AM, Nuno Lopes <nunoplopes at sapo.pt> wrote:
>
>>>> Shuxin: I'm not breaking any assumption here.
>>> If you don't think you break the assumption,  tell us your immediate reaction to
>>> the class name "ObjectSizeOffsetVisitor".
>>>
>>> It seem so odd to me that  xxx_ObjectWhatever() return a meaningful value however we have no idea which
>>> object it is talking about.
>>>
> Nuno,
>
> I think, the issue is mostly one of naming things as precisely as possible or at least have the documentation describe it as accurate as possible.
>
> If we call a function "getObjectSize" vs. "getPtrAdjustedSizeOfPointedToByObject(s)", this can make a big difference. I claim it could have prevented the basicaa bug we had: I think it was probably due to the misconception that getObjectSize will always return the size of the pointed to underlying object - as it turns out this was not the case  - or it has evolved over time not to be the case anymore. Either way, by naming things more precisely we can avoid future bugs.
>
> That is all I am voting for.
>
> Best,
> Arnold




More information about the llvm-commits mailing list