Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#7111 closed Feature Requests (fixed)

Switch case's default missing in date_time

Reported by: Gaurav Gupta <g.gupta@…> 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)

date_formatting.hpp_patch (430 bytes) - added by Gaurav Gupta <g.gupta@…> 6 years ago.
Fix for the reported Bug.

Download all attachments as: .zip

Change History (11)

Changed 6 years ago by Gaurav Gupta <g.gupta@…>

Attachment: date_formatting.hpp_patch added

Fix for the reported Bug.

comment:1 Changed 6 years ago by Marshall Clow

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 Changed 6 years ago by Gaurav Gupta <g.gupta@…>

Severity: ProblemCosmetic

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 Changed 6 years ago by viboes

Cc: viboes added
Resolution: invalid
Status: newclosed
Type: BugsFeature 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 Changed 6 years ago by Gaurav Gupta <g.gupta@…>

Resolution: invalid
Status: closedreopened

If our application is using "Werror" flag, then if default case is not mentioned ,it is reported as an error.

comment:5 Changed 6 years ago by Marshall Clow

Ok; I added a default case. But you should bug the gcc folks, since it's a bogus warning.

Last edited 6 years ago by Marshall Clow (previous) (diff)

comment:6 Changed 6 years ago by Marshall Clow

(In [80703]) Added default case label to silence bogus warning; Refs #7111

comment:7 Changed 6 years ago by Marshall Clow

Owner: changed from az_sw_dude to Marshall Clow
Status: reopenednew

comment:8 Changed 6 years ago by Gaurav Gupta <g.gupta@…>

Thanks Marshall, Please commit similar warning errors in Ticket# 7112 & 7113.

comment:9 Changed 6 years ago by Marshall Clow

Resolution: fixed
Status: newclosed

(In [80797]) Merge bug fixes to release; Fixes #5550 Fixes #6136 Fixes #6513 Fixes #7111 Fixes #7112 Fixes #7113 Fixes #7342 Fixes #7426

comment:10 Changed 6 years ago by Marshall Clow

BTW, clang now warns on this code.

Warning: default label in switch which covers all enumeration values [ -Wcovered-switch-default ]
Note: See TracTickets for help on using tickets.