This is an archive of the discontinued LLVM Phabricator instance.

[C2x] Implement support for nullptr and nullptr_t
ClosedPublic

Authored by aaron.ballman on Oct 3 2022, 1:27 PM.

Details

Summary

This introduces support for nullptr and nullptr_t in C2x mode. The proposal accepted by WG14 is: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3042.htm

Note, there are quite a few incompatibilities with the C++ feature in some of the edge cases of this feature. Therefore, there are some FIXME comments in tests for testing behavior that might change after WG14 has resolved national body comments (a process we've not yet started). So this implementation might change slightly depending on the resolution of comments. This is called out explicitly in the release notes as well.

Diff Detail

Event Timeline

aaron.ballman created this revision.Oct 3 2022, 1:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2022, 1:27 PM
aaron.ballman requested review of this revision.Oct 3 2022, 1:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 3 2022, 1:27 PM
erichkeane added inline comments.
clang/include/clang/AST/PrettyPrinter.h
68–69

Does Nullptr here not have to change? Shouldn't we care to use the "nullptr" in C mode instead of '0' now?

201

Is there a reason this isn't a bitfield as well?

clang/include/clang/Basic/TokenKinds.def
393

Its a shame we have to do this... the CXX11_KEYWORD (and presumably the reverse) is used for 'future' diagnostics IIRC. So not having this as a C2X_KEYWORD means we lose the 'future keyword' diagnostics, right?

clang/lib/Sema/SemaExpr.cpp
8730

what does this do with the GNU ternary-thing?

shafik added a subscriber: shafik.Oct 3 2022, 2:37 PM
shafik added inline comments.
clang/lib/Sema/SemaCast.cpp
2999

Curious why put the comment after? When we use butprone-argument-comment for function parameters we put them before.

shafik added inline comments.Oct 3 2022, 5:16 PM
clang/lib/Sema/SemaExpr.cpp
8730

Does this cover if one is a pointer with a null pointer value?

12587

This was in the if (getLangOpts().CPlusPlus) block previously, did you mean to move it out?

12620

This was in the if (getLangOpts().CPlusPlus) block previously, did you mean to move it out?

aaron.ballman marked 8 inline comments as done.

Updated based on review feedback.

clang/include/clang/AST/PrettyPrinter.h
68–69

Yes it should, but it's an NFC change. The only thing using Nullptr is the APValue pretty printer and I can't find a way to exercise that code path in C. That said, once we add support for constexpr in C, I believe we will be able to get into this path so we might as well handle it now.

If anyone can think of a test case that attempts to print a diagnostic on a pointer typed APValue in C, I'm happy to add it.

201

Nope, great catch! Think-o on my part. :-)

clang/include/clang/Basic/TokenKinds.def
393

Nope, we do the right thing now after Usman's changes. I'll add a test to demonstrate so we have better coverage.

clang/lib/Sema/SemaCast.cpp
2999

No real reason aside from muscle memory. I'll switch it up.

clang/lib/Sema/SemaExpr.cpp
8730

It behaves as expected; I've added additional test coverage to demonstrate that, good call!

12587

Yup, moving it out is intentional. This now covers C2x 6.5.9p5 as well.

12620

Yes -- this was handling Objective-C++ behavior and now that C has nullptr we want it to cover Objective-C as well.

erichkeane accepted this revision.Oct 7 2022, 9:49 AM

Happy with this, thanks!

This revision is now accepted and ready to land.Oct 7 2022, 9:49 AM
This revision was landed with ongoing or failed builds.Oct 14 2022, 7:06 AM
This revision was automatically updated to reflect the committed changes.