Skip to content

tinyformat: force USE_VARIADIC_TEMPLATES #8000

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 5, 2016
Merged

tinyformat: force USE_VARIADIC_TEMPLATES #8000

merged 2 commits into from
May 5, 2016

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented May 4, 2016

Now that we started using c++11, force use of variadic templates.

The autodetection may be wonky on some compilers, see discussion here and is unnecessary for us anyhow.

@laanwj
tinyformat: force USE_VARIADIC_TEMPLATES
9eaa0af 
Now that we started using c++11, force use of variadic templates.

The autodetection may be wonky on some compilers, see discussion
[here](bitcoin#7982 (comment))
and is unnecessary for us anyhow.
@theuni
Copy link
Member

theuni commented May 4, 2016

ut ACK

@laanwj
Copy link
Member Author

laanwj commented May 4, 2016

Looks like there is another tinyformat-related c++1 TODO here: https://github.com/bitcoin/bitcoin/blob/master/src/util.h#L83

@paveljanik
Copy link
Contributor

utACK 9eaa0af

@fanquake
Copy link
Member

fanquake commented May 4, 2016

utACK 9eaa0af

@jonasschnelli
Copy link
Contributor

utACK 9eaa0af

@laanwj
Copy link
Member Author

laanwj commented May 4, 2016

Added a second commit that makes LogPrint and error into variadic templates.

@theuni
Copy link
Member

theuni commented May 4, 2016

may as well use perfect forwarding here, since all args are universal references:

template<typename T1, typename... Args>
bool error(const char* fmt, T1&& v1, Args&&... args)
{
    LogPrintStr("ERROR: " + tfm::format(fmt, std::forward<T1>(v1), std::forward<Args>(args)...) + "\n");
...

@laanwj
Copy link
Member Author

laanwj commented May 4, 2016

@cfields I use the same convention as the functions in tinyformat.h itself now:

template<typename T1, typename... Args>
std::string format(const char* fmt, const T1& v1, const Args&... args)
{
    std::ostringstream oss;
    format(oss, fmt, v1, args...);
    return oss.str();
}

I'm not sure perfect forwarding would win anything unless it's done at all levels.

@theuni
Copy link
Member

theuni commented May 5, 2016

blah, I assumed if it used variadics it also forwarded nicely. Indeed, no point in changing.

ut ACK 08d7b56

@laanwj
Copy link
Member Author

laanwj commented May 5, 2016

blah, I assumed if it used variadics it also forwarded nicely. Indeed, no point in changing.

Well I agree with changing it, but yea should probably do that at once throughout the entire thing, in a separate pull.

@laanwj laanwj merged commit 08d7b56 into bitcoin:master May 5, 2016
laanwj added a commit that referenced this pull request May 5, 2016
@laanwj
Merge #8000: tinyformat: force USE_VARIADIC_TEMPLATES
42a6753 
08d7b56 util: switch LogPrint and error to variadic templates (Wladimir J. van der Laan)
9eaa0af tinyformat: force USE_VARIADIC_TEMPLATES (Wladimir J. van der Laan)
laanwj added a commit to laanwj/bitcoin that referenced this pull request Jun 27, 2016
@laanwj
util: Update tinyformat
695041e 
Updates `tinyformat.h` to commit
c42f/tinyformat@3a33bbf upstream.

Makes sure that our local changes are kept:

- bitcoin#3767 1b8fd35 Make tinyformat errors raise an exception instead of assert()ing
- bitcoin#4735 9b6d4c5 Move strprintf define to tinyformat.h
- bitcoin#4748 6e5fd00 include stdexcept (for std::exception)
- bitcoin#8000 9eaa0af force USE_VARIADIC_TEMPLATES
- Add `std::string format(const std::string &fmt...` added this
  at the time, as we want to be able to do `strprintf(_(...), ...)`

Inspired by bitcoin#8264.
@laanwj laanwj mentioned this pull request Jun 27, 2016
Merged
@daira daira mentioned this pull request Oct 25, 2016
Merged
zkbot pushed a commit to zcash/zcash that referenced this pull request Oct 25, 2016
Auto merge of #1640 - daira:upstream-update-tinyformat, r=daira
6bad499 
util: Update tinyformat

Updates `tinyformat.h` to commit
c42f/tinyformat@3a33bbf upstream.

Makes sure that our local changes are kept:

- bitcoin/bitcoin#3767 1b8fd35 Make tinyformat errors raise an exception instead of assert()ing
- bitcoin/bitcoin#4735 9b6d4c5 Move strprintf define to tinyformat.h
- bitcoin/bitcoin#4748 6e5fd00 include stdexcept (for std::exception)
- bitcoin/bitcoin#8000 9eaa0af force USE_VARIADIC_TEMPLATES
- Add `std::string format(const std::string &fmt...` added this
  at the time, as we want to be able to do `strprintf(_(...), ...)`

Inspired by bitcoin/bitcoin#8264.

For Zcash: ref #1349
@dagurval dagurval mentioned this pull request Apr 23, 2017
Merged
protonn pushed a commit to argentumproject/argentum that referenced this pull request May 7, 2017
@laanwj @protonn
util: Update tinyformat
2e09c3d 
Updates `tinyformat.h` to commit
c42f/tinyformat@3a33bbf upstream.

Makes sure that our local changes are kept:

- bitcoin#3767 1b8fd35 Make tinyformat errors raise an exception instead of assert()ing
- bitcoin#4735 9b6d4c5 Move strprintf define to tinyformat.h
- bitcoin#4748 6e5fd00 include stdexcept (for std::exception)
- bitcoin#8000 9eaa0af force USE_VARIADIC_TEMPLATES
- Add `std::string format(const std::string &fmt...` added this
  at the time, as we want to be able to do `strprintf(_(...), ...)`

Inspired by bitcoin#8264.
codablock pushed a commit to codablock/dash that referenced this pull request Dec 20, 2017
@laanwj @codablock
Merge bitcoin#8000: tinyformat: force USE_VARIADIC_TEMPLATES
d55b78e 
08d7b56 util: switch LogPrint and error to variadic templates (Wladimir J. van der Laan)
9eaa0af tinyformat: force USE_VARIADIC_TEMPLATES (Wladimir J. van der Laan)
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 3, 2019
@laanwj
util: Update tinyformat
0320890 
Updates `tinyformat.h` to commit
c42f/tinyformat@3a33bbf upstream.

Makes sure that our local changes are kept:

- bitcoin#3767 1b8fd35 Make tinyformat errors raise an exception instead of assert()ing
- bitcoin#4735 9b6d4c5 Move strprintf define to tinyformat.h
- bitcoin#4748 6e5fd00 include stdexcept (for std::exception)
- bitcoin#8000 9eaa0af force USE_VARIADIC_TEMPLATES
- Add `std::string format(const std::string &fmt...` added this
  at the time, as we want to be able to do `strprintf(_(...), ...)`

Inspired by bitcoin#8264.
@random-zebra random-zebra mentioned this pull request Mar 23, 2020
Merged
furszy added a commit to PIVX-Project/PIVX that referenced this pull request Mar 27, 2020
@furszy
Merge #1449: [Util] tinyformat / LogPrint backports
9ef9f5d 
9fb0a43 util: Throw tinyformat::format_error on formatting error (random-zebra)
67eb699 util: Properly handle errors during log message formatting (random-zebra)
66ec97b Do not evaluate hidden LogPrint arguments (random-zebra)
2713458 util: Remove zero-argument versions of LogPrint and error (random-zebra)
500dfee util: Update tinyformat (random-zebra)
6837887 util: switch LogPrint and error to variadic templates (random-zebra)
0fa578f tinyformat: force USE_VARIADIC_TEMPLATES (random-zebra)

Pull request description:

  this backports the following pull requests from upstream bitcoin:
  - bitcoin#8000 (0fa578f, 6837887)
  > Now that we started using c++11, force use of variadic templates.
  The autodetection may be wonky on some compilers, see discussion here and is unnecessary for us anyhow.

  - bitcoin#8274 (500dfee, 2713458)
  > Updates tinyformat.h to commit c42f/tinyformat@3a33bbf upstream.

  - bitcoin#9417 (66ec97b)
  > There are a few cases where hashes are computed inside LogPrint arguments - where they usually go unused. As LogPrint statements should never have side effects besides printing something, we can avoid the evaluation in this case.
  Advantage: perhaps a small performance improvement; I haven't benchmarked.
  Disadvantage: if we would have statements with side-effects, this could make this a bit harder to debug.

  - bitcoin#9963 (67eb699, 9fb0a43)
  > Instead of having an exception propagate into the program (which at worst causes a crash) when an error happens while formatting a log message, just print a message to the log. This message clearly indicates what log message was formatted wrongly, and what error happened during formatting it.

ACKs for top commit:
  Fuzzbawls:
    ACK 9fb0a43
  furszy:
    cool, utACK 9fb0a43 .

Tree-SHA512: 29a902bb712612ca093a8fe863e9eff1c17d40955bbc37a5ceb6f057068ac6150dfcffd6836a6b0bd6295aa9bffd0925eb69e50f82ccaa7d84215efc274a59a4
akshaynexus pushed a commit to ZENZO-Ecosystem/ZENZO-Core that referenced this pull request Mar 30, 2020
@furszy @akshaynexus
Merge PIVX-Project#1449: [Util] tinyformat / LogPrint backports
5c753ce 
9fb0a43 util: Throw tinyformat::format_error on formatting error (random-zebra)
67eb699 util: Properly handle errors during log message formatting (random-zebra)
66ec97b Do not evaluate hidden LogPrint arguments (random-zebra)
2713458 util: Remove zero-argument versions of LogPrint and error (random-zebra)
500dfee util: Update tinyformat (random-zebra)
6837887 util: switch LogPrint and error to variadic templates (random-zebra)
0fa578f tinyformat: force USE_VARIADIC_TEMPLATES (random-zebra)

Pull request description:

  this backports the following pull requests from upstream bitcoin:
  - bitcoin#8000 (0fa578f, 6837887)
  > Now that we started using c++11, force use of variadic templates.
  The autodetection may be wonky on some compilers, see discussion here and is unnecessary for us anyhow.

  - bitcoin#8274 (500dfee, 2713458)
  > Updates tinyformat.h to commit c42f/tinyformat@3a33bbf upstream.

  - bitcoin#9417 (66ec97b)
  > There are a few cases where hashes are computed inside LogPrint arguments - where they usually go unused. As LogPrint statements should never have side effects besides printing something, we can avoid the evaluation in this case.
  Advantage: perhaps a small performance improvement; I haven't benchmarked.
  Disadvantage: if we would have statements with side-effects, this could make this a bit harder to debug.

  - bitcoin#9963 (67eb699, 9fb0a43)
  > Instead of having an exception propagate into the program (which at worst causes a crash) when an error happens while formatting a log message, just print a message to the log. This message clearly indicates what log message was formatted wrongly, and what error happened during formatting it.

ACKs for top commit:
  Fuzzbawls:
    ACK 9fb0a43
  furszy:
    cool, utACK 9fb0a43 .

Tree-SHA512: 29a902bb712612ca093a8fe863e9eff1c17d40955bbc37a5ceb6f057068ac6150dfcffd6836a6b0bd6295aa9bffd0925eb69e50f82ccaa7d84215efc274a59a4
nathanielcwm added a commit to nathanielcwm/Gridcoin-Research that referenced this pull request May 7, 2021
@nathanielcwm
Port bitcoin/bitcoin#8000
0e1fe3b 
Rebase tinyformat.
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@laanwj @theuni @paveljanik @fanquake @jonasschnelli