royjacobson requested changes to this revision. royjacobson added a reviewer: clang-language-wg. royjacobson added a comment. This revision now requires changes to proceed.
Hi Javier, thank you for submitting this patch! As far as I could tell, this patch implements the CWG2171 defect report from 2016: https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#2171. That means that you'll have to add a test inside clang/test/CXX/drs/dr21xx.cpp to make sure it shows on the DRs status page. This change is also a large ABI break, which means it has to be considered carefully. I think the best approach here would be to revert to the previous behavior when the -fclang-abi-compat flag is used for clang <= 14, but I'm not sure about this. I would like the approval of someone with more ABI experience here. The code itself and the tests look good! If you'll need help with the changes I mentioned please let me know. ================ Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9786-9789 + if (RT && ((RT->getPointeeType().getCVRQualifiers() & Qualifiers::Const) == + Qualifiers::Const)) { + ConstArg = true; + } ---------------- I think this should work instead? No need to check for RT since we already returned if !RT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127593/new/ https://reviews.llvm.org/D127593 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
