[PATCH] D136449: [Clang] Implement P2513

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this! Can you add more details to the patch summary as to what this paper does? Paper numbers help, but don't convey a whole lot of information at a glance if we need to come back to this review for code archeology.

Please also add a test to the correct file in clang/test/CXX/drs/ and regenerate our C++ DR status page.

Are we missing changes for "with an integral conversion [conv.integral] if necessary for the source and destination value." ?



================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:701
+    Builder.defineMacro("__cpp_char8_t",
+                        LangOpts.CPlusPlus20 ? "202207L" : "201811L");
   Builder.defineMacro("__cpp_impl_destroying_delete", "201806L");
----------------
Why do we not want to go with `202207L` regardless of language mode if this is being applied as a DR?


================
Comment at: clang/lib/Sema/SemaInit.cpp:4466-4467
         }
+        }
       }
   }
----------------
Unintentional indentation changes?


================
Comment at: clang/lib/Sema/SemaInit.cpp:5858-5861
+      case SIF_UTF8StringIntoPlainChar: {
         SetFailed(FK_UTF8StringIntoPlainChar);
         return;
+      }
----------------
Unrelated change?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136449/new/

https://reviews.llvm.org/D136449