Skip to content

Conversation

@Empact
Copy link
Contributor

@Empact Empact commented Aug 23, 2018

The call with this default argument is redundant with prevector(size_type) on line 251.

The call with this default argument is redundant with prevector(size_type).
@Empact Empact changed the title Remove default argument to prevector constructor to remove ambiguity Remove ambiguity in construction of prevector Aug 23, 2018
@Empact
Copy link
Contributor Author

Empact commented Aug 23, 2018

Note the effective implementation of resize(n) in this case is:

        change_capacity(n);
        _size += n;
        fill(item_ptr(0), n);

Copy link
Contributor

@AkioNak AkioNak left a comment

Choose a reason for hiding this comment

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

utACK

@DrahtBot
Copy link
Contributor

Note to reviewers: This pull request conflicts with the following ones:

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.

}

explicit prevector(size_type n, const T& val = T()) : _size(0) {
explicit prevector(size_type n, const T& val) : _size(0) {
Copy link
Member

Choose a reason for hiding this comment

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

The explicit keyword is also redundant for constructions with >1 argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Neat!

It's not really relevant here, but I'll expand on @Empact's comment because it's something I didn't know, maybe others will benefit as well.

Post c++11:

    using ScriptType = prevector<28, unsigned char>;
    ScriptType vec{1, 255};   // always works
    vec = ScriptType{1, 255}; // always works
    vec = {1, 255};           // only works if prevector's ctor is not explicit.

caveat: the 3rd example above does not actually work because of prevector's templated iterator ctor, but that's unrelated.

@sipa
Copy link
Member

sipa commented Aug 23, 2018

utACK

@theuni
Copy link
Member

theuni commented Aug 23, 2018

utACK 497e90c.

For reference, with c++11, std::vector's matching constructor also removed its default argument and dropped the explicit qualifier.

@laanwj laanwj merged commit 497e90c into bitcoin:master Aug 27, 2018
laanwj added a commit that referenced this pull request Aug 27, 2018
497e90c Remove default argument to prevector constructor to remove ambiguity (Ben Woosley)

Pull request description:

  The call with this default argument is redundant with `prevector(size_type)` on line 251.

Tree-SHA512: 4d22e6f4cd56e4b700596d7f5afc945ec6684636a94690fa16a1bbb34e4f53b6340f53a6c314fea213359426474125228ba7193388789f8a13308506358e92db
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
497e90c Remove default argument to prevector constructor to remove ambiguity (Ben Woosley)

Pull request description:

  The call with this default argument is redundant with `prevector(size_type)` on line 251.

Tree-SHA512: 4d22e6f4cd56e4b700596d7f5afc945ec6684636a94690fa16a1bbb34e4f53b6340f53a6c314fea213359426474125228ba7193388789f8a13308506358e92db
@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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants