On Monday, October 21, 2013 10:37:57 Chris wrote:
> Usually same-name
> variables are only used in the constructor. Using them in other
> places in the class is not recommendable

Well, that's up to the programmer, and plenty of folks use them in places like setters as well.

> but is it a good solution to categorically rule out warnings?

Honestly? Yes, I think that it is. I am of the opinion that the _only_ warnings that the compiler should have are things that will become errors later but aren't immediately in order to give programmers the chance to change their code before it breaks. For instance, override is now required on virtual functions which override a base class function, whereas before it wasn't - we used a warning to ease the transition, and that makes sense. But ultimately, it became an error.

Because it is not good practice to ever leave warnings in your code, I am firmly of the belief that something should be an error or completely legal and nothing in between. Otherwise, you're needlessly forced to change your code just because it _might_ have a problem.

Additional lint-like tools can be written and used to check for anything which "might" be wrong, and the compiler can be left to report only stuff that is definitively wrong. And once we have a D lexer and parser in Phobos (and we're getting close to having a lexer), writing such tools will become much easier, and then such tools can warn programmers about what _they_ want to be warned about but which isn't necessarily wrong.

- Jonathan M Davisbut is

On Monday, 21 October 2013 at 09:05:58 UTC, Jonathan M Davis wrote:
> On Monday, October 21, 2013 10:37:57 Chris wrote:
>> Usually same-name
>> variables are only used in the constructor. Using them in other
>> places in the class is not recommendable
>
> Well, that's up to the programmer, and plenty of folks use them in places like
> setters as well.
>
>> but is it a good solution to categorically rule out warnings?
>
> Honestly? Yes, I think that it is. I am of the opinion that the _only_
> warnings that the compiler should have are things that will become errors
> later but aren't immediately in order to give programmers the chance to change
> their code before it breaks. For instance, override is now required on virtual
> functions which override a base class function, whereas before it wasn't - we
> used a warning to ease the transition, and that makes sense. But ultimately,
> it became an error.
>
> Because it is not good practice to ever leave warnings in your code, I am
> firmly of the belief that something should be an error or completely legal and
> nothing in between. Otherwise, you're needlessly forced to change your code
> just because it _might_ have a problem.
>
> Additional lint-like tools can be written and used to check for anything which
> "might" be wrong, and the compiler can be left to report only stuff that is
> definitively wrong. And once we have a D lexer and parser in Phobos (and we're
> getting close to having a lexer), writing such tools will become much easier,


> and then such tools can warn programmers about what _they_ want to be warned
> about but which isn't necessarily wrong.
>
> - Jonathan M Davisbut is

That would be a good solution, so ideally programmers could have a list of "bad practice" and check against that too.

I only wonder which other use cases there are for same name variables other than constructors and setters.

void setValue(string input) {
  this.input = input;
}

But there you have this. But a function (in the same class) like

void processInput() {
  auto input = // ...
  // 20 lines later
  input = std.string.format("Hello %s!", someString);
}

Why would one want to write code like this? Why should a short-lived local variable assume the name of a class variable? This is a source of confusion and I don't consider this good practice. Maybe it's just personal taste.

On Monday, October 21, 2013 11:24:06 Chris wrote:
> But there you have this. But a function (in the same class) like
> 
> void processInput() {
>    auto input = // ...
>    // 20 lines later
>    input = std.string.format("Hello %s!", someString);
> }
> 
> Why would one want to write code like this? Why should a short-lived local variable assume the name of a class variable? This is a source of confusion and I don't consider this good practice. Maybe it's just personal taste.

That may very well be bad practice (I certainly wouldn't advise writing code like that), but it's also not definitively wrong. Unless it's definitely a bug, I think that the compiler should keep quiet about it, because anything that it yells about has to be treated as a bug, and we shouldn't be forcing people to change their code just because there _might_ be a bug in it.

- Jonathan M Davis

On Monday, 21 October 2013 at 09:36:48 UTC, Jonathan M Davis wrote:
> On Monday, October 21, 2013 11:24:06 Chris wrote:
>> But there you have this. But a function (in the same class) like
>> 
>> void processInput() {
>>    auto input = // ...
>>    // 20 lines later
>>    input = std.string.format("Hello %s!", someString);
>> }
>> 
>> Why would one want to write code like this? Why should a
>> short-lived local variable assume the name of a class variable?
>> This is a source of confusion and I don't consider this good
>> practice. Maybe it's just personal taste.
>
> That may very well be bad practice (I certainly wouldn't advise writing code
> like that), but it's also not definitively wrong. Unless it's definitely a bug,
> I think that the compiler should keep quiet about it, because anything that it
> yells about has to be treated as a bug, and we shouldn't be forcing people to
> change their code just because there _might_ be a bug in it.
>
> - Jonathan M Davis

So the solution would be an additional layer that can be turned on / off. Warnings like "unused variable" or "shadowing class declaration" are quite useful. I don't see any other cases where a warning would be nice (at the moment).

On Monday, October 21, 2013 12:50:44 Chris wrote:
> So the solution would be an additional layer that can be turned on / off. Warnings like "unused variable" or "shadowing class declaration" are quite useful. I don't see any other cases where a warning would be nice (at the moment).

Perhaps, but Walter is against adding much in the way of switches to dmd, and he has pretty much the same position on warnings that I do. Also, with a 3rd party tool, it'll be a lot easier to get warnings on exactly what you want, whereas with the compiler, you have to convince Walter that it should be a warning. Having a configurable tool for additional checks is just more flexible and avoids all of the arguments over what should and should be a warning.

Also, because of -w (which makes a warning an error), it would actually be _very_ bad for something like "unused variable" to be a warning, because then it would be an error, which would result in a _lot_ of traits failing to compile, because it's _very_ common for those to have unused variables. It also would cause problems with RAII. Ideally, -w wouldn't even exist, since it essentially changes what is and isn't legal in the language, but the fact that it does exist makes it that much more precarious to add warnings to the compiler.

- Jonathan M Davis

On Monday, 21 October 2013 at 10:50:45 UTC, Chris wrote:
> So the solution would be an additional layer that can be turned on / off. Warnings like "unused variable" or "shadowing class declaration" are quite useful. I don't see any other cases where a warning would be nice (at the moment).

And this additional layer is called "static analysis tool", should be configurable and does not belong to compiler.

Warnings must die.