Extend the documentation of PartialOrd #53386

gnzlbg · 2018-08-15T11:36:10Z

Closes #50230 .

cc @ExpHP @RalfJung @LukasKalbertodt @fbstj

@ExpHP

Closes rust-lang#50230 . cc @ExpHP @RalfJung @LukasKalbertodt @fbstj

rust-highfive · 2018-08-15T11:36:20Z

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

gnzlbg · 2018-08-15T11:38:28Z

I tried to merge all the comments of https://github.com/LukasKalbertodt/partial-cmp-docs/pull/1/files , but there is still one unresolved issue open by @RalfJung :

And why state irreflexivity and others nit also for >? Also, antisymmetry as given is a property of both < and > in combination, which I don't think is the usual formulation. (Or rather, the usual definition of for > to be the dual of < to asymmetry and antisymmetry as given here are actually the same statement.)

Also, i'm not a native speaker, and this is a tricky topic, so it would be cool if someone from the docs team would take a look at this and further process it for easy consumption by users (cc @steveklabnik )

gnzlbg · 2018-08-15T11:39:10Z

src/libcore/cmp.rs

@@ -8,17 +8,16 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.

-//! Functionality for ordering and comparison.
+//! Functionality for ordering and relational operations.


Should this be just relational operations ? This is what comparisons are.

I wouldn't mind keeping the old version ("ordering and comparisons"). I don't think we gain a lot from being precise here.

gnzlbg · 2018-08-15T11:42:39Z

src/libcore/cmp.rs

+//!   (more on this below).
+//!
+//! The distinction between these traits is important: it tell us what it _means_ for a 
+//! sequence of values to be ordered.


This is the pragmatic way I choose to put it, might not be the best one.

@LukasKalbertodt I basically decided to put the "intro docs" to PartialOrd at the top level of the module. Otherwise the docs of PatialOrd and other traits end up restating the same things over and over again and is hard to keep them in sync.

Typo: "it tellS us"

I think putting the text describing the differences in the mod documentation is a good idea.

The second part of this sentence does not talk about a distinction, so I am confused what this is even telling us.

Actually I am not even sure which distinction you mean: PartialEq vs Eq, or Eq vs Ord...

rust-highfive · 2018-08-15T11:43:44Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:05:11]    Compiling syn v0.14.4
[00:05:19]    Compiling serde_json v1.0.24
[00:05:27]    Compiling serde_derive v1.0.70
[00:05:49]    Compiling tidy v0.1.0 (file:///checkout/src/tools/tidy)
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:16: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:40: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:43: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:46: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:46: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:47: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:48: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:49: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:52: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:52: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:53: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:54: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:54: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:57: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:57: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:60: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:61: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:61: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:62: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:64: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:64: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:65: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:65: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:66: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:66: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:69: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:70: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:71: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:72: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:73: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:74: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:77: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:77: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:78: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:79: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:80: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:80: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:81: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:82: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:82: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:83: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:84: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:84: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:85: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:86: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:87: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:87: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:88: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:576: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:576: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:579: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:579: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:580: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:580: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:581: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:581: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:584: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:584: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:585: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:585: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:592: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:600: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:600: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:601: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:601: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:604: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:605: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:606: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:608: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:610: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:611: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:611: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:618: line longer than 100 chars
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:618: trailing whitespace
[00:05:56] tidy error: /checkout/src/libcore/cmp.rs:621: line longer than 100 chars
[00:05:58] Build completed unsuccessfully in 0:00:54
[00:05:58] Build completed unsuccessfully in 0:00:54
[00:05:58] make: *** [tidy] Error 1
[00:05:58] Makefile:79: recipe for target 'tidy' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:01f59780
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
The command "date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
" exited with 0.
travis_fold:start:after_failure.1
travis_time:start:10353c64
$ sudo tail -n 500 /var/log/syslog
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] ACPI: FACP 0x00000000BFFFCF60 0000F4 (v02 Google GOOGFACP 00000001 GOOG 00000001)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] ACPI: DSDT 0x00000000BFFF3470 0017B2 (v01 Google GOOGDSDT 00000001 GOOG 00000001)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] ACPI: FACS 0x00000000BFFFCF00 000040
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] ACPI: FACS 0x00000000BFFFCF00 000040
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] ACPI: SSDT 0x00000000BFFF65F0 006d9f134a9d kernel: [    0.000000] Booting paravirtualized kernel on KVM
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] clocksource: refined-jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 7645519600211568 ns
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] setup_percpu: NR_CPUS:512 nr_cpumask_bits:512 nr_cpu_ids:4 nr_node_ids:1
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] PERCPU: Embedded 34 pages/cpu @ffff8803ffc00000 s98392 r8192 d32680 u524288
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] pcpu-alloc: s98392 r8192 d32680 u524288 alloc=1*2097152
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] pcpu-alloc: [0] 0 1 2 3 
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] Built 1 zonelists in Node order, mobility grouping on.  Total pages: 3870588
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] Policy zone: Normal
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-4.4.0-101-generic root=UUID=752b4ef1-0512-4cae-b541-f03ffd29be1b ro cgroup_enable=memory swapaccount=1 apparmor=0 console=ttyS0 console=ttyS0
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] PID hash table entries: 4096 (order: 3, 32768 bytes)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.000000] Calgary: detecting Calgary via: reg 0x14: [mem 0xfebfe000-0xfebfe07f]
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.730250] pci 0000:00:04.0: [1af4:1000] type 00 class 0x020000
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.736017] pci 0000:00:04.0: reg 0x10: [io  0xc040-0xc07f]
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.740269] pci 0000:00:04.0: reg 0x14: [mem 0xfebff000-0xfebff0ff]
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.753350] ACPI: PCI Interrupt Link [LNKA] (IRQs 5 *10 11)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.755479] ACPI: PCI Interrupt Link [LNKB] (IRQs 5 *10 11)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.757646] ACPI: PCI Interrupt Link [LNKC] (IRQs 5 10 *11)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.760508] ACPI: PCI Interrupt Link [LNKD] (IRQs 5 10 *11)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.762644] ACPI: PCI Interrupt Link [LNKS] (IRQs *9)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.782952] ACPI: Enabled 16 GPEs in block 00 to 0F
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.783969] vgaarb: loaded
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.784570] SCSI subsystem initialized
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.785183] libata version 3.00 loaded.
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148dob-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.793221] NetLabel:  unlabeled traffic allowed by default
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.794113] amd_nb: Cannot enumerate AMD northbridges
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.794983] clocksource: Switched to clocksource kvm-clock
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.802340] pnp: PnP ACPI init
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.802864] pnp 00:00: Plug and Play ACPI device, IDs PNP0b00 (active)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.802931] pnp 00:01: Plug and Play ACPI device, IDs PNP0303 (active)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.802975] pnp 00:02: Plug and Play ACPI device, IDs PNP0f13 (active)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.803052] pnp 00:03: Plug and Play ACPI device, IDs PNP0501 (active)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.803099] pnp 00:04: Plug and Play ACPI device, IDs PNP0501 (active)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.803141] pnp 00:05: Plug and Play ACPI device, IDs PNP0501 (active)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.803189] pnp 00:06: Plug and Play ACPI device, IDs PNP0501 (active)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    0.803345] pnp: PnP ACPI: found 7 devices
Aug 15 11:36:44 travie79-4bb5-b4a7-148d9f134a9d kernel: [    3.053226] ima: No TPM chip found, activating TPM-bypass!
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.054928] evm: HMAC attrs: 0x1
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.056630]   Magic number: 14:884:630
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.057960] rtc_cmos 00:00: setting system clock to 2018-08-15 11:36:36 UTC (1534332996)
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.060575] BIOS EDD facility v0.16 2004-Jun-25, 0 devices found
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.062190] EDD information not available.
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.063283] PM: Hibernation image not present or could not be loaded.
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.064648] Freeing unused kernel memory: 1496K
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.065314] Write protecting the kernel read-only data: 14336k
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.066909] Freeing unused kernel memory: 1956K
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.067898] Freeing unused kernel memory: 92K
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.082212] systemd-udevd[118]: starting version 204
Aug 15 11:36:44 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [    3.135945] scsi host0: Virtio SCSI HBA
Aug 15b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user account for me.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Created user account me.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user account for henrik.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-clock-skew: INFO Synced system time with hardware clock.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Created user account henrik.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user account for emma.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Created user account emma.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user account for igor.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Created user account igor.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user account for konstantinhaase.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Created user account konstantinhaase.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user account for aj.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Created user account aj.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user 79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user account for carmen.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [   12.743959] Initializing XFRM netlink socket
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [   12.755111] Netfilter messages via NETLINK v0.30.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [   12.758655] ctnetlink v0.93: registering with nfnetlink.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Created user account carmen.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Creating a new user account for maria.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Created user account maria.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d google-accounts: INFO Removing user packer.
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [   12.875275] floppy0: no floppy controllers found
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d pollinate: system was previously seeded at [2017-12-05 19:31:29.715998981 +0000]
Aug 15 11:36:45 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d pollinate: To re-seed this system again, use the -r|--reseed option
Aug 15 11:36:46 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d cron[1708]: (CRON) INFO (pidfile fd = 3)
Aug 15 11:36:46 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d pollinate: system was previously seeded at [2017-12-05 19:31:29.715998981 +0000]
Aug 15 11:36:46 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9a9d ntpd[1844]: Listen normally on 7 docker0 fd9a:8454:6789:13f7::1 UDP 123
Aug 15 11:39:35 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d ntpd[1844]: peers refreshed
Aug 15 11:39:35 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d ntpd[1844]: new interface(s) found: waking up resolver
Aug 15 11:39:47 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [  194.515809] docker0: port 1(veth1f20871) entered forwarding state
Aug 15 11:43:40 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [  427.323460] vethe0be177: renamed from eth0
Aug 15 11:43:40 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [  427.354644] docker0: port 1(veth1f20871) entered disabled state
Aug 15 11:43:40 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [  427.385625] docker0: port 1(veth1f20871) entered disabled state
Aug 15 11:43:40 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [  427.387543] device veth1f20871 left promiscuous mode
Aug 15 11:43:40 travis-job-047a683b-7e79-4bb5-b4a7-148d9f134a9d kernel: [  427.387547] docker0: port 1(veth1f20871) entered disabled state
travis_fold:end:after_failure.1
travis_fold:start:after_failure.2
travis_time:start:0e54e074

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

gnzlbg · 2018-08-15T12:51:46Z

r? @steveklabnik

pitdicker · 2018-08-15T16:14:09Z

src/libcore/cmp.rs

+//!
+//! Two commonly encountered examples are integers and floating-point numbers. For example, every [`i32`] 
+//! value is comparable to every other. If you can tell that two `i32` values are distinc, then these two
+//! values never compare equal, and if you sorte a sequence of `i32` values like `[1, 1, 0, -5]` using `<=`,


Just a small nit while reading the new docs: sort

Typo: "distinc" => "distinct"

pitdicker · 2018-08-15T16:14:36Z

src/libcore/cmp.rs

+//! order, and denote this by implementing `Ord` for `i32`.
+//!
+//! However, none of this holds for IEEE-754 floating-point numbers like `f32`. There is a special `f32` value 
+//! called "Not a Number" (`NaN`) for which any relational operation alwys return false, that is: 


pietroalbini · 2018-08-27T15:31:18Z

Ping from triage @steveklabnik! This PR needs your review.

LukasKalbertodt

I finally managed to review this. Sorry for the delay!

And thanks a lot for pushing this topic.

I left a few comments. Many are just typos or minor things. However, as you can see below, I'm not quite happy with changes in the module docs -- but maybe it's just me.

Also note that all "I would..." comments should be taken with a grain of salt since I'm not a native speaker.

Oh and maybe you could summarize the prior discussion and what this PR mainly wants to achieve so that @steveklabnik can start reviewing this more quickly.

LukasKalbertodt · 2018-08-27T15:52:28Z

src/libcore/cmp.rs

@@ -8,17 +8,16 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.

-//! Functionality for ordering and comparison.
+//! Functionality for ordering and relational operations.


I wouldn't mind keeping the old version ("ordering and comparisons"). I don't think we gain a lot from being precise here.

LukasKalbertodt · 2018-08-27T15:53:40Z

src/libcore/cmp.rs

 //!
-//! # Examples
+//! state that values of a type can be compared for equality, or ordered, but that
+//! not all values of the type can be compared to all other values (more on this below).


I guess this can be removed as it appears below already?

LukasKalbertodt · 2018-08-27T15:57:06Z

src/libcore/cmp.rs

+//! This module also provides the following two traits:
+//!
+//! * [`Eq`]: which requires `PartialEq`, states that all values of a type can be
+//!   compared to all others,


Alternative:

Eq: marker trait which states that all values of a type can be compared to all other values of the same type (requires PartialEq<Self>)

LukasKalbertodt · 2018-08-27T15:59:18Z

src/libcore/cmp.rs

+//!   (more on this below).
+//!
+//! The distinction between these traits is important: it tell us what it _means_ for a 
+//! sequence of values to be ordered.


Typo: "it tellS us"

I think putting the text describing the differences in the mod documentation is a good idea.

LukasKalbertodt · 2018-08-27T15:59:48Z

src/libcore/cmp.rs

+//!
+//! Two commonly encountered examples are integers and floating-point numbers. For example, every [`i32`] 
+//! value is comparable to every other. If you can tell that two `i32` values are distinc, then these two
+//! values never compare equal, and if you sorte a sequence of `i32` values like `[1, 1, 0, -5]` using `<=`,


Typo: "distinc" => "distinct"

LukasKalbertodt · 2018-08-27T16:15:55Z

src/libcore/cmp.rs

+//! the types they accept with the [`Eq`] and [`Ord`] traits, while those for which a partial order or partial equality
+//! is enough can use the `PartialEq` and `PartialOrd` traits.
+//!
+//! This leads us to the final question: which traits should a type implement ? There is no easy answer: a type should only 


Nit: space before ?. Also: the lines are pretty long here.

LukasKalbertodt · 2018-08-27T16:17:57Z

src/libcore/cmp.rs

+/// Trait for values that can be partially ordered.
+///
+/// This traits defines the operators `<`, `>`, `<=`, and `>=` which compare two values and return `bool`, 
+/// where `<` and `>` form [_strict_ partial ordering relations][wiki_porder].


I would split this sentence. "[...] return bool. The operators < and > form [...]"

I'd move the remark about strict ordering relations far down to all the corollaries.

LukasKalbertodt · 2018-08-27T16:18:30Z

src/libcore/cmp.rs

+/// This traits defines the operators `<`, `>`, `<=`, and `>=` which compare two values and return `bool`, 
+/// where `<` and `>` form [_strict_ partial ordering relations][wiki_porder].
+///
+/// The `PartialOrd` trait  has fewer requirements of the ordering relation than Ord. In particular, `partial_cmp` 


Nits: two spaces after trait and Ord not in monofont

Also "requirements on the ordering". I think.

LukasKalbertodt · 2018-08-27T16:22:27Z

src/libcore/cmp.rs

+/// From these rules it follows that `PartialOrd` must be implemented symmetrically and transitively.
+/// That is, for two distinct types `T` and `U`, `T: PartialOrd<U>` requires the following trait 
+/// implementations to exist and satisfy these rules: `T: PartialOrd<T>`, `U: PartialOrd<T>`, 
+/// and `U: PartialOrd<U>`. If for a third distinct type `V` `T: PartialOrd<V>` is provided, then


Mhhh I don't quite get why T: PartialOrd<T> and U: PartialOrd<U> are required from the rules above. I am probably just missing something obvious...

This is required by transitivity (if a < b and b < c then a < c).

Suppose that a: T, b: U, and c: T., and that a < b and b < c are both true. For a < c to also be true it would need to be well-typed, and that requires T: PartialOrd<T>.

Ah right, that makes sense. Thanks :)

LukasKalbertodt · 2018-08-27T16:24:49Z

src/libcore/cmp.rs

+/// Stronger ordering relations can be expressed by using the `Eq` and `Ord` traits, where the `PartialOrd` 
+/// methods provide:
+///
+/// * a [_non-strict_ partial ordering relation](https://en.wikipedia.org/wiki/Partially_ordered_set#Strict_and_non-strict_partial_orders)


Did you mean to use [wiki_porder] here? Since you added the link at the bottom...

gnzlbg · 2018-08-29T18:10:06Z

cc @RalfJung could you comment on these changes? I will then go and update the PR with both your feedback, if you agree with @LukasKalbertodt feedback a 👍 on their comments would be great!

RalfJung · 2018-08-30T16:16:07Z

src/libcore/cmp.rs

 //!
-//! # Examples
+//! state that values of a type can be compared for equality, or ordered, but that


I find this hard to parse, where does the sentence even begin and what is the subject matching this verb?

RalfJung · 2018-08-30T16:17:06Z

src/libcore/cmp.rs

+//! * [`Eq`]: which requires `PartialEq`, states that all values of a type can be
+//!   compared to all others,
+//! * [`Ord`]: which requires `PartialOrd` and `Eq`, states that the ordering
+//!   relations expressed by the comparison operators form all total ordering relations 


What does it mean to form "all total ordering relations"?

Or do you mean, "all the comparison operators form total ordering relations"?

RalfJung · 2018-08-30T16:18:57Z

src/libcore/cmp.rs

+//! `a == NaN` is always `false`, even when `a` is `NaN`. For this reason `f32` only implements `PartialOrd`, and it 
+//! does not implement neither `Eq` nor `Ord`. What does this mean in practice? It means two things:
+//!
+//! * there are sequences of `f32`s that we can order using `<=` and will always produce the same unique 


RalfJung · 2018-08-30T16:19:02Z

src/libcore/cmp.rs

+//! * there are sequences of `f32`s that we can order using `<=` and will always produce the same unique 
+//!   result: sorting `[3.0, 5.0, 1.0]` into `[1.0, 3.0, 5.0]`.
+//!
+//! * there are also sequences of `f32`s that when ordered using `<=` will produce a different result depending


RalfJung · 2018-08-30T16:22:02Z

src/libcore/cmp.rs

+//!
+//! Another example is the "subset relation" on sets in which, for sets `a` and `b`, `a < b` is true if and only 
+//! if `a` is a proper subset of `b` (e.g. `{4, 9} < {1, 4, 5, 9}` is true). The problem is that when neither 
+//! `a` is a subset of `b` nor `b` is a subset of `a` nor `a = b` (e.g. `{1, 3}` and `{1, 4}`), we cannot sensible 


RalfJung · 2018-08-30T16:23:25Z

src/libcore/cmp.rs

+//! `a` is a subset of `b` nor `b` is a subset of `a` nor `a = b` (e.g. `{1, 3}` and `{1, 4}`), we cannot sensible 
+//! compare `a` and `b`, so that `a < b`, `a == b`, and `a > b` all return false.
+//!
+//! The distinction between a _total_ order and a _partial_ order is important, and impacts what it means for


This refers to the distinction made above, right? It should be "This distinction" then.

Generally, I feel this needs more structure. All the text explaining partial vs total order should be in some kind of appropriately titeled subsection.

RalfJung · 2018-08-30T16:25:03Z

src/libcore/cmp.rs

+//! total order for floating point numbers that is more expensive to compute. So which ordering should they implement? Arguably,
+//! they should offer both orders and let the user choose, but they can only implement the trait once. For these kind of types,
+/// for which multiple orderings exist, it might make sense to not implement any order at all, but use another mechanism 
+//! (e.g. a wrapper type) to allow users to select the order they want. 


I don't see what's so hard about this TBH. Once you have a type and a relation (or two, = and <) fixed, there is no doubt any more about which traits to implement.

RalfJung · 2018-08-30T16:25:18Z

src/libcore/cmp.rs

-/// Trait for values that can be compared for a sort-order.
+/// Trait for values that can be partially ordered.
+///
+/// This traits defines the operators `<`, `>`, `<=`, and `>=` which compare two values and return `bool`, 


"This trait"

RalfJung · 2018-08-30T16:27:59Z

src/libcore/cmp.rs

-/// transitively: if `T: PartialOrd<U>` and `U: PartialOrd<V>` then `U: PartialOrd<T>` and `T:
-/// PartialOrd<V>`.
+/// * _transitivity_: if `a < b` and `b < c` then `a < c`
+/// * _duality_: if `a < b` then `b > a`


Shouldn't it say somewhere that consistency with == from PartialEq is required?

Basically, this trait contains strictly more information than PartialEq, and it must be the case that == behaves exactly as if implemented via partial_cmp == Some(Equal).

RalfJung · 2018-08-30T16:50:21Z

I feel like the module introduction spends lots of text explaining things most users will not find relevant... all this array sorting and so on.

I would start by introducing the 4 traits, and then explain that the Partial* variants exist because there are types that to not satisfy all the usual properties we expect from Eq: For example, floating points, and finite sets. This shouldn't take nearly as much text as you have here now.

For PartialOrd (and this is a concern I already raised earlier but it seems it got lost), I'd focus everything around partial_cmp. This is the one thing the user is providing, and it contains all the information. So, as the introducing, I'd write

This traits defines the operators <, >, <=, and >= which compare two values and return bool. All of these operations are defined in terms of a single function, partial_cmp(a, b), which says how a and b are related: One can be smaller than the other, they can be equal -- and they can be unrelated. This is also the key difference to Ord, which does not allow elements to be unrelated.

With partial_cmp defined, it is easy to define all six comparison operators:

a < b if and only if partial_cmp(a, b) == Some(Less)

a > b if and only if partial_cmp(a, b) == Some(Greater)

a == b if and only if partial_cmp(a, b) == Some(Equal)

a != b if and only if !(a == b)

a <= b if and only if a < b || a == b

a >= b if and only if a > b || a == b

However, not every choice of partial_cmp makes sense. The following must be satisfied for all a, b, and c:

[transitivity and duality go here]

Moreover, partial_cmp must be consistent with PartialEq: == and != as defined above in terms of partial_cmp must produce the same results as the eq and ne methods from PartialEq. Finally, the lt (<), le (<=), gt (>), and ge (>=) methods of PartialEq must be consistent with the definition of the corresponding relations in terms of partial_cmp (this is the case for the default implementations, but if you choose to overwrite those for performance reasons, then it is your responsibility to uphold this guarantee).

Then go on about various corollaries (my open issue would be resolved by remarking that transitivity and duality still hold after swapping < and >), and partial strict ordering relations (Wikipedia link), and whatever else you think is worth mentioning.

TimNN · 2018-09-11T17:41:54Z

Ping from triage, @gnzlbg: Skimming the comments above, I believe this PR requires an update from you.

TimNN · 2018-09-25T17:39:34Z

Ping from triage @gnzlbg: it looks like this PR needs to be updated.

TimNN · 2018-10-16T17:32:33Z

Ping from triage @gnzlbg: I'm closing this due to inactivity per the triage procedure. Thank you for your contribution and please feel free to (re-)open this or another PR in the future.

Extend the documentation of PartialOrd

32d92d2

Closes rust-lang#50230 . cc @ExpHP @RalfJung @LukasKalbertodt @fbstj

rust-highfive assigned alexcrichton Aug 15, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 15, 2018

gnzlbg commented Aug 15, 2018

View reviewed changes

rust-highfive assigned steveklabnik and unassigned alexcrichton Aug 15, 2018

pitdicker reviewed Aug 15, 2018

View reviewed changes

LukasKalbertodt mentioned this pull request Aug 19, 2018

Tracking issue for RFC 2351, "Add is_sorted to the standard library" #53485

Closed

10 tasks

LukasKalbertodt suggested changes Aug 27, 2018

View reviewed changes

RalfJung reviewed Aug 30, 2018

View reviewed changes

gnzlbg mentioned this pull request Aug 31, 2018

PartialEq example is not symmetric #53844

Closed

TimNN added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2018

TimNN closed this Oct 16, 2018

TimNN added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 16, 2018

Extend the documentation of PartialOrd #53386

Extend the documentation of PartialOrd #53386

Conversation

gnzlbg commented Aug 15, 2018

rust-highfive commented Aug 15, 2018

gnzlbg commented Aug 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rust-highfive commented Aug 15, 2018

gnzlbg commented Aug 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietroalbini commented Aug 27, 2018

LukasKalbertodt left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnzlbg commented Aug 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung commented Aug 30, 2018

TimNN commented Sep 11, 2018

TimNN commented Sep 25, 2018

TimNN commented Oct 16, 2018

LukasKalbertodt left a comment •

edited

Loading