-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Support for variadic version of the Format function #10
Comments
To have performance comparable to that of sprintf, Formatting API has no direct equivalent in IOStreams. IOStreams use concatenation-based API which is more comparable to that of fmt::Writer w; w << arg1; w << arg2; ... That said I'd very much want to add support for variadic version of the inline void CollectArgs(std::deque<fmt::BasicFormatter<char>::Arg> &) {} template<typename ArgT, typename... Args> inline void CollectArgs( std::deque<fmt::BasicFormatter<char>::Arg> &format_args, const ArgT &arg, const Args & ... args) { format_args.emplace_back(arg); CollectArgs(format_args, args...); } template<typename... Args> inline std::string Format(const StringRef &format, const Args & ... args) { std::deque<fmt::BasicFormatter<char>::Arg> format_args; CollectArgs(format_args, args...); fmt::Writer w; fmt::BasicFormatter<char> f(w, format.c_str()); for (const fmt::BasicFormatter<char>::Arg &arg : format_args) f << arg; return fmt::str(f); } I think there are better ways to implement this that will require minor extension of the BasicFormatter API, so I'll leave this issue open for now. |
I've do CollectArgs as template<typename ArgT, typename... Args>
inline void CollectArgs(
std::deque<fmt::BasicFormatter<char>::Arg> &format_args,
const ArgT &arg, const Args & ... args) {
format_args.emplace_back(arg);
if (sizeof...(Args)>0) { CollectArgs(format_args, args...); }
} The the condition is either true or false, and the compiler optimization will eliminate it And then Format would become template<typename... Args>
inline std::string Format(const StringRef &format, const Args & ... args) {
if (sizeof...(Args)>0) {
std::deque<fmt::BasicFormatter<char>::Arg> format_args;
CollectArgs(format_args, args...);
fmt::Writer w;
fmt::BasicFormatter<char> f(w, format.c_str());
for (const fmt::BasicFormatter<char>::Arg &arg : format_args)
f << arg;
return fmt::str(f);
} else { return fmt::str(format); }
} The code is easier to understand this way I believe. The compiled output should be the same however, except for Format with no args which would be sightly more efficient. As it doesn't construct then illiterate the empty format_args, or attempt to format a string with no args, however it also eliminate any error checking if a string is passed with formatting tokens. As for better ways to implement I would go for adding an apply formatting member to the template<typename... Args>
inline std::string Format(const StringRef &format, const Args & ... args) {
if (sizeof...(Args)>0) {
std::deque<fmt::BasicFormatter<char>::Arg> format_args;
CollectArgs(format_args, args...);
fmt::Writer w;
fmt::BasicFormatter<char> f(w, format.c_str());
/* Here we replace the internal list, and then format the string,
should be more efficient as we replace the list rather than
rebuilding/copying the list
*/
f.apply_args(format_args);
return fmt::str(f); } else { return fmt::str(format); }
} |
I think it is more consistent if the Format with no args checks the format string. Having the same function behave differently depending on the number of arguments may be confusing to users. Besides, other formatting functions such as I agree that something like |
OK here's the initial attempt to provide support for variadic functions in It makes writing the variadic template<typename... Args> inline std::string Format(const StringRef &format, const Args & ... args) { fmt::Writer w; fmt::BasicFormatter f(w, format.c_str(), {args...}); return fmt::str(f); } |
Dose the rest of the code depend upon C++11?, if not you should wrap the new code in preprocessed tags to verify that C++11 is supported, as I known VC++2010 (still in use by lots of people I believe) doesn't support C++11. And this change would break the library for them. |
The rest of the code is C++98, so that's right, the C++11 part should be conditionally compiled. |
VC2010 can be easily disregarded after VC2013 came out - ferchrissakes it's basically 2014 now! I would also vote for the use of C++11 wherever it makes sense and is at least supported by the latest compilers. With conditionals you can maintain C++98 compatibility as long as your heart desires :D |
I can confirm that the variadic format now works on cygwin 1.7.25, compiled with g++ 4.7.3 using -std=gnu++11 to provide C++11 support. using the following code: template<typename... Args> inline std::string const Format(std::string const &format,Args const & ... args) {
fmt::Writer w;
fmt::BasicFormatter<char> f(w, format.c_str(), {args...});
return fmt::str(f); }
template<typename... Args> inline std::string const Format(uint16_t const format,Args const & ... args) { return Format(GetMsg(format),args...); }
template<typename... Args> inline std::string const Format(char const * format,Args const & ... args) { return Format(GetMsg(format),args...); } and } catch (std::exception &e) {
try {
_LogError(LANG_ID::FATAL_EXCEPTION,e.what());
OutputBacktrace(std::cout); // Wrtie up 200 levels of backtrace to STDOUT
} catch (std::exception &ee) {
std::cout << "ERROR: An unhandled exception has occured" << std::endl;
std::cout << "\t" << e.what() << std::endl;
std::cout << "ERROR: " << ee.what() << std::endl;
}
return EXIT_FAILURE;
} and virtual const char* what() const throw() { return I18N::Format(LANG_ID::THREAD_EXCEPTION,this->name,this->code).c_str(); } where
and
and Generates the output due to an exception
Overall this confirms the code works with 0,1 and 2 arguments in my compilation. |
Cool, thanks for checking it out. I think it's time to close the issue then. |
The stream operator << dosen't work as expected, based on the use being equivalent to std::ostream.
throws exception, to many formatting tokens on the second line, as it immediately attempts to format the string, but not all arguments have been passed.
P.S. I need to use this method as my code is actually
Where RescursiveFormat(msg,args...); reduce to (by template meta programing, and compiler optimization)
For however may arguments are passed.
This allows formatting to be done by
Format(<format_string>,...) i.e. like sprintf, by type-safe, and using format as the underlining formatting method.
The text was updated successfully, but these errors were encountered: