Skip to content
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

Feature Request: support for manual creation of ArgList #528

Closed
tnovotny opened this issue Jun 25, 2017 · 21 comments
Closed

Feature Request: support for manual creation of ArgList #528

tnovotny opened this issue Jun 25, 2017 · 21 comments

Comments

@tnovotny
Copy link
Contributor

tnovotny commented Jun 25, 2017

I need a formatter interface. I would like to use fmtlib as one of the formatters. Maybe I missed something, but I was unable to find a way to build up an ArgList programmatically without accessing internals of fmtlib. Here is the general idea of what I am trying to accomplish:

struct ArgBuilder {

   using Formatter = fmt::BasicFormatter<char, fmt::ArgFormatter<char>>;
   using MakeArg = fmt::internal::MakeArg<Formatter>;

   ArgBuilder()
   : types_{}
   , index_{} {
   }

   // some functions to add an Arg    

   void
   append(std::string_view view) {
      _append(fmt::StringRef(view.data(), view.size()));
   }

   void
   append(int num) {
      _append(num);
   }

   // some method to build the ArgList

   fmt::ArgList
   build() const {
      if (index_ <= 16) return {types_, values_.data()};

      return {types_, args_.data()};
   }

  private:
   template <typename T>
   void
   _append(T t) {
      if (index_ < 16) {
         auto val = MakeArg(t);
         values_[index_] = val;
         types_ += (static_cast<uint64_t>(val.type) << (static_cast<uint64_t>(index_) << 2ull));
         ++index_;
      } else {
         adjust_size();

         auto arg = MakeArg(t);
         args_[index_] = arg;
         ++index_;
      }
   }

   void
   adjust_size() {
      args_.resize(index_ + 1);

      if (index_ == 16) {
         for (int i = 0; i < 16; ++i) {
            fmt::internal::Arg arg;
            fmt::internal::Value& val = arg;

            val = values_[i];
            args_[i] = arg;
         }
      }
   }

   int index_;
   std::array<fmt::internal::Value, 16> values_;
   std::vector<fmt::internal::Arg> args_;
   uint64_t types_;
};

With the help of the ArgBuilder code like

fmt::format("{0}{1}{0}", "abra", "cad")

can be written as

ArgBuilder args;
args.append( "abra" );
args.append( "cad" );

fmt::format("{0}{1}{0}", args.build());
@tnovotny tnovotny changed the title Feature Request : support for manual creation of ArgList Feature Request: support for manual creation of ArgList Jun 26, 2017
@gabime
Copy link
Contributor

gabime commented Jul 1, 2017

I second this - This is a much needed feature

@tnovotny
Copy link
Contributor Author

tnovotny commented Jul 1, 2017

@gabime : Glad you like the idea. This happened while I was on an effort to put spdlog behind an interface (that I want to use with boost:dll)

I have added support for named arguments. I am willing to write up a pull request.

@gabime
Copy link
Contributor

gabime commented Jul 1, 2017

I am willing to write up a pull request.

Is it similiar to this pr?

@tnovotny
Copy link
Contributor Author

tnovotny commented Jul 1, 2017

Is it similiar to this pr?

I didn't check that deeply, but I don't think so as this was meant for fmtlib and would be a more developed version of the code above. Basically I want to move away from an existing logging framework but need to keep the existing loggers formatting for a transitional period. The formatting would be done before the logging, providing a nice separation of concerns.

Basically this can already be done now, but not without dragging in fmtlib dependencies everywhere.

logger->log( fmt::format("{0}{1}{0}", ... ) );

So I want to be able to write it like this

logger->log( "{0}{1}{0}", ... );

when the logger configured for fmtlib formatting and like this

logger->log( "%s%s%s", ... );

when the logger is configured for printf formatting.

With a formatter interface and minimal templates most of the work is done.

template <typename... Args>
void
format_( IFormatter& ) {
}

template <typename Arg, typename... Args>
void
format_( IFormatter& fmt, Arg arg, Args... args ) {
   fmt.add_arg( arg );
   format_( fmt, args... );
}

template <typename... Args>
std::string
format( IFormatter& fmt, Args... args ) {
   format_( fmt, args... );
   return fmt.to_string();
}

FmtFormatter operator"" _fmt( const char* str, std::size_t len ) {
   return FmtFormatter( std::string( str, len ) );
}

int
main( int argc, char const** argv ) {
   auto s = format( "{0}{1}{0}"_fmt, "abra", "cad" );
} 

@vitaut
Copy link
Contributor

vitaut commented Jul 1, 2017

Thanks for the suggestion. One problem with this is the lifetime of arguments. Since Arg stores a reference to the object, the following will result in use after free:

std::string s;
ArgBuilder args;
args.append(s + "a");
...

Do you have any ideas how to address this?

@foonathan
Copy link
Contributor

Prevent rvalues when calling the builder? You can still shoot yourself in the foot but not as easily.

@tnovotny
Copy link
Contributor Author

tnovotny commented Jul 1, 2017

I'm not sure whether this is better than preventing, but one could just hold on to the temporaries.

struct ArgBuilder {
  
   using Any = std::variant<std::string, ...>;

   void
   append( std::string&& name ) {
      holder_.emplace_back( std::move( name ) );
      std::string& held = std::get<std::string>( holder_.back() );
      _append( fmt::StringRef( held.data(), held.size() ) );
   }

   std::deque<Any> holder_;
};

but that leads to an ambiguous overload ....

1>Main.cpp(227): error C2668: 'ArgBuilder::append': ambiguous call to overloaded function
1>Main.cpp(74): note: could be 'void ArgBuilder::append(int)'
1>Main.cpp(63): note: or       'void ArgBuilder::append(std::string_view)'
1>Main.cpp(55): note: or       'void ArgBuilder::append(std::string &&)'
1>Main.cpp(227): note: while trying to match the argument list '(const char [3])'

so another overload is needed to disambiguate

void
append( char const* name ) {
   _append( fmt::StringRef( name, strlen( name ) ) );
}

@vitaut
Copy link
Contributor

vitaut commented Jul 4, 2017

@tnovotny The new experimental version of the library located in the std branch has a public API for constructing an argument list:

auto args = fmt::make_args(42, "foo");
fmt::vformat("{}{}", args);

The only difference is that you should pass all arguments at once. It is described in the standards proposal http://fmtlib.net/Text%20Formatting.html.

@tnovotny
Copy link
Contributor Author

tnovotny commented Jul 5, 2017

That certainly helps, but would force me to use fmt::ArgList as my interface. Right now format.h is rather lengthy and includes a lot of stuff that is not relevant to building the argument list, so it would be considerate to move the stuff relevant to argument building into a separate include file so that not too many dependencies are dragged in and compile times are minimally affected.

Still, if the arguments are already in another container, it would be very beneficial to have a way to convert them. Therefore an interface where not all the arguments are passed variadically would be helpful, which is why I went for the builder approach. But something like this would also work.

auto args = fmt::make_args_n(2);
args[0] = ftm:make_arg(42);
args[1] = fmt::make_arg("foo");

@vitaut
Copy link
Contributor

vitaut commented Dec 6, 2017

As mentioned in #590 (comment), I moved the core API that deals with argument lists to a separate header, fmt/core.h, in the std branch. The new header is very small, has few dependencies and fast to compile. A PR for a builder-type API to construct argument lists is welcome!

@tnovotny
Copy link
Contributor Author

tnovotny commented Dec 6, 2017

@vitaut Thanx for the info. I'll have a look at it.

@tnovotny
Copy link
Contributor Author

tnovotny commented Dec 7, 2017

Just as a proof of concept I tried the following (only works for < 16 args) ..

template <typename Context, int NUM_ARGS>
class arg_store_n {
private:
  // Packed is a macro on MinGW so use IS_PACKED instead.
  static const bool IS_PACKED = NUM_ARGS < internal::MAX_PACKED_ARGS;

  using value_type = typename std::conditional<IS_PACKED, internal::value<Context>, fmt::basic_arg<Context>>::type;

  // If the arguments are not packed, add one more element to mark the end.
  using Array = std::array<value_type, NUM_ARGS + ( IS_PACKED ? 0 : 1 )>;
  Array data_;

public:
  uint64_t types_ =  IS_PACKED ? 0 : -static_cast<int64_t>(NUM_ARGS);

  arg_store_n() 
  : data_() {
  }

  template <typename T>
  void set_at( size_t i, T&& t ) {
    data_[i] = internal::make_arg<IS_PACKED, Context, T>( std::forward<T>( t ) );
    types_ +=internal::get_type<T>( ) << (4*i);
  }

  uint64_t types() const {
    return types_;
  }

  const value_type*
  data() const {
    return data_.data();
  }
};

which can be used if we add another constructor to basic_format_args

template <typename Context>
class basic_format_args {
    ...
    template <int N>
    basic_format_args( const arg_store_n<Context, N> &store )
     : types_( store.types() ) {
        set_data( store.data() );
    }
    ...
};

to enable something like a builder in this manner:

#include "fmt/format.cc"

int main() {
  fmt::arg_store_n<fmt::context, 2> args;
  args.set_at( 0, 42 );
  args.set_at( 1, "foo" );

  fmt::format_args fmt_args( args );

  auto foo = fmt::vformat( "{0} {1}", fmt_args );
}

Would that be a direction you would be willing to go?

@vitaut
Copy link
Contributor

vitaut commented Dec 9, 2017

The proposed API looks fine to me, but I'd decouple it from basic_format_args by parameterizing the constructor on the store type:

    template <typename Store>
    basic_format_args(Store &store)
    : types_(store.types()) {
      set_data(store.data());
    }

and placing arg_store_n in a separate header since most users won't need it.

@vitaut
Copy link
Contributor

vitaut commented Feb 12, 2018

The std branch has been merged into master, so now you can use fmt::make_args to create an argument list dynamically:

auto args = fmt::make_args('c', 42);
fmt::vprint("{} {}", args);

However, if you want to capture arguments and pass them to a formatting function later, you can just use std::tuple:

#include <tuple>
#include "fmt/format.h"

template <typename... Args>
void print(const char* format_str, const std::tuple<Args...>& t) {
  std::apply([=](const auto&... args) { fmt::print(format_str, args...); }, t);
}

int main() {
  auto args = std::make_tuple('c', 42);
  print("{} {}", args);
}

@vitaut vitaut closed this as completed Feb 12, 2018
@tnovotny
Copy link
Contributor Author

tnovotny commented Nov 18, 2018

@vitaut I had another look and the current state of fmtlib. I'm slightly confused, do you mean fmt::make_format_args? That seems to work great if I want to use the fmtlib style formatting, but what if I want to use printf style? The only thing that I managed to come up with

fmt::printf_args = fmt::make_format_args<fmt::printf_context<fmt::internal::buffer>::type>("foo", 42);

still requires an internal type, so a corresponding fmt::make_printf_args function seems to be missing (or maybe I justed missed it ...).

@vitaut
Copy link
Contributor

vitaut commented Nov 18, 2018

do you mean fmt::make_format_args?

Yes, the function was renamed per C++ standardization committee feedback.

As for the printf context, a PR to add a type alias is welcome.

@MattiasEppler
Copy link

Have a question to this issue. Its not realy clear for me how to use this.
I need to format my string with a std::vector. After I take a look arround I ended here.

But I still not understand how to format like this

void DoSomething(std::vector<int>& vect)
{
    fmt::format("Some {0} template {2} string {1}", vect);
}

@tnovotny
Copy link
Contributor Author

tnovotny commented Jan 28, 2019

Well, you'd need to do something like this as format doesn't accept an arbitrary container. (not sure if this compiles)

void DoSomething(std::vector<int>& vect)
{
   auto fargs = fmt::make_format_args(vect[0], vect[1], vect[2]);    
   fmt::format("Some {0} template {2} string {1}", fargs);
}

@MattiasEppler
Copy link

MattiasEppler commented Jan 28, 2019

Ist not my Intension to do this manualy.
The string I get from file, I do not kwow how many args in the string and the vect.

Better Example:

string DoSomeThing(const std::string& text, const std::vector<int>& vect)
{
    return  fmt::Format(text, vect); 
}

@tnovotny
Copy link
Contributor Author

tnovotny commented Jan 28, 2019

Ist not my Intension to do this manualy.

Well, AFAIK there is no other way ATM. See also here:
Manually construct ArgList and pass to fmt::print

The dynamic ArgBuilder that I proposed is not in the library. You can hack something like that yourself but it will be brittle, as it will have to rely on internals. It may well be that the code above no longer compiles.

@vitaut
Copy link
Contributor

vitaut commented Jan 29, 2019

See #819, but note that there is an open issue with that approach (#858).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants