script: Disallow silent bool -> CScript conversion #18621

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2004-scriptNoBool changing 1 files +1 −1
  1. MarcoFalke commented at 12:57 PM on April 13, 2020: member

    Makes nonsensical stuff like ScriptToAsmStr(false,false); a compile failure

  2. script: Disallow silent bool -> CScript conversion 88884ee8d8
  3. laanwj commented at 1:02 PM on April 13, 2020: member

    ACK 88884ee8d8dcd5303b20e54801b03f9631959c76

  4. DrahtBot added the label Consensus on Apr 13, 2020
  5. promag commented at 1:57 PM on April 13, 2020: member

    ACK 88884ee8d8dcd5303b20e54801b03f9631959c76.

  6. DrahtBot commented at 4:37 PM on April 13, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18612 (script: Remove undocumented and unused operator+ by MarcoFalke)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  7. DrahtBot cross-referenced this on Apr 13, 2020 from issue script: Remove undocumented and unused operator+ by MarcoFalke
  8. practicalswift commented at 5:11 PM on April 13, 2020: contributor

    ACK 88884ee8d8dcd5303b20e54801b03f9631959c76

    explicit is better than implicit.

    More generally I think we should err on the safe side and only allow implicit conversions via explicit opt-in (instead of disallowing via explicit opt-out) :)

  9. jb55 approved
  10. jb55 commented at 5:14 PM on April 13, 2020: contributor

    ACK 88884ee8d8dcd5303b20e54801b03f9631959c76

  11. ryanofsky approved
  12. ryanofsky commented at 5:26 PM on April 13, 2020: contributor

    Code review ACK 88884ee8d8dcd5303b20e54801b03f9631959c76

    I think it's not possible for just marking a constructor explicit to change which overload a compiler chooses like sipa was speculating about in https://github.com/bitcoin/bitcoin/pull/18612/files#r407269834 (assuming no sfinae overloads). But might be worth verifying this PR doesn't change compiler output.

  13. MarcoFalke commented at 5:53 PM on April 13, 2020: member

    But might be worth verifying this PR doesn't change compiler output.

    I get the same bin with gcc/clang on O2

  14. sipa commented at 6:27 PM on April 13, 2020: member

    @ryanofsky FWIW, that is not true in general.

    The following code

    #include <stdio.h>
    
    struct A { };
    struct B : public A { };
    struct C {
        C(const A&) {printf("A\n");}
        C(const B&) {printf("B\n");}
    };
    
    int main(void) {
        const B objb = B();
        const C objc = objb;
    
        return 0;
    }
    

    will print B. When the C::C(const B&) constructor is marked explicit, it prints A. So it seems that:

    • Explicit constructors only match in explicit contexts.
    • deleted functions/constructors match just as much as non-deleted ones.
    • Among all matching functions the most specific one is chosen.
    • If the most specific one is deleted, there is an error.
  15. instagibbs commented at 5:22 PM on April 14, 2020: member

    utACK 88884ee8d8dcd5303b20e54801b03f9631959c76

  16. ryanofsky commented at 6:14 PM on April 14, 2020: contributor

    @ryanofsky FWIW, that is not true in general.

    Thanks, makes sense! I was thinking implicit constructor conversions would be pretty low in precedence but it makes sense derived-to-base conversions are lower

  17. fanquake merged this on Apr 15, 2020
  18. fanquake closed this on Apr 15, 2020

  19. MarcoFalke deleted the branch on Apr 15, 2020
  20. sidhujag referenced this in commit 9376e7f339 on Apr 15, 2020
  21. ComputerCraftr referenced this in commit 0d51754852 on Jun 10, 2020
  22. Fabcien referenced this in commit 4f308eaa81 on Jan 18, 2021
  23. PastaPastaPasta referenced this in commit a2c2ca980b on Jun 27, 2021
  24. PastaPastaPasta referenced this in commit 0ab09c07c2 on Jun 28, 2021
  25. PastaPastaPasta referenced this in commit 05384cd2c2 on Jun 29, 2021
  26. PastaPastaPasta referenced this in commit b756c19fd0 on Jul 1, 2021
  27. PastaPastaPasta referenced this in commit 49e93b17d2 on Jul 1, 2021
  28. PastaPastaPasta referenced this in commit 71d4883643 on Jul 14, 2021
  29. PastaPastaPasta referenced this in commit 7b74287215 on Jul 14, 2021
  30. bitcoin locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:54 UTC