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

Rustdoc needs to handle conditional compilation somehow #1998

Open
brson opened this Issue Mar 16, 2012 · 27 comments

Comments

Projects
None yet
@brson
Copy link
Contributor

brson commented Mar 16, 2012

We have some functions that are reimplemented per architecture, with docs only on one implementation. Rustdoc should ideally be able to merge these docs.

More seriously though a bunch of our libc hierarchy can't be viewed because it's conditionally compiled and the docs are built on linux.

Probably rustdoc should extract its documentation before pruning the AST for conditional compilation, collapse things with the same name into a single document, then run resolve.

@graydon

This comment has been minimized.

Copy link
Contributor

graydon commented Mar 21, 2012

I think the latter case is probably best, yes. Collapse-same-name is a bit of a kludge but not entirely unexpected.

@ghost ghost assigned brson Apr 13, 2012

@catamorphism

This comment has been minimized.

Copy link
Contributor

catamorphism commented Jun 10, 2013

Not backwards-compatible. Nominating for milestone 4, well-covered (having documentation tools work the way we expect is helpful in making sure the documentation covers everything...)

@graydon

This comment has been minimized.

Copy link
Contributor

graydon commented Jun 13, 2013

doc-coverage can be assured elsewhere, but this is embarrassing. should be fixed before production ready.

@graydon

This comment has been minimized.

Copy link
Contributor

graydon commented Jun 13, 2013

accepted for production-ready milestone

@msullivan

This comment has been minimized.

Copy link
Contributor

msullivan commented Aug 10, 2013

Nothing new to say except to link to #5413.

@cmr

This comment has been minimized.

Copy link
Member

cmr commented Aug 15, 2013

#8125

This is not yet fixed by rustdoc_ng, and requires a bit of thought.

@cmr

This comment has been minimized.

Copy link
Member

cmr commented Sep 18, 2013

This is a hard problem with no good solution. If I use the AST before configuration, I can't have any hyperlinking for those items. Is that worth it?

@catamorphism

This comment has been minimized.

Copy link
Contributor

catamorphism commented Oct 24, 2013

Low, no milestone

@cmr

This comment has been minimized.

Copy link
Member

cmr commented Jul 28, 2014

My current thinking:

  1. Parse, get unexpanded, unconfigured AST
  2. Pull out everything cfg'd with the same name into sidetable
  3. Configure, expand.
  4. Pull out everything with the same name again.
  5. Run rest of core.

Then, later, when cleaning, check if the item's nodeid is in the sidetable of cfg'd things. If so, add them to a vec of "alternatives" in the item, cleaning each.

This depends on:

  1. The AST never being renumbered. (I don't think this is true today? @pcwalton?)
  2. Holding onto a reference to the pre-expanded AST will cause that node to still be alive later on, but not otherwise affect the AST. (I think this is true today.)

When rendered, we'd have a list of "alternative definitions", listing the cfg's that would have enabled them. This of course doesn't handle things that are actually conditionally enabled/disabled, just things that are defined multiple times.

@pcwalton

This comment has been minimized.

Copy link
Contributor

pcwalton commented Jul 29, 2014

I'm not sure about AST renumberings off the top of my head.

@KostaCitrix

This comment has been minimized.

Copy link

KostaCitrix commented Jul 13, 2015

Please fix. It's really confusing to see missing/wrong documentation if working on non-linux. This is one more 'gotcha' that you need to know, so this makes it harder to start working with rust on non-linux platform.

Thank you for you consideration! :)

@cmr

This comment has been minimized.

Copy link
Member

cmr commented Jul 14, 2015

@KostaCitrix this is a challenging issue to fix and we're all aware of the usability impacts this has. Demanding "please fix" doesn't really help solve the technical issues.

@tomjakubowski

This comment has been minimized.

Copy link
Contributor

tomjakubowski commented Jul 16, 2015

This is potentially less of a problem now that docs are installed with the distribution. It does seem, though, like this fact isn't particularly well known to newcomers.

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jul 16, 2015

In some ways, but it still doesn't lay out a good welcome mat to those who aren't using Linux.

@retep998

This comment has been minimized.

Copy link
Member

retep998 commented Jul 17, 2015

I ran out of fingers to count the number of times I've seen people not find Windows specific extension traits because the online docs were linux-only.

@tomjakubowski

This comment has been minimized.

Copy link
Contributor

tomjakubowski commented Jul 17, 2015

How about building docs on all three supported platforms and uploading to doc.rust-lang.org/nightly/windows, nightly/linux, etc?

Of course this doesn't solve the general problem of rustdoc and conditional compilation.

On Jul 16, 2015, at 19:32, Peter Atashian notifications@github.com wrote:

I ran out of fingers to count the number of times I've seen people not find Windows specific extension traits because the online docs were linux-only.


Reply to this email directly or view it on GitHub.

@lilyball

This comment has been minimized.

Copy link
Contributor

lilyball commented Jul 26, 2015

One potential problem with trying to do the AST approach is if the same function/type is defined for multiple platforms with different doc comments, which doc comment do we show for the item? Or maybe even more importantly, same function with different types, or same type with different public fields?

One possible solution might be to add a selector somewhere on the page that lets you pick the platform you wish to view, and it would use that to show the correct documentation/types/fields. It could still show AST elements that aren't available on that platform but perhaps greyed out, to indicate that they're not available.

Another solution is to do what @tomjakubowski suggested and just upload docs for each platform to a different path. The main rustdoc UI could then still have the selector, which changes the URL to view that platform's documentation (and sets a cookie that is used by the non-platform-specific documentation path to automatically jump to the platform-specific path). One downside to this is every documentation URL that people share would then be a platform-specific URL.

@jonas-schievink

This comment has been minimized.

Copy link
Member

jonas-schievink commented Feb 6, 2016

One potential problem with trying to do the AST approach is if the same function/type is defined for multiple platforms with different doc comments, which doc comment do we show for the item?

In this case, I think we should emit a warning (that can be made into an error via a command line flag, to make this useful for CI or local testing, but still preserves compatibility) and just pick one (maybe the native one for compatibility). If only one of these item has docs, this is a non-issue (which should be the preferred way to document this IMO).

Or maybe even more importantly, same function with different types, or same type with different public fields?

I'm not sure about what a type means to Rustdoc, but I don't think it uses type information. This would mean that at least type aliases are handled automatically: The type can be something like libc::size_t, which is defined to something different depending on platform, but still acts the same way.

Now all remaining cases should be rare, so we could emit a warning for them as well. Which item we actually pick... no idea, maybe the same item we would pick today (like above).

I could also imagine solving these 2 issues by just adding docs for both items and marking them with their cfg attribute (something that would be nice in any case).

How about building docs on all three supported platforms

http://doc.rust-lang.org/libc is apparently doing that already, but it's not the best from a usability perspective, and I don't think std is doing the same, which is the whole reason I wrote this ;) (needed some Windows-specific trait, didn't find it, got annoyed at Rustdoc)

@aochagavia

This comment has been minimized.

Copy link
Contributor

aochagavia commented Jun 17, 2016

Note that this also has an effect on the save-analysis API (and it will be an issue when the RLS is implemented). For instance: if you are working on Windows, but are editing Linux-only code, the RLS won't do anything for you (no code completion, no jump to definition, etc) since the edited code is effectively invisible to save-analysis.

cc @nrc

@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Jun 27, 2016

crates.fyi is another solution to this issue /cc @onur

@nrc

This comment has been minimized.

Copy link
Member

nrc commented Jun 27, 2016

I don't think there is a good answer to this in rustdoc itself. The better answer (which Steve hints at) is that we should build and host docs for every platform, not just Linux.

@retep998

This comment has been minimized.

Copy link
Member

retep998 commented Jun 27, 2016

Tell me when winapi's docs are finally up on crates.fyi

@retep998

This comment has been minimized.

Copy link
Member

retep998 commented Sep 7, 2016

As an update to this, crates.fyi is now docs.rs and has documentation for all tier 1 platforms. It even has winapi docs.

@valarauca

This comment has been minimized.

Copy link

valarauca commented Nov 23, 2016

In the short term isn't this solved with

#[cfg(any(feature = "doc", target_yada="foobar"))]
@golddranks

This comment has been minimized.

Copy link
Contributor

golddranks commented Jun 23, 2017

In addition to simply unifying different features and platforms in the docs, there should be a clear visual indicator that some API is available only in the presence of some particular feature – this is especially important for features, which unlike platforms are easily toggleable but hard to discover.

@QuietMisdreavus

This comment has been minimized.

Copy link
Member

QuietMisdreavus commented Jul 29, 2017

I've thought a bit about this recently, so i'll brain-dump into this issue in case anyone else is interested:

I call this the "holy grail" of rustdoc, because all the ways i know of to scrape a crate for docs will hit against some kind of fundamental limitation. Using the compiler itself (like rustdoc currently does) or using save-analysis (like RLS and the pending rustdoc rewrite) requires you to build or partially build the item in question, forcing you to pick a target and lose all the cfg-tagged things that don't match it.

Side-stepping the compiler and reading the raw source code yourself (i.e. going through syn or the like) gets you more information, but there's a serious road-block: anything to do with macros or build scripts. From what i've been told, macro expansion and conditional compilation happen at the same time, because one can affect the other. Therefore, it's not possible (at the moment, at least) to get the equivalent of --pretty=expanded without also processing cfg attributes. You're stuck either implementing macro expansion yourself as well or just ignoring anything that happens in a macro. Ditto for build scripts, which need you to go through cargo to properly build and execute it, and you're stuck in the same problem as before - you have to pick a target.

To do this with the do-it-yourself /syn approach, you'd need to scan the crate first to find any cfg attribute, see if you can parse what the attribute needs, pick a target that satisfies it, then run --pretty=expanded on the crate to get the cfg'd items specifically for that. Merge those in somehow with the "base" run (for the default target) and render that. Putting this in current rustdoc or in the analysis generation for RLS would require all of this to happen basically before anything else in the compilation flow.

My idea for surfacing these things in docs is to duplicate the item for each cfg, adding a marker to something indicating the contents of the attribute and showing each one in turn. That way, in case the docs for something is different for a given platform, you can see exactly which platform(s) it applies to. This can lead to some duplication or awkward formatting, especially with something like libc or anything that uses cfg_if to assemble complex conditional compilation setups. (cfg_if is a perfect example of macros and conditional compilation working together to make this really complicated.) For example, in std::os:

  • cfg(unix) unix: Experimental extensions to std for Unix platforms.
  • cfg(windows) windows: Experimental extensions to std for Windows.

Or among the trait implementations for File:

  • cfg(unix) impl AsRawFd for File
    • (contents of unix::io::AsRawFd...)

(...)

  • cfg(windows) impl AsRawHandle for File
    • (contents of windows::io::AsRawHandle...)

...on the other hand, this doesn't take into account doing something like cfg_attr(unix, doc = "something for unix")... >_> But it should get common cases.

@kennytm kennytm referenced this issue Aug 10, 2017

Open

Tracking issue for `#[doc(cfg(...))]` #43781

1 of 3 tasks complete

bors added a commit that referenced this issue Aug 10, 2017

Auto merge of #43348 - kennytm:fix-24658-doc-every-platform, r=alexcr…
…ichton

Expose all OS-specific modules in libstd doc.

1. Uses the special `--cfg dox` configuration passed by rustbuild when running `rustdoc`. Changes the `#[cfg(platform)]` into `#[cfg(any(dox, platform))]` so that platform-specific API are visible to rustdoc.

2. Since platform-specific implementations often won't compile correctly on other platforms, `rustdoc` is changed to apply `everybody_loops` to the functions during documentation and doc-test harness.

3. Since platform-specific code are documented on all platforms now, it could confuse users who found a useful API but is non-portable. Also, their examples will be doc-tested, so must be excluded when not testing on the native platform. An undocumented attribute `#[doc(cfg(...))]` is introduced to serve the above purposed.

Fixes #24658 (Does _not_ fully implement #1998).

bors added a commit that referenced this issue Aug 11, 2017

Auto merge of #43348 - kennytm:fix-24658-doc-every-platform, r=alexcr…
…ichton

Expose all OS-specific modules in libstd doc.

1. Uses the special `--cfg dox` configuration passed by rustbuild when running `rustdoc`. Changes the `#[cfg(platform)]` into `#[cfg(any(dox, platform))]` so that platform-specific API are visible to rustdoc.

2. Since platform-specific implementations often won't compile correctly on other platforms, `rustdoc` is changed to apply `everybody_loops` to the functions during documentation and doc-test harness.

3. Since platform-specific code are documented on all platforms now, it could confuse users who found a useful API but is non-portable. Also, their examples will be doc-tested, so must be excluded when not testing on the native platform. An undocumented attribute `#[doc(cfg(...))]` is introduced to serve the above purposed.

Fixes #24658 (Does _not_ fully implement #1998).

bors added a commit that referenced this issue Aug 13, 2017

Auto merge of #43348 - kennytm:fix-24658-doc-every-platform, r=alexcr…
…ichton

Expose all OS-specific modules in libstd doc.

1. Uses the special `--cfg dox` configuration passed by rustbuild when running `rustdoc`. Changes the `#[cfg(platform)]` into `#[cfg(any(dox, platform))]` so that platform-specific API are visible to rustdoc.

2. Since platform-specific implementations often won't compile correctly on other platforms, `rustdoc` is changed to apply `everybody_loops` to the functions during documentation and doc-test harness.

3. Since platform-specific code are documented on all platforms now, it could confuse users who found a useful API but is non-portable. Also, their examples will be doc-tested, so must be excluded when not testing on the native platform. An undocumented attribute `#[doc(cfg(...))]` is introduced to serve the above purposed.

Fixes #24658 (Does _not_ fully implement #1998).
@steveklabnik

This comment has been minimized.

Copy link
Member

steveklabnik commented Mar 11, 2019

Triage: this is still the holy grail. cfg(rustdoc) lets us do some things that this would be useful for, but not all of them. I'm not sure that fixing this is possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.