-
Notifications
You must be signed in to change notification settings - Fork 301
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
DirectMLX.h Split fix a tiny bug which caused by variable 'axisSizeSum' set but not used #223
base: master
Are you sure you want to change the base?
Conversation
@@ -2342,7 +2341,14 @@ namespace dml | |||
axisSizeSum += outputAxisSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this now fail to compile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concur, as I see axisSizeSum
being used on line 2342.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue #220 describes the detailed error log. TheaxisSizeSum
used in 2342 only be assignment for the newer clang. The axisSizeSum
only be used for assert
statement on line 2345.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 2341 should be deleted now.
@@ -2342,7 +2341,14 @@ namespace dml | |||
axisSizeSum += outputAxisSize; | |||
} | |||
|
|||
#if defined(_DEBUG) | |||
uint32_t axisSizeSum = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: tab
@@ -2342,7 +2341,14 @@ namespace dml | |||
axisSizeSum += outputAxisSize; | |||
} | |||
|
|||
#if defined(_DEBUG) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#ifndef NDEBUG
would be better here, since NDEBUG corresponds for C-style assertions, and _DEBUG
is a non-standard macro defined by Visual Studio, but not necessarily other compilers. Also you'll need to delete line 2341.
No description provided.