[clang] Implement C23 <stdckdint.h>

[ZijunZhao]

Summary

https://github.com/llvm/llvm-project/issues/62248

[enh]

clang/lib/Headers/stdckdint.h

14

it does look like it _should_ be possible to not have it set though? llvm/llvm-project/clang/lib/Frontend/InitPreprocessor.cpp has:

if (LangOpts.GNUCVersion != 0) {
  // Major, minor, patch, are given two decimal places each, so 4.2.1 becomes
  // 40201.
  unsigned GNUCMajor = LangOpts.GNUCVersion / 100 / 100;
  unsigned GNUCMinor = LangOpts.GNUCVersion / 100 % 100;
  unsigned GNUCPatch = LangOpts.GNUCVersion % 100;
  Builder.defineMacro("__GNUC__", Twine(GNUCMajor));
  Builder.defineMacro("__GNUC_MINOR__", Twine(GNUCMinor));
  Builder.defineMacro("__GNUC_PATCHLEVEL__", Twine(GNUCPatch));
  Builder.defineMacro("__GXX_ABI_VERSION", "1002");

  if (LangOpts.CPlusPlus) {
    Builder.defineMacro("__GNUG__", Twine(GNUCMajor));
    Builder.defineMacro("__GXX_WEAK__");
  }
}

[enh]

clang/lib/Headers/stdckdint.h

14

/me wonders whether the right test here is actually #if __has_feature(__builtin_add_overflow) (etc)...

but at this point, you definitely need an llvm person :-)

[xbolva00]

clang/lib/Headers/stdckdint.h

14

#ifndef __has_builtin // Optional of course.

#define __has_builtin(x) 0  // Compatibility with non-clang compilers.

#endif

...
#if has_builtin(builtin_trap)

__builtin_trap();

#else

abort();

#endif

[yabinc]

clang/lib/Headers/stdckdint.h

14

From https://clang.llvm.org/docs/LanguageExtensions.html#checked-arithmetic-builtins, we can check them with
has_builtin(builtin_add_overflow) && has_builtin(builtin_sub_overflow) && has_builtin(builtin_mul_overflow).
I saw some code also checks if GNUC >= 5:

The GNUC checks can not be removed until we depend on GCC >= 10.1
which is the first version that returns true for has_builtin(builtin_add_overflow)
#if GNUC >= 5 || has_builtin(builtin_add_overflow)

I guess we don't need to support real gcc using this header here. So maybe only checking __has_builtin is enough?

By the way, if __builtin_add_overflow may not appear on some targets, do we need to modify tests to specify triple like "-triple "x86_64-unknown-unknown"" in https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/builtins-overflow.c#L5 ?

[hiraditya]

clang/lib/Headers/stdckdint.h

14

/me wonders whether the right test here is actually #if has_feature(builtin_add_overflow) (etc)...

i think that should be added.

I guess we also need a with __STDC_VERSION__ > 202000L? in princple we'd have a C23 number for it but i'm not sure if that has been added to clang yet.

[hiraditya]

clang/test/Headers/stdckdint.cpp

2

seems like we don't have a -std=gnu23, or -std=c23 standard flag for this in clang yet.

https://godbolt.org/z/7dKnGEWWE

we probably need it before testing stdckdint i guess?

[enh]

clang/lib/Headers/stdckdint.h

14

i think that should be added.

i was advising the opposite --- now this is a standard C23 feature, any architectures where __builtin_*_overflow doesn't work need to be found and fixed. and we'll do that quicker if we unconditionally expose these and (more importantly!) run the tests.

I guess we also need a with STDC_VERSION > 202000L?

_personally_ i think that's silly because you can't hide the header file, so it doesn't make any sense to just have it empty if someone's actually #included it. we don't do this anywhere in bionic for example, for this reason. but obviously that's an llvm decision, and it does look like the other similar headers _do_ have this check, so, yeah, probably.

clang/test/Headers/stdckdint.cpp

2

other headers just use > and the previous version. (though see stdalign.h if you're looking for some random cleanup to do!)

[ZijunZhao]

clang/test/Headers/stdckdint.cpp

2

seems like we don't have a -std=gnu23, or -std=c23 standard flag for this in clang yet.

In the local testing, -std=c++23 works and all tests pass😂

[enh]

clang/test/Headers/stdckdint.cpp

2

C23 != C++23... they don't even really coordinate with one another... talk to hboehm about that some time :-)

[ZijunZhao]

clang/test/Headers/stdckdint.cpp

2

ohhh I think gnu++23 != gnu23 either 😂

[enh]

clang/test/Headers/stdckdint.cpp

2

correct. the "c" or "c++" part means "standard stuff" and replacing it with "gnu" or "gnu++" means "standard stuff _and_ extensions".

[hiraditya]

clang/lib/Headers/stdckdint.h

14

i was advising the opposite -- now this is a standard C23 feature, any architectures where __builtin

you're right. it seems like __builtin_add_overflow is expected to be defined by default (https://github.com/llvm/llvm-project/blob/main/clang/test/Sema/builtins-overflow.c#L4).

and it does look like the other similar headers _do_ have this check, so, yeah, probably.

yeah, Several headers have checks for stdc_version that supported e.g., https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdint.h#L504.

nit: It will be nice to add a reference to C23 that added this. i.e., 7.20. example: https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/stdint.h#L910

[ZijunZhao]

clang/test/Headers/stdckdint.cpp

2

I try to grep "std>" in clang/test/Headers but find nothing, and nothing in stdalign.h is about >

[enh]

I try to grep "std>" in clang/test/Headers but find nothing, and nothing in stdalign.h is about >

grep for __STDC_VERSION__ instead.

[aaron.ballman]

Thank you for working on this! Adding some more reviewers for more opinions on the approach.

I think we should split the current test file up and move it around a bit. 1) It should be a .c test and not a .cpp test as this is C23 functionality. 2) I'd like to see the codegen functionality tested in clang/test/C/C2x/ (in a file with n2683 in the name, as that's the paper adding the feature); it should follow the same form as the other tests in that directory. Be sure to test the requirements of C2x 7.20.1p2. 3) There should be semantic tests for diagnostics added as well, like what happens when passing non-integer arguments, non-pointer result, different argument types, etc. These should also be in the clang/test/C/C2x directory with a file with n2683 in the name. Pay special attention to plain char, bool, and _BitInt types as operands (see 7.20.1p3).

