Skip to content

Conversation

@Warrows
Copy link

@Warrows Warrows commented Aug 6, 2019

Gets rid of compiler warnings such as "warning: catching polymorphic type 'class std::runtime_error' by value [-Wcatch-value=]"

@Warrows Warrows self-assigned this Aug 6, 2019
@CaveSpectre11
Copy link

CaveSpectre11 commented Aug 6, 2019

Some questions:

  • The format is consistent in these changes, but it's not consistent through the code base. Should the format be <datatype>& <identifier> or <datatype> &<identifier>?
  • without an identifier; some places in the code are by reference, some by value
    test\alert_tests\cpp Line 96: catch (std::exception) { }
    rpc\rawtransaction.cpp Line 620: } catch (const std::exception&) {
  • While we're at it; should we define the catches to be const?
  • Should this be expanded to include the test modules?
    test\benchmark_zerocoin.cpp
    test\libzerocoin_tests.cpp
    test\alert_tests.cpp

@Warrows Warrows force-pushed the 2019-08_exceptions-by-ref branch 2 times, most recently from 1954e3d to 3b61e81 Compare August 7, 2019 12:19
@Warrows
Copy link
Author

Warrows commented Aug 7, 2019

Addressing your questions:

@CaveSpectre11
Copy link

Good reference; I will check it out so I can be more authoritative in the future ;)

@Warrows Warrows force-pushed the 2019-08_exceptions-by-ref branch 2 times, most recently from 05e8d12 to 5780e0d Compare August 14, 2019 09:58
@Warrows Warrows added this to the 4.0.0 milestone Aug 14, 2019
random-zebra
random-zebra previously approved these changes Sep 16, 2019
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 5780e0d093568cd421547f12574bf2c7ada253a0

@random-zebra
Copy link

Needs rebase

Gets rid of compiler warnings such as "warning: catching polymorphic type 'class std::runtime_error' by value [-Wcatch-value=]"
@Warrows Warrows force-pushed the 2019-08_exceptions-by-ref branch from ef33500 to bd68647 Compare September 19, 2019 07:41
@Warrows
Copy link
Author

Warrows commented Sep 19, 2019

Rebased

@random-zebra
Copy link

Doesn't compile. There's some conflict leftover in masternode.cpp since #1001 has been merged.

@Warrows Warrows force-pushed the 2019-08_exceptions-by-ref branch from bd68647 to c826092 Compare September 19, 2019 09:16
@Warrows
Copy link
Author

Warrows commented Sep 19, 2019

Indeed. Should be good now.

@furszy furszy self-requested a review September 20, 2019 21:27
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c826092

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c826092

random-zebra added a commit that referenced this pull request Sep 21, 2019
c826092 [Refactor] Replace tabs with spaces (warrows)
b69f37e [Refactor] Add const qualifier to exception catching (warrows)
bf91dac [Compilation] Pass caught exceptions by reference (warrows)

Pull request description:

  Gets rid of compiler warnings such as "warning: catching polymorphic type 'class std::runtime_error' by value [-Wcatch-value=]"

ACKs for top commit:
  furszy:
    ACK [`c826092`](c826092)
  random-zebra:
    ACK c826092

Tree-SHA512: a2c02fdeab2d0e3cddde7a983456a680776592e9381be90968e0dc04bc86b7d5ea4d9aeb8259d39060fcd04583bb35259170fe5c28b8f0e96cf7aa94a3f21b2c
@random-zebra random-zebra merged commit c826092 into PIVX-Project:master Sep 21, 2019
@Warrows Warrows deleted the 2019-08_exceptions-by-ref branch September 22, 2019 20:24
Fuzzbawls added a commit that referenced this pull request Dec 21, 2020
… in CreateSig

2523f81 [Refactor] Pass caught logic_error by reference in CreateSig (random-zebra)

Pull request description:

  Exceptions/errors caught should be passed by reference.
  If the catch-block is not modifying the exception/error, the reference should be `const` (same as we did way back in #979)

  Fix one instance in `TransactionSignatureCreator::CreateSig` where we are passing a logic_error by copy, causing the `catching polymorphic type` warning reported in #2076

ACKs for top commit:
  furszy:
    utACK 2523f81
  Fuzzbawls:
    utACK 2523f81

Tree-SHA512: d13a0a2a9372610f3a5e67016053afbbac94d30f5d3dd8568352f7fd99356336058405f8526eaf1e6f751e2e48d7405425ad7a8a3a3f0ba8c6db620d12710ad2
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

Successfully merging this pull request may close these issues.

4 participants