Bug 12550 – Deprecate -noboundscheck and replace with more useful -boundscheck= option

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2014-04-08T14:47:00Z
Last change time
2014-04-12T13:00:00Z
Assigned to
nobody
Creator
eco

Comments

Comment #0 by eco — 2014-04-08T14:47:38Z
-noboundscheck should be deprecated and replaced with -boundscheck=<all|safe|none> for a couple of reasons. 1. What -noboundscheck actually does is confusing. Its purpose is to turn off bounds checking in @safe code (and all other code) which comes as a surprise to a lot of people. -release turns off bounds checking in non-@safe code (which also surprises some people) but leaves it on for @safe code. 2. There is currently no way to turn on bounds checking for release builds currently. 3. There is currently no way to turn off bounds checking for non-@safe code without pulling in everything -release does (or turning off bounds checking for @safe code too). If we want to take this one step further there should be command line options (both enabling and disabling) for every effect -release has and -release should just be an alias for specifying a set of those options. (Martin Krejcirik came up with the idea for -boundscheck=)
Comment #1 by dev — 2014-04-09T08:26:14Z
IIRC Andrei has mentioned that bounds checking should be the default so the flags must be for disabling this behaviour. -noboundscheck=all -noboundscheck=unsafe
Comment #2 by schveiguy — 2014-04-10T15:14:54Z
I'd like to chime in here and say that allowing disabling bounds checks on @safe code is completely wrong, and should be treated as a bug. In no case, should one be able to disable bounds checks on @safe code. Otherwise, it immediately becomes un-@safe, and @safe loses all it's credibility. The original purpose of -noboundschecks was to allow turning off bounds checks in debug code I thought, not @safe code. I have no idea why it turns off bounds checks on @safe code. I would consider that a bug.
Comment #3 by monarchdodra — 2014-04-10T15:24:49Z
(In reply to Steven Schveighoffer from comment #2) > Otherwise, it immediately becomes un-@safe, and @safe loses all it's > credibility. Think of it as an option that makes as @safe code actually @trusted.
Comment #4 by schveiguy — 2014-04-10T16:15:52Z
(In reply to monarchdodra from comment #3) > Think of it as an option that makes as @safe code actually @trusted. No. @trusted code is trusted because the developer guarantees it. @safe code is @safe because the compiler guarantees it. @safe code that has no bounds checks basically becomes unqualified code that @safe code can call.
Comment #5 by monarchdodra — 2014-04-10T16:29:34Z
(In reply to Steven Schveighoffer from comment #4) > (In reply to monarchdodra from comment #3) > > > Think of it as an option that makes as @safe code actually @trusted. > > No. @trusted code is trusted because the developer guarantees it. @safe code > is @safe because the compiler guarantees it. Yeah, which is why the developer uses a compiler switch that says "no need to do any checking in the safe code, I'm guaranteeing I'm making all my calls in ways that won't break this safe code". So, yes, does basically become trusted, because the developer said so. > @safe code that has no bounds checks basically becomes unqualified code that > @safe code can call. *at the developper's request*. Yes. But like most things, there's a time and place for everything. At worst, it is a benching switch. Or a profiling switch to see how much the checking is costing.
Comment #6 by schveiguy — 2014-04-10T16:38:20Z
(In reply to monarchdodra from comment #5) > (In reply to Steven Schveighoffer from comment #4) > > No. @trusted code is trusted because the developer guarantees it. @safe code > > is @safe because the compiler guarantees it. > > Yeah, which is why the developer uses a compiler switch that says "no need > to do any checking in the safe code, I'm guaranteeing I'm making all my > calls in ways that won't break this safe code". So, yes, does basically > become trusted, because the developer said so. The developer is not always the builder. e.g. template code. Note that the developer has this option -- use @trusted instead of @safe. > But like most things, there's a time and place for everything. At worst, it > is a benching switch. Or a profiling switch to see how much the checking is > costing. Change @safe to @trusted in the code to test this.
Comment #7 by mk — 2014-04-10T17:04:33Z
(In reply to Steven Schveighoffer from comment #6) > The developer is not always the builder. e.g. template code. Also the builder is not always the developer. It's much easier (and I would say safer), changing a command line option, than changing the code, just to test how fast it runs. Sometimes you don't care for possible bugs, you just want it to run as fast as possible. The pull request explicitly warns that this option should be used with care. Also there is a discussion about version identifiers, which should allow the developer some control over this (for example emit a warning or stop the compilation in some super secure functions).
Comment #8 by schveiguy — 2014-04-10T17:21:54Z
(In reply to Martin Krejcirik from comment #7) > (In reply to Steven Schveighoffer from comment #6) > > The developer is not always the builder. e.g. template code. > > Also the builder is not always the developer. It's much easier (and I would > say safer), changing a command line option, than changing the code, just to > test how fast it runs. And incorrect. If you are trying to fix a performance issue, removing bounds checks on an entire program is much more damaging and fruitless than removing bounds checks on a bottleneck loop that you have read and understood. I find the argument unconvincing that someone who is working on performance is simply going to do this as a test measure before doing their *real* profiling task. We should do as much as possible to avoid blunt-instrument speedups that compromise the safety of systems that are *specifically* labelled safe without any care or thought. Focused optimization via profiling is a better approach in every case.
Comment #9 by code — 2014-04-10T18:25:09Z
Please consider that you will scare of useres that care for maximum performance (gaming, embended) if a noboundscheck does not turn off all bounds checks. If you really care about performance you will always be wondering how much time you loose from boundschecks in safe code, especially on mobile platforms branches can be very expensive. Additinally you should not take the "tight loop optimization" idiom to seriously. Modern optimized code bases often have a "general slowness" problem. Nothing is really the hot spot, and almost all code just runs to slow.
Comment #10 by eco — 2014-04-10T18:30:39Z
Forgot to post a link to the pull request. https://github.com/D-Programming-Language/dmd/pull/3443
Comment #11 by schveiguy — 2014-04-10T18:32:38Z
(In reply to Benjamin Thaut from comment #9) > Please consider that you will scare of useres that care for maximum > performance (gaming, embended) if a noboundscheck does not turn off all > bounds checks. Don't use @safe for tuned code.
Comment #12 by code — 2014-04-10T19:31:02Z
Thanks to attribute inference some stuff automatically becomes @safe. Also phobos and druntime use @safe. I don't want to pay the performance penatly for those.
Comment #13 by schveiguy — 2014-04-10T20:06:28Z
(In reply to Benjamin Thaut from comment #12) > Thanks to attribute inference some stuff automatically becomes @safe. Also > phobos and druntime use @safe. I don't want to pay the performance penatly > for those. Good point, but the compiler could easily compile on demand 2 versions, one that is @safe and one that isn't, using the unsafe version wherever possible. I'd rather see that in any case.
Comment #14 by github-bugzilla — 2014-04-12T12:59:59Z
Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/23748b988fe60b2f6ab697c482afe9511d3f444a Add -boundscheck=[on|safeonly|off] option Replaces -noboundscheck (which becomes deprecated) Fixes issue #12550