You should also add a release note to clang/docs/ReleseNotes.rst so users know about the new feature.

And you should update clang/www/c_status.html to add a row for N2683 that says it's implemented in Clang 18; I had left the header file out of the list originally thinking this functionality would come from the c standard library linked into the application.

clang/lib/Headers/stdckdint.h

14

Yeah, I think this should be guarded by __STDC_VERSION__ and not __GNUC__ -- this is a C23 feature, not a GNU feature.

We could expose these APIs in earlier C modes, but the macros defined here do not use a reserved identifier and so we'd be stealing those names out from under the user and so we might not want to. We don't expose unreachable in earlier language modes (in stddef.h) or the width macros (in stdint.h), so I lean towards not exposing this functionality in older language modes.

We could also limit this functionality to just C (and not expose it in C++ mode). We don't do that for any of the other C standard library headers, however, so I think we should continue to expose the functionality in C++.

Also, should we be falling back to the system-installed header if in hosted mode and one exists?

The file is missing the __STDC_VERSION_STDCKDINT_H__ macro definition from C2x 7.20p2

19

This seems to be causing failures with precommit CI around modules builds.

clang/lib/Lex/PPDirectives.cpp

210–212

This code should be re-formatted so it fits within the usual 80-col limit again.

clang/test/Headers/stdckdint.cpp

2

This isn't a GNU feature, so we don't need to enable a GNU mode or set a gnu version. (Note, the file should be a .c file, not a .cpp file)

[cor3ntin]

clang/lib/Headers/stdckdint.h

14

The content of C headers in C++ mode is dictated by http://eel.is/c++draft/support.c.headers#other-1.
I think making them empty until WG21 has the opportunity to discuss a rebase of C++26 on top of C2x would make sense conservatively.

The fact that these things are macros and that C++ is working on c++-specific solution to the same problem makes me think that being conservative is wise

[ZijunZhao]

clang/lib/Headers/stdckdint.h

14

The file is missing the STDC_VERSION_STDCKDINT_H macro definition from C2x 7.20p2

https://en.cppreference.com/mwiki/index.php?title=STDC_VERSION_STDCKDINT_H&action=edit&redlink=1 I think this one needs to be done, too?

[enh]

clang/lib/Headers/stdckdint.h

14

cppreference.com doesn't usually have _pages_ for these macros. https://en.cppreference.com/w/c/types references several, for example, but without including pages for them. (or, if you're specifically looking for a __STDC_... macro, look at https://en.cppreference.com/w/c/numeric/complex for a couple of "we mention it, but it doesn't get its own page" examples.)

[aaron.ballman]

clang/lib/Headers/stdckdint.h

14

FWIW, you should rely on the standard working draft before relying on cppreference. It's not that cppreference is often wrong, it's more that it's not the Source of Truth either.

FWIW, the last publicly available version of the C2x working drafts is: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf -- I don't think anything materially changed between that working draft and the final publication (at least, I don't spot any differences when I compare with my C23 DIS copy).

[hiraditya]

clang/lib/Headers/stdckdint.h

14

specifically page 330, https://open-std.org/JTC1/SC22/WG14/www/docs/n3096.pdf#page=330

clang/test/Headers/stdckdint.cpp

2

Do we have plans to add -std=c23 anytime soon? -std=c2x defines __STDC_VERSION__ 202000L

[aaron.ballman]

clang/test/Headers/stdckdint.cpp

2

Do we have plans to add -std=c23 anytime soon? -std=c2x defines STDC_VERSION 202000L

We needed to wait for WG14 to finalize the standard before we could decide what name to use or what macro value to pick. Both are settled now, so I plan to make those changes Sometime Soon™

[ZijunZhao]

Another followup question: I check https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2683.pdf and I only add Core Proposal here. Do I need to add Supplemental Proposal, like some types?

[aaron.ballman]

In D157331#4575224, @ZijunZhao wrote:

Another followup question: I check https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2683.pdf and I only add Core Proposal here. Do I need to add Supplemental Proposal, like some types?

Ah, this is a bit confusing! tl;dr: No need to add the supplemental proposal.

The way to trace this down yourself is:

• The working draft (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf) has a list of what papers were adopted and applied to the standard. You'll find N2683 under the June 2021 virtual meeting heading. That's how you know which paper was adopted.
• You can look at the paper to see the proposed wording, prior art, motivation, etc. That gets you 95% of the way but you need to know what words actually went into the standard. So use the original paper to give you ideas on what to implement, what to test, etc, but verify against the wording in the standard.
• Because this was discussed at the June 2021 virtual meeting, you should look at those meeting minutes (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2802.pdf) to see what was adopted. You can search for 2683 to find the relevant discussion in the minutes. Most importantly, you can see what polls were taken. The two polls of interest were:

Straw Poll: Does the committee wish to adopt the Core proposal from N2683 into C23? 13-0-5 Core proposal goes into C23.
Straw Poll: Does the committee want the "ckd_" identifiers from N2683's Core proposal in future library directions to be potentially reserved identifiers? 18-0-0

This is how we know that only the core proposal was added, and not the supplemental proposal. Checking that belief against the standard wording itself verifies that only the core proposal was added and the supplemental bits are left out.

(Hopefully that explanation makes some sense, but if you have more questions, just ask!)

clang/test/Headers/stdckdint.cpp

2

The user-facing parts of those changes are at https://reviews.llvm.org/D157606

[ZijunZhao]

In D157331#4576540, @aaron.ballman wrote:

In D157331#4575224, @ZijunZhao wrote:

Another followup question: I check https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2683.pdf and I only add Core Proposal here. Do I need to add Supplemental Proposal, like some types?

Ah, this is a bit confusing! tl;dr: No need to add the supplemental proposal.

The way to trace this down yourself is:

• The working draft (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf) has a list of what papers were adopted and applied to the standard. You'll find N2683 under the June 2021 virtual meeting heading. That's how you know which paper was adopted.
• You can look at the paper to see the proposed wording, prior art, motivation, etc. That gets you 95% of the way but you need to know what words actually went into the standard. So use the original paper to give you ideas on what to implement, what to test, etc, but verify against the wording in the standard.
• Because this was discussed at the June 2021 virtual meeting, you should look at those meeting minutes (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2802.pdf) to see what was adopted. You can search for 2683 to find the relevant discussion in the minutes. Most importantly, you can see what polls were taken. The two polls of interest were:

Straw Poll: Does the committee wish to adopt the Core proposal from N2683 into C23? 13-0-5 Core proposal goes into C23.
Straw Poll: Does the committee want the "ckd_" identifiers from N2683's Core proposal in future library directions to be potentially reserved identifiers? 18-0-0

This is how we know that only the core proposal was added, and not the supplemental proposal. Checking that belief against the standard wording itself verifies that only the core proposal was added and the supplemental bits are left out.

(Hopefully that explanation makes some sense, but if you have more questions, just ask!)

Yes, that's very helpful! Thank you!

[ZijunZhao]

1. rename to .c file
2. create clang/test/C/C2x/n2683.c and add codegen tests and semantic tests
3. update clang/docs/ReleseNotes.rst
4. update clang/www/c_status.html
5. reformat PPDirectives.cpp
6. set STDC_VERSION instead of gnu

[enh]

clang/lib/Headers/stdckdint.h

11

i think this should just be __STDCKDINT_H to match the other headers' include guards, and then you want a _separate_ #define that defines __STDC_VERSION_STDCKDINT_H__ (and defines it to the value you're claiming to define it to in the release notes, but don't seem to actually be defining it to here :-) ).

(and maybe add a test that the macro is defined with a value to the tests?)

19

i think this #else should be deleted now? (it will give a misleading error if you build with pre-c23.)

[ZijunZhao]

1. separate two files in clang/test/C/C2x
2. make some update about the macro STDC_VERSION_STDCKDINT_H add the test for testing it

[ZijunZhao]

Skip static_assert() tests because constexpr is not supported in C23 yet: https://github.com/llvm/llvm-project/issues/64742

[enh]

clang/test/C/C2x/n2359.c

40 ↗

(On Diff #550910)

don't you need another test somewhere that this _is_ defined under some circumstances? (and a definition in the header itself!)

[ZijunZhao]

clang/test/C/C2x/n2359.c

40 ↗

(On Diff #550910)

I follow the cases like __STDC_VERSION_LIMITS_H__ and __STDC_VERSION_STDATOMIC_H__ . They are not defined in the <limits.h> or <stdatomic.h>.

[ZijunZhao]

Reformat clang/lib/Lex/PPDirectives.cpp. I use git-clang-format due to previous pre-check failure but lots of modifications in clang/lib/Lex/ directory. Should I keep running git-clang-format if the pre-check fails again?

[aaron.ballman]

clang/docs/ReleaseNotes.rst

113–115 ↗

(On Diff #551607)

No need to mention the version macro in this case (I did it above because __STDC_VERSION__ is a commonly used and previously had a placeholder value instead of a final value).

clang/lib/Headers/stdckdint.h

1

The formatting for this is still off (it line wraps here and on line 7-8 -- it should be shortened so that the closing comment fits within the 80 col limit).

13

Should a hosted build attempt to do an include_next into the system library and then fall back to the compiler builtins, or should we treat this like stdbool.h where the compiler always wins? CC @jyknight @jrtc27 @efriedma

My intuition is that we want to include_next in case the system has better facilities than the compiler does.

15

You should also have a macro definition:

#define __STDC_VERSION_STDCKDINT_H__ 202311L

I've not yet added the __STDC_VERSION_* macros to the other headers we provide because those require some further testing to determine if we conform to C23 or not. However, because this is a brand new header being implemented, we're doing that conformance testing now, so we should go ahead and implement the version macro now.

clang/lib/Lex/PPDirectives.cpp

210–212

I was thinking of something more along these lines.

clang/test/C/C2x/n2359.c

40 ↗

(On Diff #550910)

I commented on that a bit above.

clang/test/C/C2x/n2683.c

4 ↗

(On Diff #551607)

The first line of the comment is the paper number and whether we support it or not, and the second line is the title of the paper, which is why there's less information here now. :-)

16 ↗

(On Diff #551607)

It looks like the builtins are missing some checks that are required by the C standard.

7.20p3: Both type2 and type3 shall be any integer type other than "plain" char, bool, a bit-precise integer type, or an enumerated type, and they need not be the same. ...

So we should get a (warning?) diagnostic on all of these uses.

We should also add a test when the result type is not suitable for the given operand types. e.g.,

void func(int one, int two) {
  short result;
  ckd_add(&result, one, two); // `short` may not be suitable to hold the result of adding two `int`s

  const int other_result = 0;
  ckd_add(&other_result, one, two); // `const int` is definitely not suitable because it's not a modifiable lvalue
}

This is because of:

7.20.1p4: It is recommended to produce a diagnostic message if type2 or type3 are not suitable integer types, or if *result is not a modifiable lvalue of a suitable integer type.

19 ↗

(On Diff #551607)

Also, can you change the test file to use two spaces for indentation instead of four? Same in the other test file.

clang/test/C/C2x/n2683_2.c

1–2 ↗

(On Diff #551607)

This kind of comment is used when passing -verify on the RUN line and the test has no diagnostics. Since we're not passing -verify, there's no need for the comment.

clang/test/Headers/stdckdint.c

1–2 ↗

(On Diff #551607)

I'm assuming that -fgnuc-version=4.2.1 is not needed for the test case?

[ZijunZhao]

1. Reformat test files in C/C2x
2. Update the definition and the test about __STDC_VERSION_STDCKDINT_H__
3. Update release docs and the introduction in the test file
4. Update RUN command in the test files
5. Update PPDirectives.cpp

[ZijunZhao]

clang/test/C/C2x/n2683.c

16 ↗

(On Diff #551607)

yes, I am trying to add the tests about short type and const variable but there is no warning about inappropriate type 😢 and for const one I get `result argument to overflow builtin must be a pointer to a non-const integer ('const int *' invalid)` error 😂

[ZijunZhao]

clang/lib/Headers/stdckdint.h

1

oh yes I set 80 col limit and they don't exceed the line.

I attach the screenshort and hope it help explain. I think I might misunderstand something?

[aaron.ballman]

clang/lib/Headers/stdckdint.h

1

Oh! I was thrown off by the C-style commenting, but you're right, this is correct as-is (and matches how the other C headers look), sorry for the noise!

14

I think this should only be defined when the function-like macros are being defined, so this should move down into the #if block below. That way, people can do:

#if __STDC_VERSION_STDCKDINT_H__  >= 202311L

to know they've got access to the macros. Which is weird because they could also do #ifdef ckd_add, but oh well, C is weird.

clang/test/C/C2x/n2683.c

16 ↗

(On Diff #551607)

I think the diagnostic with a const result is reasonable enough (unfortunate it talks about builtins but we can correct the wording later). The other type checking is something that will need to be added to: https://github.com/llvm/llvm-project/blob/8f6a1a07cb85980013c70d5af6d28f5fcf75e732/clang/lib/Sema/SemaChecking.cpp#L368

One thing you'll have to do is determine whether the builtin is being used via the standard interface or whether it's being used directly by the user. We don't want to change the behavior of the call through __builtin_add_overflow because that can break existing code. So you'll need to see if the call expression is a macro expansion from a macro named ckd_add (for example) and perform the extra checking only in that case.

[ZijunZhao]

1. define STDC_VERSION_STDCKDINT_H
2. check short type

[ZijunZhao]

Rename the warn_overflow_builtin_can_not_be_short for precision

[ZijunZhao]

Prevent result type from being short and fix the breaking in builtinoverflow.cpp

[ZijunZhao]

Add integer type test and update c2x tests.

[aaron.ballman]

I have some suggested changes for the way we're checking and diagnosing unsuitable types; I've not tried my suggestions out though, so if you run into problems, please let me know.

clang/include/clang/AST/Type.h

2159 ↗

(On Diff #553639)

I'd rather put this logic into SemaChecking.cpp than expose it in Type.h. This isn't a type predicate most other code should need to use.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8616–8625 ↗

(On Diff #553639)

8624–8625 ↗

(On Diff #553639)

There's a few things that aren't correct here, so I'm suggesting a different approach, but for your own understanding:

• All warning diagnostics need to be added to a warning group, using InGroup<>.
• The warning text is very specific to only one problematic situation; it should be generalized to cover as many code patterns as it can.
• We can likely reuse the existing error (err_overflow_builtin_must_be_int) instead of making a new one.

clang/lib/Sema/SemaChecking.cpp

388 ↗

(On Diff #553639)

396–401 ↗

(On Diff #553639)

414–431 ↗

(On Diff #553639)

[ZijunZhao]

1. Remove pure integer check to the right place
2. Update error msg and all related tests
3. Add explanations to header file

[ZijunZhao]

clang/lib/Sema/SemaChecking.cpp

388 ↗

(On Diff #553639)

I keeps my original code because it just checks once but this checks many times and in case BuiltinType::WChar_U checking is missing(I know isWideCharType() can be added but I still think check the type is between Short and Int128, and UShort and UInt128 will be quicker and a bit safer?).

414–431 ↗

(On Diff #553639)

I don't think the else if part should be removed. We should make sure the result type is not short type. In ValidCkdIntType() checking, short type is a valid type.

[ZijunZhao]

Remove the short type check part.
From Aaron Ballman:

Instead, we'd want to do some data flow analysis (in the clang static analyzer) so we know the potential range of values of each of the operands and can diagnose the result type from there if we can prove there's a potential for overflow due to type confusion.

And I agree.

[pirama]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8618–8619 ↗

(On Diff #554784)

The warning selector is slightly different from the one for the result.

clang/lib/Headers/stdckdint.h

13

The include_next question is still open. Any preference here?

IMO, since the standard explicitly delegates to the compiler builtin when available, we may not need to include_next - unless there are other conventions around this.

[jrtc27]

clang/lib/Headers/stdckdint.h

13

FreeBSD has just added an implementation because Clang doesn’t have one. GCC will include_next if _LIBC_STDCKDINT_H isn’t defined and there is a header to include. I don’t really know why that exists to be honest, I don’t see a use for it and view this like foointrin.h headers. But probably we should be compatible with that. I don’t see a use case for it in FreeBSD though.

[enh]

(yeah, similar for Android --- we'll just use the clang one as-is. but if there's already precedent for allowing include_next, that doesn't hurt us either.)

[ZijunZhao]

Reformat the error msg

[ZijunZhao]

update the reformat msg and tests

[ZijunZhao]

add #include_next

[ZijunZhao]

update comments

[ZijunZhao]

Remove c++ related conditions

[pirama]

@aaron.ballman do you have additional comments or does this patch looks good to merge?

[erichkeane]

In D157331#4648417, @pirama wrote:

@aaron.ballman do you have additional comments or does this patch looks good to merge?

Aaron is away this week at a conference in Norway (see his Discourse announcement), so you're not likely to see a response until next week.

[aaron.ballman]

clang/lib/Sema/SemaChecking.cpp

373–387 ↗

(On Diff #556504)

How about something like this to avoid the code duplication?

393–396 ↗

(On Diff #556504)

I think this should get us what we need.

I think wchar_t behavior is fine; in C++, that's a builtin datatype and should not be used with these APIs and in C, that's a typedef to some integer type and is allowed.

407–408 ↗

(On Diff #556504)

Minor naming nit.

388 ↗

(On Diff #553639)

Yeah, I wasn't happy with how many extra checks my variant would come up with, but I am presuming the optimizer is able to combine them into something reasonable. I was more concerned about readability of the code, but I think we can fix yours up and it's still sufficiently readable.

414–431 ↗

(On Diff #553639)

I think the else if should be removed; there's no reason the result type cannot be short. e.g.,

short s1 = 1, s2 = 1, s3;
ckd_add(&s3, s1, s2);

Yes, s1 and s2 are promoted to int, but 1 + 1 can be represented in short just fine.

[ZijunZhao]

1. Simplify code in SemaChecking.cpp
2. Fix the nit and rename the variable in SemaChecking.cpp
3. Update the condition in SemaChecking.cpp
4. Add wchar_t test to show our care

[aaron.ballman]

Precommit CI seems to be finding issues with your changes:

[3278/3832] Building CXX object tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o
FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o
ccache /usr/bin/c++ -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/build/build-clang/tools/clang/lib/Sema -I/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/clang/lib/Sema -I/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/clang/include -I/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/build/build-clang/tools/clang/include -I/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/build/build-clang/include -I/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -std=c++17 -MD -MT tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o -MF tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o.d -o tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaChecking.cpp.o -c /var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/clang/lib/Sema/SemaChecking.cpp
/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/clang/lib/Sema/SemaChecking.cpp: In lambda function:
/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/clang/lib/Sema/SemaChecking.cpp:380:64: error: no matching function for call to 'clang::Lexer::getImmediateMacroName(bool)'
  380 |     return BuiltinID == P.first && Lexer::getImmediateMacroName(
      |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
  381 |       TheCall->getExprLoc().isMacroID() && Lexer::getImmediateMacroName(
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  382 |         TheCall->getExprLoc(), S.getSourceManager(), S.getLangOpts()) == P.second
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  383 |     );
      |     ~
In file included from /var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/clang/lib/Sema/SemaChecking.cpp:56:
/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/clang/include/clang/Lex/Lexer.h:509:20: note: candidate: 'static llvm::StringRef clang::Lexer::getImmediateMacroName(clang::SourceLocation, const clang::SourceManager&, const clang::LangOptions&)'
  509 |   static StringRef getImmediateMacroName(SourceLocation Loc,
      |                    ^~~~~~~~~~~~~~~~~~~~~
/var/lib/buildkite-agent/builds/linux-56-7f758798dd-khkmx-1/llvm-project/clang-ci/clang/include/clang/Lex/Lexer.h:509:20: note:   candidate expects 3 arguments, 1 provided

[ZijunZhao]

Update the SemaChecking.cpp and the test.

[ZijunZhao]

Rebase to latest main branch

[aaron.ballman]

Precommit CI is still finding some issues that boil down to:

# .---command stderr------------
# | C:\ws\src\clang\test\Modules/Inputs/System/usr/include\module.map:19:12: error: header 'stdckdint.h' not found
# |    19 |     header "stdckdint.h"
# |       |            ^
# | C:\ws\src\clang\test\Modules/Inputs/System/usr/include\uses_other_constants.h:2:10: note: submodule of top-level module 'cstd' implicitly imported here
# |     2 | #include <float.h>
# |       |          ^
# | C:\ws\src\clang\test\Modules\cstd.m:4:9: fatal error: could not build module 'uses_other_constants'
# |     4 | @import uses_other_constants;
# |       |  ~~~~~~~^~~~~~~~~~~~~~~~~~~~
# | C:\ws\src\clang\test\Modules/Inputs/System/usr/include\module.map:19:12: error: header 'stdckdint.h' not found
# |    19 |     header "stdckdint.h"
# |       |            ^
# | 3 errors generated.
# `-----------------------------
# error: command failed with exit status: 1

I commented on what I think needs to change to fix the issues.

clang/test/Modules/Inputs/System/usr/include/module.map

17–21

I think these changes should be removed, there is no stdckdint.h file in test/Modules/Inputs/System/usr/include.

[ZijunZhao]

Remove stdckdint.h in module.map

[aaron.ballman]

clang/test/C/C2x/n2683_2.c

7–15 ↗

(On Diff #557589)

Sorry for not spotting this sooner, but when emitting LLVM IR, these %ident identifiers are sometimes replaced with different text depending on how the bot is set up. e.g., one bot may use %result64 while another bot may decide to name it %0. The way we usually handle this is to use a feature from FileCheck that lets you name a regex pattern and then use that. I showed some examples of how to capture the name and how to use the name as a replacement above, but I'll leave it to you to do the rest of the changes. You should do this for anything starting with % in this file and in clang/test/Headers/stdckdint.c.

[jrtc27]

clang/test/C/C2x/n2683_2.c

7–15 ↗

(On Diff #557589)

Or just use update_cc_test_checks.py and let it do that for you? These tests don't look like they do anything weird that the UTC scripts won't like.

[aaron.ballman]

clang/test/C/C2x/n2683_2.c

7–15 ↗

(On Diff #557589)

WE HAVE A TOOL FOR THIS?!?!

I had no idea. 😳

Do we have any docs for this? CC @erichkeane for awareness as well.

[ZijunZhao]

Name a regex pattern and then use that to avoid post-commit CI breakage.

[xbolva00]

Please use update_cc_test_checks.py

[jrtc27]

clang/test/C/C2x/n2683_2.c

14 ↗

(On Diff #557618)

You missed this one. But please just use the script, it makes everyone's lives easier, both in the present (no need to worry that you've forgotten anything) and in the future (should Clang's code generation change it's much easier to rerun the script rather than hand-edit every test; which also includes downstreams that alter behaviour, which I am very familiar with thanks to my day job).

[ZijunZhao]

Run the script to update the test

Test command:
llvm/utils/update_cc_test_checks.py --clang build/bin/clang ./clang/test/C/C2x/n2683_2.c
llvm/utils/update_cc_test_checks.py --clang build/bin/clang ./clang/test/Headers/stdckdint.c

[jrtc27]

One more thought: we need to specify a triple for the tests as otherwise it'll use whatever default you have configured in LLVM, which will affect the exact code generated (especially wrt argument and return lowering), no?

[aaron.ballman]

In D157331#4653222, @jrtc27 wrote:

One more thought: we need to specify a triple for the tests as otherwise it'll use whatever default you have configured in LLVM, which will affect the exact code generated (especially wrt argument and return lowering), no?

Now that we're using the script (which generates a lot more CHECK lines), I think we will have to add a triple, good catch. x86_64-unknown-unknown seems like it would be a reasonable triple (but I defer to @jrtc27 if she's got other ideas).

[jrtc27]

In D157331#4653224, @aaron.ballman wrote:

In D157331#4653222, @jrtc27 wrote:

One more thought: we need to specify a triple for the tests as otherwise it'll use whatever default you have configured in LLVM, which will affect the exact code generated (especially wrt argument and return lowering), no?

Now that we're using the script (which generates a lot more CHECK lines), I think we will have to add a triple, good catch. x86_64-unknown-unknown seems like it would be a reasonable triple (but I defer to @jrtc27 if she's got other ideas).

Yeah, generally "pick your favourite x86_64 triple". In RISC-V land we generally go for a bare riscv64 as the triple for anything that's not OS-specific and just pick up whatever the bare-metal ABI is (normally close to whatever Linux and BSDs do), but historically x86_64 seems to be a bit more of a mix of everything (bare, linux-gnu, unknown-unknown, something apple-y). I'd lean towards just x86_64, but it doesn't hugely matter here.

[ZijunZhao]

Add --triple=x86_64 into the tests.

[jrtc27]

clang/test/C/C2x/n2683_2.c

5–6 ↗

(On Diff #557621)

UTC won't insert such a blank line if it doesn't exist in the input

[ZijunZhao]

1. set --triple=x86-64 as the first argument
2. run the script for the tests

[aaron.ballman]

LGTM! Please wait for @jrtc27 to also sign off before landing.

[jrtc27]

clang/test/C/C2x/n2683_2.c

5–6 ↗

(On Diff #557621)

Not done despite being marked done

clang/test/Headers/stdckdint.c

3 ↗

(On Diff #557622)

[ZijunZhao]

Add the blank lines back and then rerun the utc script

[jrtc27]

Thanks, my concerns in the tests have been addressed

[mstorsjo]

This change broke building a recent version of gettext. Gettext uses gnulib for polyfilling various utility functions. Since not long ago, these functions internally use <stdckdint.h>, https://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/malloca.c?id=ef5a4088d9236a55283d1eb576f560aa39c09e6f. If the toolchain doesn't provide the header, gnulib provides a fallback version of its own: https://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/stdckdint.in.h

Now since Clang provides it since this commit, gnulib doesn't provide their own fallback, but goes on to use ckd_add. This fails with Clang's <stdckdint.h> implementation, since gnulib isn't built in C23 mode but still wants to use <stdckdint.h>:

i686-w64-mingw32-clang -DHAVE_CONFIG_H -DEXEEXT=\".exe\" -I. -I../../../gettext-runtime/gnulib-lib -I..  -I../intl -I../../../gettext-runtime/intl -DDEPENDS_ON_LIBICONV=1 -DDEPENDS_ON_LIBINTL=1 -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include  -Wno-cast-qual -Wno-conversion -Wno-float-equal -Wno-sign-compare -Wno-undef -Wno-unused-function -Wno-unused-parameter -Wno-float-conversion -Wimplicit-fallthrough -Wno-pedantic -Wno-sign-conversion -Wno-type-limits -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include -g -O2 -c -o libgrt_a-malloca.o `test -f 'malloca.c' || echo '../../../gettext-runtime/gnulib-lib/'`malloca.c
../../../gettext-runtime/gnulib-lib/malloca.c:53:8: error: call to undeclared function 'ckd_add'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
   53 |   if (!ckd_add (&nplus, n, plus) && !xalloc_oversized (nplus, 1))
      |        ^
1 error generated.
make: *** [Makefile:2246: libgrt_a-malloca.o] Error 1

It seems like GCC's version of this header exposes the functionality even if compiled in an older language mode: https://github.com/gcc-mirror/gcc/blob/f019251ac9be017aaf3c58f87f43d88b974d21cf/gcc/ginclude/stdckdint.h

Would you consider changing the Clang version to do the same? Otherwise we would need to convince gnulib to not try to use/polyfill this header unless gnulib itself is compiled in C23 mode.

[aaron.ballman]

In D157331#4654265, @mstorsjo wrote:

This change broke building a recent version of gettext. Gettext uses gnulib for polyfilling various utility functions. Since not long ago, these functions internally use <stdckdint.h>, https://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/malloca.c?id=ef5a4088d9236a55283d1eb576f560aa39c09e6f. If the toolchain doesn't provide the header, gnulib provides a fallback version of its own: https://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/stdckdint.in.h

Now since Clang provides it since this commit, gnulib doesn't provide their own fallback, but goes on to use ckd_add. This fails with Clang's <stdckdint.h> implementation, since gnulib isn't built in C23 mode but still wants to use <stdckdint.h>:

i686-w64-mingw32-clang -DHAVE_CONFIG_H -DEXEEXT=\".exe\" -I. -I../../../gettext-runtime/gnulib-lib -I..  -I../intl -I../../../gettext-runtime/intl -DDEPENDS_ON_LIBICONV=1 -DDEPENDS_ON_LIBINTL=1 -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include  -Wno-cast-qual -Wno-conversion -Wno-float-equal -Wno-sign-compare -Wno-undef -Wno-unused-function -Wno-unused-parameter -Wno-float-conversion -Wimplicit-fallthrough -Wno-pedantic -Wno-sign-conversion -Wno-type-limits -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include -g -O2 -c -o libgrt_a-malloca.o `test -f 'malloca.c' || echo '../../../gettext-runtime/gnulib-lib/'`malloca.c
../../../gettext-runtime/gnulib-lib/malloca.c:53:8: error: call to undeclared function 'ckd_add'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
   53 |   if (!ckd_add (&nplus, n, plus) && !xalloc_oversized (nplus, 1))
      |        ^
1 error generated.
make: *** [Makefile:2246: libgrt_a-malloca.o] Error 1

It seems like GCC's version of this header exposes the functionality even if compiled in an older language mode: https://github.com/gcc-mirror/gcc/blob/f019251ac9be017aaf3c58f87f43d88b974d21cf/gcc/ginclude/stdckdint.h

Would you consider changing the Clang version to do the same? Otherwise we would need to convince gnulib to not try to use/polyfill this header unless gnulib itself is compiled in C23 mode.

Well that's a fun situation. The identifiers exposed by the header are not in a reserved namespace and they're macros, which is usually not something we'd want to expose in older language modes because of the risk of breaking code. But in this case, there's an explicit opt-in to a brand-new header file so it shouldn't break code to expose the contents in older language modes. But I expect there to be changes to this file in C2y, and those additions could break code if we're not careful.

CC @jrtc27 @jyknight @efriedma for opinions from others, but my thinking is that we should expose the contents in earlier language modes. We support the builtin in those modes, and the user is explicitly asking for the header to be included -- it seems pretty rare for folks to want the header to be empty, especially given that __has_include will report "sure do!". I don't think there's a conformance issue here.

[jrtc27]

In D157331#4654353, @aaron.ballman wrote:

In D157331#4654265, @mstorsjo wrote:

This change broke building a recent version of gettext. Gettext uses gnulib for polyfilling various utility functions. Since not long ago, these functions internally use <stdckdint.h>, https://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/malloca.c?id=ef5a4088d9236a55283d1eb576f560aa39c09e6f. If the toolchain doesn't provide the header, gnulib provides a fallback version of its own: https://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/stdckdint.in.h

Now since Clang provides it since this commit, gnulib doesn't provide their own fallback, but goes on to use ckd_add. This fails with Clang's <stdckdint.h> implementation, since gnulib isn't built in C23 mode but still wants to use <stdckdint.h>:

i686-w64-mingw32-clang -DHAVE_CONFIG_H -DEXEEXT=\".exe\" -I. -I../../../gettext-runtime/gnulib-lib -I..  -I../intl -I../../../gettext-runtime/intl -DDEPENDS_ON_LIBICONV=1 -DDEPENDS_ON_LIBINTL=1 -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include  -Wno-cast-qual -Wno-conversion -Wno-float-equal -Wno-sign-compare -Wno-undef -Wno-unused-function -Wno-unused-parameter -Wno-float-conversion -Wimplicit-fallthrough -Wno-pedantic -Wno-sign-conversion -Wno-type-limits -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include -g -O2 -c -o libgrt_a-malloca.o `test -f 'malloca.c' || echo '../../../gettext-runtime/gnulib-lib/'`malloca.c
../../../gettext-runtime/gnulib-lib/malloca.c:53:8: error: call to undeclared function 'ckd_add'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
   53 |   if (!ckd_add (&nplus, n, plus) && !xalloc_oversized (nplus, 1))
      |        ^
1 error generated.
make: *** [Makefile:2246: libgrt_a-malloca.o] Error 1

It seems like GCC's version of this header exposes the functionality even if compiled in an older language mode: https://github.com/gcc-mirror/gcc/blob/f019251ac9be017aaf3c58f87f43d88b974d21cf/gcc/ginclude/stdckdint.h

Would you consider changing the Clang version to do the same? Otherwise we would need to convince gnulib to not try to use/polyfill this header unless gnulib itself is compiled in C23 mode.

Well that's a fun situation. The identifiers exposed by the header are not in a reserved namespace and they're macros, which is usually not something we'd want to expose in older language modes because of the risk of breaking code. But in this case, there's an explicit opt-in to a brand-new header file so it shouldn't break code to expose the contents in older language modes. But I expect there to be changes to this file in C2y, and those additions could break code if we're not careful.

CC @jrtc27 @jyknight @efriedma for opinions from others, but my thinking is that we should expose the contents in earlier language modes. We support the builtin in those modes, and the user is explicitly asking for the header to be included -- it seems pretty rare for folks to want the header to be empty, especially given that __has_include will report "sure do!". I don't think there's a conformance issue here.

Yes. That's also what we do for stdalign.h, which is the same case.

[aaron.ballman]

In D157331#4654362, @jrtc27 wrote:

In D157331#4654353, @aaron.ballman wrote:

In D157331#4654265, @mstorsjo wrote:

This change broke building a recent version of gettext. Gettext uses gnulib for polyfilling various utility functions. Since not long ago, these functions internally use <stdckdint.h>, https://git.savannah.gnu.org/cgit/gnulib.git/commit/lib/malloca.c?id=ef5a4088d9236a55283d1eb576f560aa39c09e6f. If the toolchain doesn't provide the header, gnulib provides a fallback version of its own: https://git.savannah.gnu.org/cgit/gnulib.git/tree/lib/stdckdint.in.h

Now since Clang provides it since this commit, gnulib doesn't provide their own fallback, but goes on to use ckd_add. This fails with Clang's <stdckdint.h> implementation, since gnulib isn't built in C23 mode but still wants to use <stdckdint.h>:

i686-w64-mingw32-clang -DHAVE_CONFIG_H -DEXEEXT=\".exe\" -I. -I../../../gettext-runtime/gnulib-lib -I..  -I../intl -I../../../gettext-runtime/intl -DDEPENDS_ON_LIBICONV=1 -DDEPENDS_ON_LIBINTL=1 -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include  -Wno-cast-qual -Wno-conversion -Wno-float-equal -Wno-sign-compare -Wno-undef -Wno-unused-function -Wno-unused-parameter -Wno-float-conversion -Wimplicit-fallthrough -Wno-pedantic -Wno-sign-conversion -Wno-type-limits -I/home/martin/fate/vlc/src/contrib/i686-w64-mingw32/include -g -O2 -c -o libgrt_a-malloca.o `test -f 'malloca.c' || echo '../../../gettext-runtime/gnulib-lib/'`malloca.c
../../../gettext-runtime/gnulib-lib/malloca.c:53:8: error: call to undeclared function 'ckd_add'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
   53 |   if (!ckd_add (&nplus, n, plus) && !xalloc_oversized (nplus, 1))
      |        ^
1 error generated.
make: *** [Makefile:2246: libgrt_a-malloca.o] Error 1

It seems like GCC's version of this header exposes the functionality even if compiled in an older language mode: https://github.com/gcc-mirror/gcc/blob/f019251ac9be017aaf3c58f87f43d88b974d21cf/gcc/ginclude/stdckdint.h

Would you consider changing the Clang version to do the same? Otherwise we would need to convince gnulib to not try to use/polyfill this header unless gnulib itself is compiled in C23 mode.

Well that's a fun situation. The identifiers exposed by the header are not in a reserved namespace and they're macros, which is usually not something we'd want to expose in older language modes because of the risk of breaking code. But in this case, there's an explicit opt-in to a brand-new header file so it shouldn't break code to expose the contents in older language modes. But I expect there to be changes to this file in C2y, and those additions could break code if we're not careful.

CC @jrtc27 @jyknight @efriedma for opinions from others, but my thinking is that we should expose the contents in earlier language modes. We support the builtin in those modes, and the user is explicitly asking for the header to be included -- it seems pretty rare for folks to want the header to be empty, especially given that __has_include will report "sure do!". I don't think there's a conformance issue here.

Yes. That's also what we do for stdalign.h, which is the same case.

It does, but I'd be surprised if stdalign.h got new interfaces added to it, so it's a bit different. But even still, I think we should go ahead and expose these facilities in pre-C23 modes. Thanks for the second opinion!

clang/lib/Headers/CMakeLists.txt

 # core_files list contains the headers shared by all platforms.
 # Please consider adding new platform specific headers
 # to platform specific lists below.
 set(core_files
   builtins.h
   float.h
   inttypes.h
   iso646.h
   limits.h
   module.modulemap
   stdalign.h
   stdarg.h
   stdatomic.h
   stdbool.h
+  stdckdint.h
   stddef.h
   __stddef_max_align_t.h
   stdint.h
   stdnoreturn.h
   tgmath.h
   unwind.h
   varargs.h
   )

clang/lib/Headers/stdckdint.h

+/*===---- stdckdint.h - Standard header for checking integer
+ *-------------------------===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===-----------------------------------------------------------------------===
+ */
+
+#ifndef __STDCKDINT_H
+#define __STDCKDINT_H
+
+#if defined(__GNUC__)
+#define ckd_add(R, A, B) __builtin_add_overflow((A), (B), (R))
+#define ckd_sub(R, A, B) __builtin_sub_overflow((A), (B), (R))
+#define ckd_mul(R, A, B) __builtin_mul_overflow((A), (B), (R))
+#else
+#error "we need a compiler extension for this"
+#endif
+
+#endif /* __STDCKDINT_H */

clang/lib/Lex/ModuleMap.cpp

bool ModuleMap::isBuiltinHeader(StringRef FileName) {
   return llvm::StringSwitch<bool>(FileName)
            .Case("float.h", true)
            .Case("iso646.h", true)
            .Case("limits.h", true)
            .Case("stdalign.h", true)
            .Case("stdarg.h", true)
            .Case("stdatomic.h", true)
            .Case("stdbool.h", true)
+           .Case("stdckdint.h", true)
            .Case("stddef.h", true)
            .Case("stdint.h", true)
            .Case("tgmath.h", true)
            .Case("unwind.h", true)
            .Default(false);
 }
 
 bool ModuleMap::isBuiltinHeader(const FileEntry *File) {

clang/lib/Lex/PPDirectives.cpp

static bool warnByDefaultOnWrongCase(StringRef Include) {
   }
 
   // The standard C/C++ and Posix headers
   return llvm::StringSwitch<bool>(LowerInclude)
     // C library headers
     .Cases("assert.h", "complex.h", "ctype.h", "errno.h", "fenv.h", true)
     .Cases("float.h", "inttypes.h", "iso646.h", "limits.h", "locale.h", true)
     .Cases("math.h", "setjmp.h", "signal.h", "stdalign.h", "stdarg.h", true)
-    .Cases("stdatomic.h", "stdbool.h", "stddef.h", "stdint.h", "stdio.h", true)
+    .Cases("stdatomic.h", "stdbool.h", "stdckdint.h", "stddef.h", "stdint.h", "stdio.h", true)
     .Cases("stdlib.h", "stdnoreturn.h", "string.h", "tgmath.h", "threads.h", true)
     .Cases("time.h", "uchar.h", "wchar.h", "wctype.h", true)
 
     // C++ headers for C library facilities
     .Cases("cassert", "ccomplex", "cctype", "cerrno", "cfenv", true)
     .Cases("cfloat", "cinttypes", "ciso646", "climits", "clocale", true)
     .Cases("cmath", "csetjmp", "csignal", "cstdalign", "cstdarg", true)
     .Cases("cstdbool", "cstddef", "cstdint", "cstdio", "cstdlib", true)
     .Cases("cstring", "ctgmath", "ctime", "cuchar", "cwchar", true)
     .Case("cwctype", true)

clang/test/Headers/stdckdint.cpp

+// RUN: %clang_cc1 -emit-llvm  -fgnuc-version=4.2.1 -std=gnu++11 %s -o - | FileCheck --check-prefix=CHECK-NEXT %s
+
+
+#include <stdckdint.h>
+// CHECK-LABEL: define dso_local zeroext i1 @test_ckd_add() #0 {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    %0 = call { i32, i1 } @llvm.sadd.with.overflow.i32(i32 -1073741826, i32 -1073741826)
+// CHECK-NEXT:    %1 = extractvalue { i32, i1 } %0, 1 
+// CHECK-NEXT:    store i32 %2, ptr %result, align 4 
+// CHECK-NEXT:    ret i1 %1 
+// CHECK-NEXT:    } 
+// CHECK-NEXT:    ; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none) 
+// CHECK-NEXT:    declare { i32, i1 } @llvm.sadd.with.overflow.i32(i32, i32) #1 
+bool test_ckd_add() {
+  int result;
+  return ckd_add(&result, -1073741826, -1073741826);
+}
+
+// CHECK-LABEL: define dso_local zeroext i1 @test_ckd_sub() #0 {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    %0 = call { i32, i1 } @llvm.ssub.with.overflow.i32(i32 -1073741826, i32 1073741826)
+// CHECK-NEXT:    %1 = extractvalue { i32, i1 } %0, 1 
+// CHECK-NEXT:    store i32 %2, ptr %result, align 4 
+// CHECK-NEXT:    ret i1 %1 
+// CHECK-NEXT:    } 
+// CHECK-NEXT:    ; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none) 
+// CHECK-NEXT:    declare { i32, i1 } @llvm.ssub.with.overflow.i32(i32, i32) #1 
+bool test_ckd_sub() {
+  int result;
+  return ckd_sub(&result, -1073741826, 1073741826);
+}
+
+// CHECK-LABEL: define dso_local zeroext i1 @test_ckd_mul() #0 {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    %0 = call { i32, i1 } @llvm.smul.with.overflow.i32(i32 -1073741826, i32 2)
+// CHECK-NEXT:    %1 = extractvalue { i32, i1 } %0, 1 
+// CHECK-NEXT:    store i32 %2, ptr %result, align 4
+// CHECK-NEXT:    ret i1 %1 
+// CHECK-NEXT:    } 
+// CHECK-NEXT:    ; Function Attrs: nocallback nofree nosync nounwind speculatable willreturn memory(none) 
+// CHECK-NEXT:    declare { i32, i1 } @llvm.smul.with.overflow.i32(i32, i32) #1 
+bool test_ckd_mul() {
+  int result;
+  return ckd_mul(&result, -1073741826, 2);
+}

clang/test/Modules/Inputs/System/usr/include/module.map

 module cstd [system] {
   // Only in compiler support directory
   module float_constants {
     header "float.h"
   }
 
   // Only in system headers directory
   module stdio {
     header "stdio.h"
   }
 
   // In both directories (compiler support version wins, does not forward)
   module stdbool {
     header "stdbool.h"
   }
 
+  // In both directories (compiler support version wins, does not forward)
+  module stdckdint {
+    header "stdckdint.h"
+  }
+
   // In both directories (compiler support version wins, forwards)
   module stdint {
     header "stdint.h"
   }
 }
 
 module other_constants {
   explicit module dbl_max {