Bug 4595 – [tdpl] Accessing non-static member of a null reference compiles

Status
RESOLVED
Resolution
WONTFIX
Severity
enhancement
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
Other
OS
All
Creation time
2010-08-07T17:02:48Z
Last change time
2019-12-10T13:45:36Z
Keywords
accepts-invalid, TDPL
Assigned to
No Owner
Creator
Andrej Mitrovic

Comments

Comment #0 by andrej.mitrovich — 2010-08-07T17:02:48Z
This should not compile, DMD should detect that I'm trying to access a member of a null reference (stated in TDPL): class A { int x = 42; } unittest { A a; a.x = 5; } void main() { } Runtime error: object.Error: Access Violation
Comment #1 by andrej.mitrovich — 2010-08-07T17:05:41Z
And an additional example: class A { int x; } unittest { A a; assert(__traits(compiles, a.x = 5)); } void main() { } Compiles and runs succesfully. assert should fail.
Comment #2 by nfxjfg — 2010-08-07T18:46:05Z
You're obviously free to cause all access violation you want. dmd never prevented that, and no non-nullable references feature is planned.
Comment #3 by bearophile_hugs — 2010-08-07T19:21:57Z
You are mostly right nfxjfg, but please don't be harsh with people that use their time to submit problems in Bugzilla :-) See enhancement request bug 4571 for a start of a proposal about non-null types modifier in D. It needs more work, of course. And even if D will never have non-null references in its type system, DMD is already able to catch cases like this, if you compile it with -O: class Foo { int x; } void main() { Foo f; f.x = 5; } dmd 2.047 doesn't show a run-time sefault, it prints at compile-time: test.d(4): Error: null dereference in function _Dmain So adding few more logic to the compiler to catch other common cases of uninitialized bugs is not inconceivable. The case shown in this bug reports are well within the capabilities of a short analysis, good lint tools are able to warn against far more complex cases. So I think this enhancement request should be reopened.
Comment #4 by nfxjfg — 2010-08-07T19:51:33Z
(In reply to comment #3) > dmd 2.047 doesn't show a run-time sefault, it prints at compile-time: > test.d(4): Error: null dereference in function _Dmain I'd consider that a codegen bug. What if my program's semantics depend on the segfault?
Comment #5 by issues.dlang — 2010-08-07T20:37:32Z
No offense, but relying on a segfault seems rather silly. And if you really need one, I'm sure that you could easily produce one which the compiler can't catch (probably all it would take would be a function which returned null being used to initialize the variable rather than just declaring the variable). I think that it's perfectly reasonable for dmd to catch simple and obvious cases where a null reference or pointer is going to be dereferenced. In other languages trying to be safe (such as Java or C#), they do enough code analysis to scream at you if do something like that and force you to initialize the variable. D goes the route of default initialization to the closest thing to an error value rather than forcing you to initialize variables before they're used, but I think that it's perfectly reasonable for the compiler to catch simple and obvious cases and scream at you about them. I believe that the main reason that dmd doesn't do it in the general case is because it's too hard for the compiler to accurately catch more complex cases without making the compiler much more complex in its code flow analysis (and Walter doesn't want that kind of complexity). There's no question that you cannot rely on the compiler to catch all of the references and pointers that you forgot to initialize, but I see nothing wrong with it catching some of them.
Comment #6 by bearophile_hugs — 2010-08-07T21:02:16Z
> What if my program's semantics depend on the segfault? That means relying on undefined or OS/machine-dependant behaviour. You are not programming in D any more. I reopen this, but as enhancement request, because while I don't think that Walter will implement this very soon, it's a legit feature request for a D compiler to catch at compile-time one more case of uninitialized object reference.
Comment #7 by nfxjfg — 2010-08-07T21:46:00Z
(In reply to comment #6) > That means relying on undefined or OS/machine-dependant behaviour. You are not > programming in D any more. Then what is supposed to happen on a null reference access in D according to your opinion?
Comment #8 by bearophile_hugs — 2010-08-08T06:00:38Z
Generally the D program stops, some things happen deterministically inside the GC and the D code and the underlying C runtime, some events happen to the threads of your D code in a partially deterministic way, but then what exactly happens is specified by the Operating System and Memory Management Unit of your computer.
Comment #9 by andrej.mitrovich — 2010-08-08T09:16:57Z
This is not a feature request. The TDPL specifically states that in *simple* cases such as these, DMD should flag this as an error. Otherwise if it can't figure out if it indeed is a null-reference, it will compile and let the user handle it. I usually compile with all warnings on. Thanks for the -O trick, bearophile. I never thought adding optimizations can catch bugs like that. :)
Comment #10 by bearophile_hugs — 2010-08-08T12:05:25Z
With -O -unittest DMD is already able to catch the first of the two shown cases (it doesn't catch the case with __traits(compiles, a.x = 5).
Comment #11 by bearophile_hugs — 2010-09-21T04:45:47Z
See also bug 4906
Comment #12 by andrei — 2011-12-18T20:20:09Z
Failing unittest in TDPL: unittest { class A { int x; } A a; assert(!__traits(compiles, a.x = 5)); } Such programs must be statically rejected, guaranteed if there's no intervening flow in between definition and first use. We can work on improving that later, but for now let's get the obvious case out of the way.
Comment #13 by andrej.mitrovich — 2013-09-17T12:54:33Z
(In reply to comment #12) > Failing unittest in TDPL: > > unittest > { > class A { int x; } > A a; > assert(!__traits(compiles, a.x = 5)); > } > > Such programs must be statically rejected, guaranteed if there's no intervening > flow in between definition and first use. We can work on improving that later, > but for now let's get the obvious case out of the way. After a few years of D use, I actually find the above will create problems. We often use traits like these to check whether something is compilable, even if the object is left uninitialized. For example if you want to check whether method "foo" of object "c" is callable with an int type: static assert(__traits(compiles, c.foo(1))); Making this fail now because of flow analysis could hurt code that does unittesting. I think a runtime exception is sufficient right now, and the OP feature could be part of some specialized flow-analysis tool.
Comment #14 by k.hara.pg — 2013-09-19T00:33:55Z
(In reply to comment #13) > After a few years of D use, I actually find the above will create problems. We > often use traits like these to check whether something is compilable, even if > the object is left uninitialized. Today, code flow analysis for class ctor call (super(...) and this(...)) behaves as follows. class Foo { this(int) {} } class Bar : Foo { this() { static assert(is(typeof(super(0)))); static if (is(typeof(super(0)))) {} // inside typeof(), code flow analysis never works. super(1); // OK static assert(is(typeof(super(0)))); static if (is(typeof(super(0)))) {} // also OK super(2); // Error: multiple constructor call } this(int) { static assert( __traits(compiles, super(1))); // inside __traits(compiles), code flow analysis is enabled. super(1); // Error: multiple constructor call static assert(!__traits(compiles, super(2))); // The _second_ super call makes error. super(3); // Error: multiple constructor call } } This is expected behavior, from my compiler-internal knowledge. - `is(typeof(exp))` tests that the exp has valid type or not. For type calculation, the code flow analysis and its validation result is just unnecessary. - `__traits(compiles, exp)` tests the exact validness of `exp` at there. In other words, `__traits(compiles)` converts the error occurrence on the `exp` semantic analysis to the compile-time boolean value. In there, code flow analysis is enabled, and the errors will be counted normally.
Comment #15 by andrej.mitrovich — 2013-09-19T03:52:35Z
(In reply to comment #14) > - `is(typeof(exp))` tests that the exp has valid type or not. For type > calculation, the code flow analysis and its validation result is just > unnecessary. > > - `__traits(compiles, exp)` tests the exact validness of `exp` at there. In > other words, `__traits(compiles)` converts the error occurrence on the `exp` > semantic analysis to the compile-time boolean value. In there, code flow > analysis is enabled, and the errors will be counted normally. Well that explains why sometimes is(typeof()) works where __traits(compiles) doesn't. But as far as I know none of this is properly documented, and it's probably why people file bugs when a difference in behavior is found.
Comment #16 by andrej.mitrovich — 2014-01-23T08:13:19Z
*** Issue 11975 has been marked as a duplicate of this issue. ***
Comment #17 by andrej.mitrovich — 2016-08-27T21:19:53Z
Well, six years after filing this I'm having second thoughts. Is anyone aware of competing language compilers which do this type of analysis at compile-time? I'm aware of static analysis tools (like pvs-studio), which make me think we should have something like that for D. A tool, rather than something built-in to the compiler. I can imagine having this in the compiler would be complicated to implement and have a lot of missed cases, or worse, false-positives.
Comment #18 by razvan.nitu1305 — 2019-12-10T13:45:36Z
I'm going to close this since Walter is against dataflow analysis in this sort of situations.