[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