This is an archive of the discontinued LLVM Phabricator instance.

[Clang] Implement static operator[]
ClosedPublic

Authored by royjacobson on Nov 20 2022, 2:35 PM.

Details

Reviewers
cor3ntin
erichkeane
Group Reviewers
Restricted Project
Commits
rG3faf1f17a5c3: [Clang] Implement static operator[]
Summary

After accepted in Kona, update the code to accept static operator[] as well.

No big code changes: accept this operator as static in SemaDeclCXX, update AST call generation in SemaOverload and update feature macros + tests accordingly.

Diff Detail

Event Timeline

royjacobson created this revision.Nov 20 2022, 2:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 20 2022, 2:35 PM
royjacobson requested review of this revision.Nov 20 2022, 2:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 20 2022, 2:35 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
royjacobson added a reviewer: Restricted Project.Nov 20 2022, 2:35 PM

Thanks for working on this, it was fast!
It looks good except for an issue with the diagnostic (which was partly pre-existing)

clang/include/clang/Basic/DiagnosticSemaKinds.td
9116–9117

Hum, I did not notice that before but this looks wrong.
If I get that right, this is how it's usually done.

ie it should not be err_ (it's a warning) and in C++23 mode it's not an ExtWarn (which are turned into error in pendantic mode afaik). There are many similar patterns in the same file

  1. Update warnings according to Corentin's comment
  2. Somehow I missed the CodeGen was completely broken. Made a few necessary follow up changes in SemaOverload and ran the tests this time...
clang/include/clang/Basic/DiagnosticSemaKinds.td
9116–9117

thanks! looks much cleaner now.

royjacobson edited the summary of this revision. (Show Details)Nov 21 2022, 1:05 PM
cor3ntin added inline comments.Nov 21 2022, 2:27 PM
clang/lib/Sema/SemaOverload.cpp
14462–14467

This looks weird, did you clang-format?

cor3ntin accepted this revision.Nov 22 2022, 8:16 AM

Beside the formatting nitpick this looks good

clang/test/CodeGenCXX/cxx2b-static-subscript-operator.cpp
15

I think this is missing a test like
Functor::operator[](42, 42) for completeness

This revision is now accepted and ready to land.Nov 22 2022, 8:16 AM

Add a missing test case.

royjacobson marked 3 inline comments as done.Nov 23 2022, 10:11 AM
royjacobson added inline comments.
clang/lib/Sema/SemaOverload.cpp
14462–14467

clang-format insists on this, personally I wouldn't write it like this either :)

royjacobson marked an inline comment as done.Nov 23 2022, 10:13 AM

Beside the formatting nitpick this looks good

Thanks! I'll wait until next week to let other people have a chance to take a look.

erichkeane accepted this revision.Nov 28 2022, 8:28 AM

LGTM, thanks!

This revision was automatically updated to reflect the committed changes.