On Tue, Sep 27, 2011 at 05:32:19PM +0200, Nicolas Desprès wrote: > On Tue, Sep 27, 2011 at 3:55 PM, Rolf Eike Beer <e...@sf-mail.de> wrote: > >> From: Nicolas Despres <nicolas.desp...@gmail.com> > >> > >> Make it a static method instead of an array. It is safer for the > >> type checking and if we add a new target type we will be warned to add > >> a case to the switch. > > > >> + case cmTarget::UNKNOWN_LIBRARY: > >> + return "UNKNOWN_LIBRARY"; > >> + // break; /* unreachable */ > >> + } > >> + return 0; > >> +} > > > > Shouldn't this spit out a warning or error message? > > Frankly I don't know. I just refactor the code and kept the same > original behavior.
This function should not expect to be passed an invalid target type as a parameter, especially since we are covering all of the enumerators. I have modified this patch to add an assertion check that the end of the function is unreachable. While at it, I also removed the commented out break statements, which are just noise. The modified patch is now in my github branch. Thanks, -- Peter -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
