[llvm-commits] [llvm] r161097 - /llvm/trunk/include/llvm/ADT/StringSwitch.h

[Nick Kledzik]

On Jul 31, 2012, at 8:23 PM, Sean Silva wrote:
> Ugh. I kind of don't like having a warning for the shadow in this case.
> 
> It's actually a useful pattern to have the constructor parameter named
> the same as the member it initializes; otherwise you end up doing
> contortions coming up with a name, including being pushed to the limit
> of engendering undefined behavior
> <http://clang.llvm.org/doxygen/CodeGenAction_8cpp_source.html#l00268>
> out of desperation for coming up with a name.
Personally, I prefer a naming convention where ivars and locals/params are named differently, so conflicts like this never arise.

Some examples in other sources bases of ivar naming conventions:
	mFoo
	m_foo
	_foo
	foo_

In these cases, the constructor argument name is just "foo" and you get the associate of ivar and parameter.

One problem with the pattern of naming the constructor argument exactly the same as the instance variable, is that if the constructor body does contain code which modifies the variable, it is non-obvious which variable the compiler is actually modifying (hence the -Wshadow warning).  

-Nick

> 
> On Tue, Jul 31, 2012 at 6:43 PM, Nick Kledzik <kledzik at apple.com> wrote:
>> Author: kledzik
>> Date: Tue Jul 31 20:43:10 2012
>> New Revision: 161097
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=161097&view=rev
>> Log:
>> Fix shadowed variable warning
>> 
>> Modified:
>>    llvm/trunk/include/llvm/ADT/StringSwitch.h
>> 
>> Modified: llvm/trunk/include/llvm/ADT/StringSwitch.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringSwitch.h?rev=161097&r1=161096&r2=161097&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/StringSwitch.h (original)
>> +++ llvm/trunk/include/llvm/ADT/StringSwitch.h Tue Jul 31 20:43:10 2012
>> @@ -48,8 +48,8 @@
>>   const T *Result;
>> 
>> public:
>> -  explicit StringSwitch(StringRef Str)
>> -  : Str(Str), Result(0) { }
>> +  explicit StringSwitch(StringRef S)
>> +  : Str(S), Result(0) { }
>> 
>>   template<unsigned N>
>>   StringSwitch& Case(const char (&S)[N], const T& Value) {
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits