Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flags for unspecified behaviour #423

Closed
wants to merge 1 commit into from

Conversation

arielb1
Copy link
Contributor

@arielb1 arielb1 commented Oct 28, 2014

@Diggsey
Copy link
Contributor

Diggsey commented Oct 28, 2014

I think this would be great for improving debugging, however:

It's important that these flags don't change the behaviour of "correct" code, it should only affect how errors are reported in "incorrect" code. eg. if there's a compiler flag that causes signed overflow to fail, then any code which causes signed overflow should always be considered "incorrect" regardless of the compiler flags. Anything which changes the behaviour of "correct" code should be contained within the code itself, otherwise you're in the nasty situation where code has to be compiled with certain flags in order to work at all. This might mean that the result of signed overflow would be better controlled by attributes on modules/functions, rather than with a compiler flag.

For the OOB case, it's not clear to me how it would behave with overloaded operators if something like #392 were implemented: are "built in" arrays and slices special cased, or does the flag change which of the safe/unsafe versions of the operator are called?

@arielb1
Copy link
Contributor Author

arielb1 commented Oct 28, 2014

@Diggsey

It controls the behaviour of the built-in operators.

Having things controlled by module attributes doesn't look like a good option to me, because they don't interact well enough with calling functions between modules. If setting -S breaks your code, then your code is broken.

@arcto
Copy link

arcto commented Oct 28, 2014

I think it would be a good idea to allow a program to specify what behaviours it expects and requires. It would be important that these flags be specified in the crate source code itself, not as command-line options.

@arcto
Copy link

arcto commented Oct 28, 2014

After thinking some more, the granularity should probably not be at the crate level but smaller. Suppose that you want to mix code with different expectations.

@arielb1
Copy link
Contributor Author

arielb1 commented Oct 28, 2014

@arcto

Programs should not rely on these for proper operation, these flags are included mostly for debugging.

@thestinger
Copy link

I don't think there should be compiler flags giving undefined behaviour to code that's well-defined. There has been a strong consensus against exposing features like this. I don't think exposing new language dialects very compiler flags is a good idea at all.

@arielb1
Copy link
Contributor Author

arielb1 commented Oct 28, 2014

@thestinger

We want =undefined at least for the unsafe-by-default options (unsafe indexing/intrinsics), I guess we don't need to allow it for others.

@thestinger
Copy link

Unsafe indexing is a library feature so the compiler is the wrong place to put the functionality. Debugging pointer arithmetic could be done by implementing support for ASAN, but I don't think it's appropriate to expose that like this. I don't think the compiler should expose new language dialects with different arithmetic semantics either. Each configurable language feature adds a bit of diversity and all of the incompatible language dialects would need to be tested.

There are several things in Rust that don't have a well-specified behaviour.
Among them are:

* Signed Integer arithmetic overflow
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually well-specified as wrapping around: http://doc.rust-lang.org/reference.html#behaviour-not-considered-unsafe
I suppose this prevents x + 1 > x and (2*x) / 2 from being optimized to true and x respectively, and it might be interesting to investigate if such optimizations would really make existing Rust code faster. But for now it is well-specified.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that even if it wasn't mentioned in the reference, it would still be well-defined. The reference is not a language specification and does not cover most of Rust's semantics. A flaw in the reference does not change the definition of the language. The implementation is the closest thing to a specification right now.

It would be backwards incompatible to alter a guarantee that the compiler provides, even if it is not written down anywhere. Fixing undefined behaviour (not the same as unspecified behaviour) is not backwards incompatible. Any UB is an implementation soundness bug..

@Diggsey
Copy link
Contributor

Diggsey commented Oct 28, 2014

I think everyone agrees that having compiler flags change the behaviour of valid programs is undesirable.

However, the problem is that some things which are currently defined as "failing" (for example OOB array access) result in a not-insignificant performance penalty in order to correctly check the failure condition. This performance penalty is the motivation for unsafe indexing.

I would prefer a new kind of failure (let's call it "unsafe failure" for now), which is specified to either fail or be UB depending on compiler settings. (This does not change which programs are valid)

normal OOB indexing -> failure
unsafe OOB indexing -> unsafe failure

This unsafe failure would specifically cover cases which are definitively errors (unsafe OOB indexing, UB in unsafe intrinsics, etc.) but where the programmer favours speed over safety.

Behavioural issues, such as how signed overflow behaves should be seperate, and be controlled by fine-grained attributes within the code itself, but one choice of behaviour might be to cause unsafe failure, as this would be the most performant option.

@thestinger
I don't agree that the compiler is the wrong place to put this: the intended use is for debugging, and being able choose between fail-fast and high performance without having to modify the code is important. Additionally, I strongly agree with your objection about incompatible language dialects, but this is avoided in my suggestion above.

@nikomatsakis
Copy link
Contributor

The feedback here has been fairly mixed, and it seems that discussion has (long since) reached conclusion. Moreover, changes made as part of the overflow RFC have rendered a lot of the cases here moot -- the remaining cases have to do with unsafe pointer accesses, and it seems to me that there is really not a lot we can do in such cases. Therefore, I'm going to close this RFC. Thank you very much for the effort put into it, however.

wycats pushed a commit to wycats/rust-rfcs that referenced this pull request Mar 5, 2019
Update the RFC templates and README to reflect RFC Process Updates RFC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants