Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc][docs] Updates implementation status for some preexisting docgen json files #89281

Merged
merged 2 commits into from
Apr 18, 2024

Conversation

Flandini
Copy link
Contributor

Moving towards displaying impl status of standard header macros. The macros aren't handled by docgen yet, so I haven't included updated rst files.

Add @nickdesaulniers for review.

@llvmbot llvmbot added the libc label Apr 18, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 18, 2024

@llvm/pr-subscribers-libc

Author: Michael Flanders (Flandini)

Changes

Moving towards displaying impl status of standard header macros. The macros aren't handled by docgen yet, so I haven't included updated rst files.

Add @nickdesaulniers for review.


Full diff: https://github.com/llvm/llvm-project/pull/89281.diff

3 Files Affected:

  • (modified) libc/utils/docgen/fenv.json (+78-3)
  • (modified) libc/utils/docgen/signal.json (+45-18)
  • (modified) libc/utils/docgen/stdbit.json (+74-20)
diff --git a/libc/utils/docgen/fenv.json b/libc/utils/docgen/fenv.json
index 0af38b16b2d982..a93ee590351083 100644
--- a/libc/utils/docgen/fenv.json
+++ b/libc/utils/docgen/fenv.json
@@ -1,7 +1,82 @@
 {
-  "macros": [
-    "__STDC_VERSION_FENV_H__"
-  ],
+  "macros": {
+    "__STDC_VERSION_FENV_H__": {
+      "defined": "7.6.5",
+      "implemented": false
+    },
+    "FE_DIVBYZERO": {
+      "defined": "7.6.9",
+      "implemented": true
+    },
+    "FE_INEXACT": {
+      "defined": "7.6.9",
+      "implemented": true
+    },
+    "FE_INVALID": {
+      "defined": "7.6.9",
+      "implemented": true
+    },
+    "FE_OVERFLOW": {
+      "defined": "7.6.9",
+      "implemented": true
+    },
+    "FE_UNDERFLOW": {
+      "defined": "7.6.9",
+      "implemented": true
+    },
+    "FE_ALL_EXCEPT": {
+      "defined": "7.6.12",
+      "implemented": true
+    },
+    "FE_DFL_MODE": {
+      "defined": "7.6.11",
+      "implemented": false
+    },
+    "FE_DOWNARD": {
+      "defined": "7.6.13",
+      "implemented": true
+    },
+    "FE_TONEAREST": {
+      "defined": "7.6.13",
+      "implemented": true
+    },
+    "FE_TONEARESTFROMZERO": {
+      "defined": "7.6.13",
+      "implemented": false
+    },
+    "FE_TOWARDZERO": {
+      "defined": "7.6.13",
+      "implemented": true
+    },
+    "FE_UPWARD": {
+      "defined": "7.6.13",
+      "implemented": true
+    },
+    "FE_DEC_DOWNWARD": {
+      "defined": "7.6.14",
+      "implemented": false
+    },
+    "FE_DEC_TONEAREST": {
+      "defined": "7.6.14",
+      "implemented": false
+    },
+    "FE_DEC_TONEARESTFROMZERO": {
+      "defined": "7.6.14",
+      "implemented": false
+    },
+    "FE_DEC_TOWARDZERO": {
+      "defined": "7.6.14",
+      "implemented": false
+    },
+    "FE_DEC_UPWARD": {
+      "defined": "7.6.14",
+      "implemented": false
+    },
+    "FE_DFL_ENV": {
+      "defined": "7.6.17",
+      "implemented": true
+    }
+  },
   "functions": {
     "feclearexcept": {
       "defined": "7.6.4.1"
diff --git a/libc/utils/docgen/signal.json b/libc/utils/docgen/signal.json
index 976021a803a672..e9eaae57364fb0 100644
--- a/libc/utils/docgen/signal.json
+++ b/libc/utils/docgen/signal.json
@@ -1,16 +1,49 @@
 {
-  "macros": [
-    "SIG_DFL",
-    "SIG_ERR",
-    "SIG_IGN",
-    "SIGABRT",
-    "SIGFPE",
-    "SIGILL",
-    "SIGINT",
-    "SIGSEGV",
-    "SIGTERM"
-  ],
+  "macros": {
+    "SIG_DFL": {
+      "defined": "7.14.3",
+      "implemented": false
+    },
+    "SIG_ERR": {
+      "defined": "7.14.3",
+      "implemented": false
+    },
+    "SIG_IGN": {
+      "defined": "7.14.3",
+      "implemented": false
+    },
+    "SIGABRT": {
+      "defined": "7.14.3",
+      "implemented": true
+    },
+    "SIGFPE": {
+      "defined": "7.14.3",
+      "implemented": true
+    },
+    "SIGILL": {
+      "defined": "7.14.3",
+      "implemented": true
+    },
+    "SIGINT": {
+      "defined": "7.14.3",
+      "implemented": true
+    },
+    "SIGSEGV": {
+      "defined": "7.14.3",
+      "implemented": true
+    },
+    "SIGTERM": {
+      "defined": "7.14.3",
+      "implemented": true
+    }
+  },
   "functions": {
+    "signal": {
+      "defined": "7.14.1.1"
+    },
+    "raise": {
+      "defined": "7.14.2.1"
+    },
     "kill": null,
     "sigaction": null,
     "sigaddset": null,
@@ -18,12 +51,6 @@
     "sigdelset": null,
     "sigemptyset": null,
     "sigfillset": null,
-    "sigprocmask": null,
-    "signal": {
-      "defined": "7.14.1.1"
-    },
-    "raise": {
-      "defined": "7.14.2.1"
-    }
+    "sigprocmask": null
   }
 }
diff --git a/libc/utils/docgen/stdbit.json b/libc/utils/docgen/stdbit.json
index 9dda0cb0f5383a..c295efa0bbe245 100644
--- a/libc/utils/docgen/stdbit.json
+++ b/libc/utils/docgen/stdbit.json
@@ -1,24 +1,78 @@
 {
-  "macros": [
-    "__STDC_VERSION_STDBIT_H__",
-    "__STDC_ENDIAN_LITTLE__",
-    "__STDC_ENDIAN_BIG__",
-    "__STDC_ENDIAN_NATIVE__",
-    "stdc_leading_zeros",
-    "stdc_leading_ones",
-    "stdc_trailing_zeros",
-    "stdc_trailing_ones",
-    "stdc_first_leading_zero",
-    "stdc_first_leading_one",
-    "stdc_first_trailing_zero",
-    "stdc_first_trailing_one",
-    "stdc_count_zeros",
-    "stdc_count_ones",
-    "stdc_has_single_bit",
-    "stdc_bit_width",
-    "stdc_bit_floor",
-    "stdc_bit_ceil"
-  ],
+  "macros": {
+    "__STDC_VERSION_STDBIT_H__": {
+      "defined": "7.18.1.2",
+      "implemented": true
+    },
+    "__STDC_ENDIAN_LITTLE__": {
+      "defined": "7.18.2.2",
+      "implemented": true
+    },
+    "__STDC_ENDIAN_BIG__": {
+      "defined": "7.18.2.2",
+      "implemented": true
+    },
+    "__STDC_ENDIAN_NATIVE__": {
+      "defined": "7.18.2.2",
+      "implemented": true
+    },
+    "stdc_leading_zeros": {
+      "defined": "7.18.3.1",
+      "implemented": true
+    },
+    "stdc_leading_ones": {
+      "defined": "7.18.4.1",
+      "implemented": true
+    },
+    "stdc_trailing_zeros": {
+      "defined": "7.18.5.1",
+      "implemented": true
+    },
+    "stdc_trailing_ones": {
+      "defined": "7.18.6.1",
+      "implemented": true
+    },
+    "stdc_first_leading_zero": {
+      "defined": "7.18.7.1",
+      "implemented": true
+    },
+    "stdc_first_leading_one": {
+      "defined": "7.18.8.1",
+      "implemented": true
+    },
+    "stdc_first_trailing_zero": {
+      "defined": "7.18.9.1",
+      "implemented": true
+    },
+    "stdc_first_trailing_one": {
+      "defined": "7.18.10.1",
+      "implemented": true
+    },
+    "stdc_count_zeros": {
+      "defined": "7.18.11.1",
+      "implemented": true
+    },
+    "stdc_count_ones": {
+      "defined": "7.18.12.1",
+      "implemented": true
+    },
+    "stdc_has_single_bit": {
+      "defined": "7.18.13.1",
+      "implemented": true
+    },
+    "stdc_bit_width": {
+      "defined": "7.18.14.1",
+      "implemented": true
+    },
+    "stdc_bit_floor": {
+      "defined": "7.18.15.1",
+      "implemented": true
+    },
+    "stdc_bit_ceil": {
+      "defined": "7.18.16.1",
+      "implemented": true
+    }
+  },
   "functions": {
     "stdc_leading_zeros_uc": {
       "defined": "7.18.3"

@nickdesaulniers nickdesaulniers self-requested a review April 18, 2024 18:00
libc/utils/docgen/fenv.json Outdated Show resolved Hide resolved
},
"raise": {
"defined": "7.14.2.1"
},
Copy link
Member

Choose a reason for hiding this comment

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

I haven't decided yet how best to "sort" these.

I think we might even want to change the "schema" for defined because:

  1. some things are only specified by POSIX, which we can hyperlink to.
  2. some things are defined by multiple standards (such as the C language standard AND POSIX)

Due to 2, these should ultimately be a list, or perhaps another object with distinct fields for C standard section number and hyperlink to POSIX docs (or linux man pages, for other APIs even).


I guess orthogonal to that though, is how do we want to keep things "in order" in the config files, and/or in the generated rst files?

If I can't highlight rows in vim and simply :sort, then maintaining a sort manually is a PITA. So I don't think we need to maintain or require sorting the json configs.

I guess a lot of bloviating, but all that to ask what is your reasoning here for moving signal+raise and do we need to change the current sort here in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess a lot of bloviating, but all that to ask what is your reasoning here for moving signal+raise and do we need to change the current sort here in this PR?

I have no strong opinions on the orderings. Based on the previous files, I've been keeping the json files in order of occurrence in the C standard for ease of auditing alongside the standard. This is why I bumped signal and raise up, I was going to let the POSIX functions fall after but totally fine with reverting or alphabetically sorting. docgen.py is sorting everything in the rst files alphabetically by function/macro name currently. Order in the rst docs seems more, 'how will people use it'?

Due to 2, these should ultimately be a list, or perhaps another object with distinct fields for C standard section number and hyperlink to POSIX docs (or linux man pages, for other APIs even).

Yeah, I haven't started on listing POSIX hyperlinks yet. I was leaning towards the latter solution with distinct fields and changing docgen.py to: change the "Standard" rst column to "C23 Standard", add an optional POSIX standard# column (do we need the standard numbers to differentiate C/POSIX standard versions?) that is only added to the generated rst if some row has a POSIX reference in the json.

Copy link
Member

Choose a reason for hiding this comment

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

I was going to let the POSIX functions fall

POSIX-only

docgen.py is sorting everything in the rst files alphabetically by function/macro name currently.

Yeah, I noticed. I suspect the json parser is using ordered dictionaries.

I think my feeling here is: if it's easy to sort via automation, keep it sorted. Otherwise, whatever. At least for the config files.

Order in the rst docs seems more, 'how will people use it'?

Right. Let's wait to collect feedback from more consumers before making a decision. We could sort by C standard section for instance. Then perhaps POSIX sections. Let's hold on to that thought until later though.


I was leaning towards the latter solution with distinct fields

SGTM

do we need the standard numbers to differentiate C/POSIX standard versions?

So far, we're only tracking the latest accepted C standard. You'll notice the sections jump around between C standard revisions because new functions get added in the middle of existing sections.

So I think we should do the same for POSIX and just refer to POSIX.1-2017.

IIUC some functions were deprecated by newer POSIX standards, but many programs won't be rewritten to reflect that, so most libc's need to provide even deprecated POSIX functionality to be somewhat portable-to. IDK if POSIX has ever removed anything; in that case we'd have to point back to prior POSIX releases.

Let's ask the maintainer of bionic (Android's libc), @enh-google , if he knows answers to the above?

To me, currently, docgen is more about:

  1. reflecting to users how "done" we are via some form of visualization (even if fragmented across numerous html pages)
  2. visualizing what could be implemented next, either by the core team or new contributors.
  3. helping implementers quickly know which section of which doc they should read to understand the semantics required by "the spec" for whichever specs are relevant.

Maybe someday docgen grows into more (such as when these things were first standardized), but I'd rather folks contribute to the upstream man pages project as duplicating what they have is frankly a waste of time and is user hostile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I noticed. I suspect the json parser is using ordered dictionaries.

I thought the current sorting in the generated rst docs was on purpose, there is an explicit sort on function names.

Another ordering to take into account, that does bug me a bit, is the ordering of the listed headers in the left side bar on libc.llvm.org: "Search Tables" and "C23 Support" in the middle of the standard headers. Thoughts on reordering these to push the standard header status to the bottom on the side bar and alphabetically sorted?

Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC some functions were deprecated by newer POSIX standards, but many programs won't be rewritten to reflect that, so most libc's need to provide even deprecated POSIX functionality to be somewhat portable-to. IDK if POSIX has ever removed anything; in that case we'd have to point back to prior POSIX releases.

Let's ask the maintainer of bionic (Android's libc), @enh-google , if he knows answers to the above?

yes, POSIX has removed things. no, most C libraries do not remove those things.

i regret all of the functions i removed. it didn't gain me anything (because we'd already done the work), and it hurt developers (if only slightly).

that said, i do not regret the ones i never added https://android.googlesource.com/platform/bionic/+/main/docs/status.md#posix --- POSIX is full of useless crap while also missing lots of essential stuff. (theoretically that makes sense viewed from their "codifying existing practice" stance, but at the same time they remove tar(1) in favor of pax(1), or add gzip to compress(1) rather than add gzip(1), so their grasp of "existing practice" is tenuous at best.)

Copy link
Member

Choose a reason for hiding this comment

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

I thought the current sorting in the generated rst docs was on purpose, there is an explicit sort on function names.

Oh, that's right. I wrote that. Either way, I don't have my heart set on that. What's more important to me is that we maintain some form of determinism in output in case folks decide to re-arrange the order of things in json. Though maybe we just keep the json sorted (painful) and rely on that ordering. Personally, I think it'll be easier to enforce the ordering if the tool just sorts every time, then we can add new stuff to the json in any order.

Another ordering to take into account, that does bug me a bit, is the ordering of the listed headers in the left side bar on libc.llvm.org

heh, I was just talking with @dpxf about that. yeah it's ugly and will grow to be unwieldy very quickly. I'd like to have one tab, that links to a newly created page, that lists each header we provide, which is a link to the corresponding page.

Copy link
Member

@nickdesaulniers nickdesaulniers left a comment

Choose a reason for hiding this comment

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

Thanks for the patch!

@nickdesaulniers nickdesaulniers merged commit 67669ea into llvm:main Apr 18, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants