#7111 closed Feature Requests (fixed)
Switch case's default missing in date_time
Reported by: | Owned by: | Marshall Clow | |
---|---|---|---|
Milestone: | To Be Determined | Component: | date_time |
Version: | Boost 1.51.0 | Severity: | Cosmetic |
Keywords: | Cc: | yogen.saini@…, viboes |
Description
In file boost/date_time/date_formatting.hpp in following code block, default case of switch condition is missing.Switch case should not be used without default case ideally.In case of default, code should break from the loop.
static ostream_type& format_month(const month_type& month, ostream_type &os) { switch (format_type::month_format()) { case month_as_short_string: { os << month.as_short_string(); break; } case month_as_long_string: { os << month.as_long_string(); break; } case month_as_integer: { os << std::setw(2) << std::setfill(os.widen('0')) << month.as_number(); break; } } return os; } // format_month };
Attached patch is the fix for it.
Attachments (1)
Change History (11)
by , 10 years ago
Attachment: | date_formatting.hpp_patch added |
---|
comment:1 by , 10 years ago
I'm sorry, but I don't see the point of this change.
The code here is checking for all the possible values of the month_format_spec enum; there are no other (legal) values.
Also, your patch does not change the behavior of the code; if an invalid value was returned the by month_format()
, the current code would do nothing, and your patch would change it so that it would ... do nothing.
Please tell me what you are trying to accomplish here.
P.S. I am completely missing what you mean by:
In case of default, code should break from the loop.
since I don't see a loop here.
comment:2 by , 10 years ago
Severity: | Problem → Cosmetic |
---|
I just want to highlight the coding practices. Its a cosmetic bug. In future if somebody add more cases, it will be easier for the user as code will be more readable and behavior of switch will be visible. Its obvious that it is still not doing anything, but the readability is more and it is according to coding guidelines.
comment:3 by , 10 years ago
Cc: | added |
---|---|
Resolution: | → invalid |
Status: | new → closed |
Type: | Bugs → Feature Requests |
There are too much work to do to clean up Boost.DateTime. Re-open if some tool is warning a bad usage.
comment:4 by , 10 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
If our application is using "Werror" flag, then if default case is not mentioned ,it is reported as an error.
comment:5 by , 10 years ago
Ok; I added a default case. But you should bug the gcc folks, since it's a bogus warning.
comment:6 by , 10 years ago
comment:7 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
comment:8 by , 10 years ago
Thanks Marshall, Please commit similar warning errors in Ticket# 7112 & 7113.
comment:9 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:10 by , 10 years ago
BTW, clang now warns on this code.
Warning: default label in switch which covers all enumeration values [ -Wcovered-switch-default ]
Fix for the reported Bug.