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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
