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

Issue 12789 - Add -transition=nan flag to diagnose default-initialized floating-point variables. #3573

Closed
wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented May 23, 2014

@ghost
Copy link
Author

ghost commented May 23, 2014

Someone mentioned you can change some NaN flags at runtime to enable throwing. But there's two problems with this:

  • You have to run the app
  • You only get one exception, you won't get information about all the NaN values
  • NaN configuration at runtime has its share of bugs, some of which I've filed. They haven't been fixed yet.

@ghost ghost closed this May 23, 2014
@ghost ghost reopened this May 23, 2014
@ghost
Copy link
Author

ghost commented May 23, 2014

Ok updated. I have at least one left-over test-case that seems to cause a false-positive, it's when an anonymous union is used:

struct S
{
    union
    {
        float x; // warning emitted
        int y;
    }
}

Anyone know why sc->isunion is false with an anonymous union? I guess in this case scope becomes S, but how do I check if we're in an anon union?

@ghost
Copy link
Author

ghost commented May 23, 2014

This switch has helped me weed out some odd runtime results when using a physics library which was a port of a C codebase. See this diff.

@dnadlinger
Copy link
Member

Isn't -transition supposed to be dealing with language changes?

@ghost
Copy link
Author

ghost commented May 23, 2014

Isn't -transition supposed to be dealing with language changes?

I kind of followed the logic of "someone is transitioning from C to D". :)

Otherwise I'll have to invent a new flag, and that ends up becoming a bikeshed competition.

@mihails-strasuns
Copy link

This flag is supposed to contain exclusively migration helpers for language changes. For anything else you'd better go with DScanner.

@ghost
Copy link
Author

ghost commented May 23, 2014

For anything else you'd better go with DScanner.

Last I checked DScanner didn't do semantic analysis. So things like avoiding warnings in version (none) blocks becomes more difficult.

@ghost
Copy link
Author

ghost commented May 23, 2014

@Dicebot: But maybe the next go-to reply will become "You'd better go with SDC". Waiting eagerly for that talk. :P

@ghost
Copy link
Author

ghost commented May 23, 2014

Anyway this isn't a must-have feature, and was implemented in 2 minutes. So there! :>

@WalterBright
Copy link
Member

This kind of thing is a good idea, but properly belongs in a lint-like program.

And besides, nan's propagate through expressions, so if you have a result that's nan you know you got the nan from somewhere - it is not a silent failure.

@ghost
Copy link
Author

ghost commented May 24, 2014

This kind of thing is a good idea, but properly belongs in a lint-like program.

I can understand when code complexity is an issue, but this is a 5-liner and it requires some semantic analysis which I get for free in DMD. Oh well..

And besides, nan's propagate through expressions, so if you have a result that's nan you know you got the nan from somewhere - it is not a silent failure.

How in a 30KLoc codebase will you find the exact code that created a NaN? It is silent because what usually happens is nothing at all - silence at runtime and weird results.

NaNs don't always cause exceptions to be thrown because there are bugs such as Issue 9813, originally filed as Issue 6303 in mid 2011.

Anyway the flag was supposed to be used to help port C/C++ libraries where you don't get the benefit of having only a couple of NaNs, C/C++ codebases that deal with floating-point are littered with hundreds of 0-initialized floating-point variables.

@WalterBright
Copy link
Member

The "weird results" should be NaN results. NaN's can be produced by many operations, not just initialization. I don't agree that NaN's are weird - based on the idea that programmers who use floating point need to make an effort to understand it, and NaN's are basic to understanding it. Floating point arithmetic has quite a few behaviors that users need to understand in order to not get garbage for answers due to underflows, accumulated rounding errors, etc.

NaN's are actually the least of the issues, because if you get a NaN result you aren't fooled into thinking it is a correct answer. Default initializing to 0.0 is much, much worse, and even worse than that, is C and C++'s practice of default initializing them to undefined garbage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants