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

add intmax #17896

Merged
merged 1 commit into from
Oct 24, 2014
Merged

add intmax #17896

merged 1 commit into from
Oct 24, 2014

Conversation

mahkoh
Copy link
Contributor

@mahkoh mahkoh commented Oct 9, 2014

Closes #17075

I don't know if this is correct. The easiest way to find out is to run the following program on all targets but I can't do it myself.

#include <stdint.h>
#include <stdio.h>

int main(void)
{
    if (sizeof(intmax_t) != 8) {
        puts("ERROR");
        return 1;
    }
}

@thestinger
Copy link
Contributor

The sad thing about intmax_t is that it's incorrect. It doesn't actually correspond to the largest integer on most platforms for ABI compatibility reasons.

@nodakai
Copy link
Contributor

nodakai commented Oct 12, 2014

@thestinger Could you elaborate on that? Do you perhaps have __int128_t in your mind? (There are also SIMD types such as __m128 or __vector but they are irrelevant for this discussion.)

For everyone's reference: POSIX on stdint.h:

@thestinger
Copy link
Contributor

@nodakai: intmax_t is frozen as part of the platform ABI on every major OS so it does not correspond to anything meaningful. There are integer types larger than it, but it will not change in size because it would break the ABI.

@@ -1426,6 +1435,9 @@ pub mod types {
pub type uintptr_t = u32;
#[cfg(target_arch = "x86_64")]
pub type uintptr_t = u64;

pub type intmax_t = i64;
pub type uintmax_t = i64;
Copy link
Member

Choose a reason for hiding this comment

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

s/i/u/

@nodakai
Copy link
Contributor

nodakai commented Oct 12, 2014

@thestinger I mean, I was curious what you had in your mind as an example of "integer types larger than (intmax_t)". __int128_t is one of them but only GCC and Clang support it as an extension and I know of no C library using it in its public headers.

Though I don't think intmax_t can be essentially useful, I think it makes a sense to import as many C standard types as possible.

Wow, OpenSSL internally depends on __int128_t. Bit shift by 107 bits!

@thestinger
Copy link
Contributor

I think we should have it all there unconditionally and it should be generated from the C headers without any additional organization. However, the current way of doing things is that libc is only for stuff that's generally correct / useful / portable and not exposed in some other way by Rust.

@mahkoh
Copy link
Contributor Author

mahkoh commented Oct 13, 2014

It would be helpful if someone with access to the bots could check if intmax_ is i64 everywhere.

@pcwalton
Copy link
Contributor

@thestinger Do you have any objections to this? I am OK with it in general but I don't have a real opinion. Just want to take action on this PR.

@thestinger
Copy link
Contributor

@pcwalton: I don't think we should pick and choose the parts of libc that we expose, but we do approach it that way right now and I don't think intmax_t is actually useful. There are many useful things that are left out and pull requests adding those have been rejected in the past.

@thestinger
Copy link
Contributor

If it was up to me, we would use automatic binding generation or at least pretend we were using it. That would mean exposing everything C does without organization into namespaces and not making our own decisions about which functionality is useful / portable. So under that model, this would be acceptable.

@alexcrichton
Copy link
Member

While not explicitly spelled out, the purpose of libsys (the current liblibc) in the runtime removal RFC is to house as much of C header files as possible. Work on this has not gone much underway just yet, however.

bors added a commit that referenced this pull request Oct 24, 2014
Closes #17075

I don't know if this is correct. The easiest way to find out is to run the following program on all targets but I can't do it myself.
```c
#include <stdint.h>
#include <stdio.h>

int main(void)
{
	if (sizeof(intmax_t) != 8) {
		puts("ERROR");
		return 1;
	}
}
```
@bors bors closed this Oct 24, 2014
@bors bors merged commit 3ff7a3d into rust-lang:master Oct 24, 2014
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 29, 2024
…kril

internal: Properly check the edition for edition dependent syntax kinds

This puts the current edition in a bunch of places, most of which I annoted with FIXMEs asside from the ones in ide-assists because I couldnt bother with those
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 29, 2024
This PR touches a lot of parts. But the main changes are changing
`hir_expand::Name` to be raw edition-dependently and only when necessary
(unrelated to how the user originally wrote the identifier),
and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware
(this was done in rust-lang#17896, but the FIXMEs were fixed here).

It is possible that I missed some cases, but most IDE parts should properly
escape (or not escape) identifiers now.

The rules of thumb are:

 - If we show the identifier to the user, its rawness should be determined
   by the edition of the edited crate. This is nice for IDE features,
   but really important for changes we insert to the source code.
 - For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update
   tests when an edition becomes stable, to avoid churn).
 - For debugging tools (helper methods and logs), I used `Edition::LATEST`.
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 29, 2024
…-keyword, r=Veykril

fix: Properly account for editions in names

This PR touches a lot of parts. But the main changes are changing `hir_expand::Name` to be raw edition-dependently and only when necessary (unrelated to how the user originally wrote the identifier), and changing `is_keyword()` and `is_raw_identifier()` to be edition-aware (this was done in rust-lang#17896, but the FIXMEs were fixed here).

It is possible that I missed some cases, but most IDE parts should properly escape (or not escape) identifiers now.

The rules of thumb are:

 - If we show the identifier to the user, its rawness should be determined by the edition of the edited crate. This is nice for IDE features, but really important for changes we insert to the source code.
 - For tests, I chose `Edition::CURRENT` (so we only have to (maybe) update tests when an edition becomes stable, to avoid churn).
 - For debugging tools (helper methods and logs), I used `Edition::LATEST`.

Reviewing notes:

This is a really big PR but most of it is mechanical translation. I changed `Name` displayers to require an edition, and followed the compiler errors. Most methods just propagate the edition requirement. The interesting cases are mostly in `ide-assists`, as sometimes the correct crate to fetch the edition from requires awareness (there may be two). `ide-completions` and `ide-diagnostics` were solved pretty easily by introducing an edition field to their context. `ide` contains many features, for most of them it was propagated to the top level function and there the edition was fetched based on the file.

I also fixed all FIXMEs from rust-lang#17896. Some required introducing an edition parameter (usually not for many methods after the changes to `Name`), some were changed to a new method `is_any_identifier()` because they really want any possible keyword.

Fixes rust-lang#17895.
Fixes rust-lang#17774.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libc doesn't have {u,}intmax_t
8 participants