Re: Fix up some fallout from "setup_git_directory()" cleanups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Mon, 31 Jul 2006, Junio C Hamano wrote:
> 
> This would have been prevented if the rest of the sources were -Wshadow
> clean.

Well, the _real_ problem is that "-Wshadow" is such a crappy switch.

For example, there is absolutely no reason to warn about every local 
variable called "link", just because it shadows the unistd.h "link()" 
function.

So I tend to think that -Wshadow is actively wrong, because it makes 
people write crappier code by forcing them to avoid not just the (few) C 
keywords as variable names, but also the _millions_ of names exported by 
various standard header files.

Which is sad. "-Wshadow" _would_ be useful, if it had been designed right. 
But that definitely implies _not_ warning about name clashes that are also 
obvious type clashes.

For example, there's basically absolutely zero reason to warn about 
something clashing with a function. If you are crazy and use nested 
functions in gcc, it's still obvious _which_ function you'd ever mean. So 
"-Wshadow" simply should _never_ warn about clashing with the external 
"link()" declaration, and certainly not for a regular "int link" kind of 
automatic variable definition.

There's also seldom (if ever) any reason to warn about a pointer variable 
being shadowed by an integer one. If you use the wrong one, you'll get a 
type warning, so why would you really care? 

> I once tried to clean things up, but there are tons of
> warnings in the current code [*1*].

And, looking at your patch, they all (or a huge majority of them) seem to 
be totally bogus (ie the code is fine, the -Wshadow warning is crap).

> Another thing that would help us is to have more tests that run
> things from subdirectories.  Any takers?

This, btw, didn't just hit "git ls-files" when run in a subdirectory, it 
hit pretty much any use of "git ls-files" with a path specifier that 
included a directory. I don't think we have many uses of that, and at 
least one of the few tests for path specifiers (t3002) only tests in the 
current directory, no paths.

			Linus
-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]