Bug 1849 – with() should cause shadowing errors if you use a member that's shadowed

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P2
Component
dmd
Product
D
Version
D1 (retired)
Platform
x86
OS
Windows
Creation time
2008-02-18T02:19:00Z
Last change time
2014-02-24T15:32:36Z
Assigned to
bugzilla
Creator
wbaxter

Comments

Comment #0 by wbaxter — 2008-02-18T02:19:31Z
Sorry for the horrible summary. Kudos to anyone who can think of a better one. Issue is this: ---- module shadow2; struct Foo { static Foo opCall(int numRows, int numCols) { Foo R; with (R) { numRows_ = numRows; numCols_ = numRows; } return R; } int numRows() { return numRows_; } int numCols() { return numCols_; } private: int numRows_ = -1; int numCols_ = -1; } void main() { auto M = Foo(10,20); assert(M.numRows == -1, "numRows Shouldn't be -1, actually"); assert(M.numCols == -1, "numCols Shouldn't be -1, actually"); assert(M.numRows == 10, "numRows Should be 10 but it isn't"); assert(M.numCols == 20, "numCols Should be 20 but it isn't"); } ---- What happens is the with() there in the static opCall causes R.numRows() to shadow the parameter named 'numRows'. So you're actually just setting numRows_ to what it already was. This is pretty evil that this happens silently, so I think it should generate a shadowing error. But the tricky part is that error should only be generated if you try to *use* a shadowed variable. If errors were triggered even when no shadowed variable was used then with() could become practically useless because such many such "potential shadowings".
Comment #1 by bugzilla — 2008-03-02T23:35:03Z
The problem is even if the compiler did generate a warning, it would be problematical because one of the whole points of with is to do this (in a scoped manner, of course).
Comment #2 by wbaxter — 2008-03-02T23:48:10Z
(In reply to comment #1) > The problem is even if the compiler did generate a warning, it would be > problematical because one of the whole points of with is to do this (in a > scoped manner, of course). You could say the same thing about plain-old {..} scopes, though. The whole point of a scope is to create a new (scoped) namespace, and yet D doesn't allow shadowing in those cases. At least I don't *think* you're saying that the point of with() is to shadow variables in outer scopes. If that's what you're saying then I guess I just don't really agree that that's a desirable thing.
Comment #3 by bugzilla — 2008-03-03T02:01:02Z
With the shadowing rules as they exist, it's a very local edit to change the name of something. But with with shadowing, it's not a local edit - you're looking at changing the member names, or names in some enclosing scope.
Comment #4 by wbaxter — 2008-03-03T02:29:35Z
(In reply to comment #3) > With the shadowing rules as they exist, it's a very local edit to change the > name of something. But with with shadowing, it's not a local edit - you're > looking at changing the member names, or names in some enclosing scope. Right, which is why I suggest only issuing the shadowing error if the member doing the shadowing is actually used inside the with{} block. It's very dangerous the way it is. Adding a member to an object in file A.d could cause behavior of code in B.d to change, without creating any sort of compiler errors. That seems to be the kind of hijacking effect that usually gets you up in arms, but here you don't seem so concerned about it for some reason.
Comment #5 by bugzilla — 2008-03-04T21:04:08Z
I agree it can be a problem, and some people argue that the with itself is inherently a bad construct for such reasons. They might be right.
Comment #6 by wbaxter — 2008-03-04T21:10:23Z
(In reply to comment #5) > I agree it can be a problem, and some people argue that the with itself is > inherently a bad construct for such reasons. They might be right. They may be. If there were a way to do "selective with" like selective import I would probably use that. with(BigObject : member, other_member) { ... } Or if I could alias dot expressions like: alias foo.bar bar then I wouldn't be tempted to use 'with' as often.
Comment #7 by dhasenan — 2008-03-05T07:47:57Z
[email protected] wrote: > http://d.puremagic.com/issues/show_bug.cgi?id=1849 > > > > > > ------- Comment #6 from [email protected] 2008-03-04 21:10 ------- > (In reply to comment #5) >> I agree it can be a problem, and some people argue that the with itself is >> inherently a bad construct for such reasons. They might be right. > > They may be. If there were a way to do "selective with" like selective import > I would probably use that. > with(BigObject : member, other_member) { ... } > > Or if I could alias dot expressions like: > alias foo.bar bar > > then I wouldn't be tempted to use 'with' as often. If you could explicitly say that something is in an external scope, that would also help. For instance: struct Foo { int a; } Foo myStruct; int a = 15; with (myStruct) { a = .a; // myStruct.a == 15 } Or simply saying any ambiguity is a compile-time error.
Comment #8 by andrei — 2011-01-08T15:13:52Z
2.051 refuses compilation with test.d(8): Error: with symbol shadow2.Foo.numRows is shadowing local symbol shadow2.Foo.opCall.numRows test.d(9): Error: with symbol shadow2.Foo.numRows is shadowing local symbol shadow2.Foo.opCall.numRows