microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
9.88k stars 1.45k forks source link

`<execution>`: scan series function passes output values to the binary (reduce) operation with par policy #4701

Open Andor233 opened 4 weeks ago

Andor233 commented 4 weeks ago

Fix #3783

<execution>: transform_inclusive_scan transform_exclusive_scan exclusive_scan inclusive_scan passes output values to the binary (reduce) operation with par policy

Add _STD to defend ADL, towards #140

StephanTLavavej commented 4 weeks ago

Thanks!

frederick-vs-ja commented 4 weeks ago

I think I've roughly found that _First == _Last is problematic (some occurences are pre-existing).

Given Incomplete is an incomplete class type and Holder is template<T> struct Holder { T t; };, vector<Holder<Incomplete>*> is a weird type. It doesn't meet the requirements of container, because while its element type is equality comparable, v1 == v2 will result in ADL into an incomplete class which will in turn cause a hard error. However, the standard doesn't ever say instantiation of vector<Holder<Incomplete>*> is UB or IFNDR. So I think it should be well-defined with limited usages.

In MSVC STL, its iterator type (_Vector_iterator<_Vector_val<_Simple_types<Holder<Incomplete>*>>>) is not even an iterator, because equality comparison between such two "iterators" also results in a hard error due to ADL. I think we should pass v.data() (which obtains raw pointers) instead of std::begin(v) or v.begin() to internal functions to make the iterator comparison well-formed.

Moreover, some internal function calls in <vector>, e.g. _Construct_in_place, should also be _STD-qualified.

Andor233 commented 4 weeks ago

Thanks!

  • Please give your PR a descriptive title, so we can see at a glance what it's doing - bug numbers are hard to memorize, and they aren't linked/previewed when they appear in PR titles.
  • GitHub's close/fix/resolve syntax works only in PR descriptions, not PR titles, so you should move "Fix #nnnn" to the description.
  • As there are test failures including GH_001596_adl_proof_algorithms (possibly more directories, I didn't check all of the logs), please ensure that the test directory is locally passing before pushing changes - i.e. please avoid relying on the CI system to iterate on changes. Locally running tests will be faster for you, and avoid unnecessarily consuming shared resources.

Thank your adivce! I'm bad at naming.

I have a problem that If I want to make the commit more clearly, so I know this commit will not pass the CI and next one will, but I still want to commit it separately, how can I avoid this situation?

StephanTLavavej commented 4 weeks ago

Thank your adivce! I'm bad at naming.

You're welcome! :smile_cat:

I have a problem that If I want to make the commit more clearly, so I know this commit will not pass the CI and next one will, but I still want to commit it separately, how can I avoid this situation?

You can create as many commits as you like locally (which is a great way to logically structure your changes), just avoid pushing them until you're confident that the checks will pass. If you push a dozen commits, the CI will run only for the last one, not all of the intermediate ones.