[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the reviews, @efriedma! I have committed in c92f505346b80fd053ef191bbc66810c9d564b0c CHANGES SINCE LAST ACTION

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-08 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103611/new/ https://reviews.llvm.org/D103611 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:15789-15790 +: Context.getCorrespondingUnsignedType(UnderlyingType); +if (Context.typesAreCompatible(PromoteType, UnderlyingType, +

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 350575. aaron.ballman added a comment. Updated the comment and removed a check for C++ mode. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103611/new/ https://reviews.llvm.org/D103611 Files: clang/lib/Sema/SemaExpr.cpp

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:15775 + if (Context.typesAreCompatible(PromoteType, UnderlyingType, + /*CompareUnqualified*/ true)) PromoteType = QualType(); If we're not

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:15783 + // test for typesAreCompatible() will already properly consider those to + // be compatible types. + if (Context.getLangOpts().CPlusPlus && !PromoteType.isNull() &&

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:15783 + // test for typesAreCompatible() will already properly consider those to + // be compatible types. + if (Context.getLangOpts().CPlusPlus && !PromoteType.isNull() &&

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 350060. aaron.ballman added a comment. Added some more logic and new tests. The CI pipeline pointed out a valid issue which has now been corrected -- C++ was not properly handling the case where one type was `int` and the other was `unsigned int`,

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 349945. aaron.ballman added a comment. Changed the test case to speculatively see if the CI pipeline is happy with this (I'm trying to avoid adding a triple to the test). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103611/new/

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103611/new/ https://reviews.llvm.org/D103611 ___ cfe-commits mailing list

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D103611#2797044 , @efriedma wrote: > I'm a little nervous about using C type merging in C++; it's not designed to > support C++ types, so it might end up crashing or something like that. I > think I'd prefer to

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 349833. aaron.ballman added a comment. Update based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103611/new/ https://reviews.llvm.org/D103611 Files: clang/lib/Sema/SemaExpr.cpp clang/test/SemaCXX/varargs.cpp Index:

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I'm a little nervous about using C type merging in C++; it's not designed to support C++ types, so it might end up crashing or something like that. I think I'd prefer to explicitly convert enum types to their underlying type. Repository: rG LLVM Github Monorepo

[PATCH] D103611: Correct the behavior of va_arg checking in C++

2021-06-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, eli.friedman, rjmccall. aaron.ballman requested review of this revision. Herald added a project: clang. Clang checks whether the type given to `va_arg` will automatically cause undefined behavior, but this check was