[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

2023-07-05 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars.cpp:2 +// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused %s + +void static_var() { cor3ntin wrote: > cor3ntin wrote: > > hubert.reinterpretcast w

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 537742. cor3ntin added a comment. Address Hubert's feedback - Add code and tests to properly support member initializer - Add code and tests to support designated initializers - Correctly hamdle members of anonymous union - Make placeholder redeclaration in

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 537745. cor3ntin added a comment. Add tests for static structured bindings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153536/new/ https://reviews.llvm.org/D153536 Files: clang/docs/ReleaseNotes.rst cla

[PATCH] D153621: [Clang] Correctly handle $, @, and ` when represented as UCN

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D153621#4449244 , @aaron.ballman wrote: > Changes generally LGTM, but I'll leave it to @tahonermann to do the final > sign-off given this is related to text. @tahonermann Ping :) Repository: rG LLVM Github Monorepo CHA

[PATCH] D139586: [Clang][C++23] Lifetime extension in range-based for loops

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin abandoned this revision. cor3ntin added a comment. The right way to do that is outline here https://reviews.llvm.org/D153701#4478240 There is no point in trying to keep this attempt around. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139

[PATCH] D153621: [Clang] Correctly handle $, @, and ` when represented as UCN

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 6 inline comments as done. cor3ntin added inline comments. Comment at: clang/test/Preprocessor/ucn-allowed-chars.c:16 - // Identifier initial characters tahonermann wrote: > I assume this line was deleted to minimize the disruption to line numb

[PATCH] D153418: Adding iconv support to CharSetConverter class

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D153418#4478766 , @tahonermann wrote: >> Please correct me if I'm wrong, I'm not too familiar with icu4c, but I think >> adding support for ICU would be the better long-term solution since it seems >> to allow the same beha

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks Shafik. I have some concern about the diagnostics being wonky because, if the init and field don't match, it's hard to be sure we won't run into simar issues (although i agree this fixes the crash.) Also, I'm not saying you should do this but... maybe this th

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:182 +namespace GH63605 { +struct { + unsigned : 2; Should we add bases? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154675/new/ https://reviews.llvm.org/

[PATCH] D153621: [Clang] Correctly handle $, @, and ` when represented as UCN

2023-07-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 537996. cor3ntin added a comment. Address Tom's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153621/new/ https://reviews.llvm.org/D153621 Files: clang/docs/ReleaseNotes.rst clang/include/clang/B

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision. Herald added a project: All. cor3ntin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Such jumps are not allowed by GCC and allowing them can lead to situations where we jumps into unevaluated statements. Fi

[PATCH] D154689: [clang] Correct calculation of MemberExpr's dependence

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/Expr.cpp:1760 + // FIXME: remove remaining dependence computation to computeDependence(). + auto Deps = E->getDependence(); Maybe we should do that now, by passing `TemplateArgs` to computeDependence?

[PATCH] D154689: [clang] Correct calculation of MemberExpr's dependence

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/Expr.cpp:1760 + // FIXME: remove remaining dependence computation to computeDependence(). + auto Deps = E->getDependence(); Fznamznon wrote: > cor3ntin wrote: > > Maybe we should do that now, by passin

[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538073. cor3ntin added a comment. I removed the changes to attributes. Nothing else changes except cxx_status/ReleaseNotes. Unevaluated strings in attributes will be back (in a separate PR) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D154689: [clang] Correct calculation of MemberExpr's dependence

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. I think this makes sense and it implements richard's suggestion. However, it's missing a release note, can you add that before landing? Thanks Repository: rG LLVM Github Monorepo CHAN

[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG95f50964fbf5: Implement P2361 Unevaluated string literals (authored by cor3ntin). Changed prior to commit: https://reviews.llvm.org/D105759?vs=538

[PATCH] D154689: [clang] Correct calculation of MemberExpr's dependence

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D154689#4480282 , @Fznamznon wrote: > In D154689#4480249 , @cor3ntin > wrote: > >> I think this makes sense and it implements richard's suggestion. >> However, it's missing a release

[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/docs/ReleaseNotes.rst:138 - Implemented `P2738R1: constexpr cast from void* `_. +- Partially implemented `P2361R6: constexpr cast from void* `_. + The changes to attributes

[PATCH] D154290: [WIP][Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538154. cor3ntin added a comment. Rebase. This is now ready for review. Note that after discussion with CWG, the consensus seems to be that the wording is fine, an implementation has to behave As if the full expression Message.data()[I] is called for each

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538249. cor3ntin added a comment. Address Shafik's feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154696/new/ https://reviews.llvm.org/D154696 Files: clang/docs/ReleaseNotes.rst clang/include/cla

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538250. cor3ntin marked an inline comment as done. cor3ntin added a comment. Missed a comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154696/new/ https://reviews.llvm.org/D154696 Files: clang/docs/Rel

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538252. cor3ntin marked an inline comment as done. cor3ntin added a comment. Fix test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154696/new/ https://reviews.llvm.org/D154696 Files: clang/docs/ReleaseNote

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 2 inline comments as done. cor3ntin added inline comments. Comment at: clang/docs/ReleaseNotes.rst:571 (`#61758 `_) +- Correcly diagnose jumps into statement expressions. + (`#63682

[PATCH] D154758: [clang][Interp] Emit correct diagnostic for uninitialized reads

2023-07-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM but please make sure to run clang-format before commiting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154758/new/ https://reviews.llvm.org/D154758 ___

[PATCH] D105759: Implement P2361 Unevaluated string literals

2023-07-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D105759#4482543 , @barannikov88 wrote: > @cor3ntin > I've been working on pretty much the same functionality in our downstream > fork. I was not aware of the paper, nor of the ongoing work in this > direction, and so I unf

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538350. cor3ntin marked 4 inline comments as done. cor3ntin added a comment. Address Timm's feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154290/new/ https://reviews.llvm.org/D154290 Files: clang/d

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16898 + } + + QualType T = Message->getType().getNonReferenceType(); tbaeder wrote: > What if the message is ` StringLiteral` but `getCharByteWidth()` doesn't > return `1`? We would get

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D154290#4482975 , @barannikov88 wrote: > According to the current wording, the static_assert-message is either > unevaluated string or an expression evaluated at compile time. > Unevaluated strings don't allow certain escape

[PATCH] D154773: [AST] Use correct APSInt width when evaluating string literals

2023-07-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a subscriber: aaron.ballman. cor3ntin added a comment. This revision is now accepted and ready to land. I think this is fine, and it's a nice simplification. However it doesn't seem to do much of anything in practice: if you look at `StringLiteral:

[PATCH] D154778: [clang] Fix __is_trivially_equality_comparable for classes which contain arrays of non-trivially equality comparable types

2023-07-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154778/new/ https://reviews.llvm.org/D154778 __

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

2023-07-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks, this looks good! (the libc++ test seems completely unrelated, it happens before compilation starts). Can you add re release note though? Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154675/new/ https://reviews.llvm.org/D154675 ___

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-10 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. > but right now I'm confused by the distinction… Why don't always evaluate the > message? 2 reasons - it would be a rather important breaking change for compiler who don't always use utf-8 at their literal encoding - we do not want to limit static_assert to the capabi

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

2023-07-10 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538545. cor3ntin added a comment. Add more lookup tests, including designated initializers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153536/new/ https://reviews.llvm.org/D153536 Files: clang/docs/Releas

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

2023-07-10 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538549. cor3ntin added a comment. Add missing designated initializer test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153536/new/ https://reviews.llvm.org/D153536 Files: clang/docs/ReleaseNotes.rst clan

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-10 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 538579. cor3ntin marked 3 inline comments as done. cor3ntin added a comment. Address Sergei's feedback - Add more tests - Support non const member functions - Make sure diagnostics messages are never produced twice - Support returning intermediate objects fr

[PATCH] D154893: [Clang] Fix some triviality computations

2023-07-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. This is a bit confusing but I think it is correct. LGTM modulo typos. Comment at: clang/docs/ReleaseNotes.rst:70 --- -- A bug in evaluating the i

[PATCH] D153621: [Clang] Correctly handle $, @, and ` when represented as UCN

2023-07-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @tahonermann I'll probably merge that EoW unless you scream! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153621/new/ https://reviews.llvm.org/D153621 ___ cfe-commits mailing l

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

2023-07-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @dblaikie Would you be willing to look at the debugger side of things in a subsequent patch? I'm not familiar with debug symbol code gen so I'm not sure I'd be able to improve thing the right way. Comment at: clang/test/SemaCXX/cxx2c-placeholder-vars

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-11 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. cor3ntin marked 2 inline comments as done. Closed by commit rGb0cc947b5d0a: [Clang] Diagnose jumps into statement expressions (authored by cor3ntin). Changed prior to

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks for the review :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154696/new/ https://reviews.llvm.org/D154696 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D153621: [Clang] Correctly handle $, @, and ` when represented as UCN

2023-07-11 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG304e97469455: [Clang] Correctly handle $, @, and ` when represented as UCN (authored by cor3ntin). Changed prior to commit: https://reviews.llvm.o

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 539467. cor3ntin marked an inline comment as done. cor3ntin added a comment. - Rename EvaluateCharPointerAsString => EvaluateCharRangeAsString - Improve diag messages - Form an overload set to diagnose invalid overloads - Add more tests Repository: rG LLV

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:16417-16418 + } + if (!Scope.destroy()) +return false; + aaron.ballman wrote: > Rather than use an RAII object and destroy it manually, let's use `{}` to > scope the RAII object app

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 539501. cor3ntin added a comment. Add tests for deleted methods and methods whose constraints are not satisfied. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154290/new/ https://reviews.llvm.org/D154290 File

[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. This looks reasonable to me. But can you add an entry into clang/docs/ReleaseNotes.rst? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154716/new/ https://reviews.llvm.org/D154716 _

[PATCH] D153701: [WIP][Clang] Implement P2718R0 "Lifetime extension in range-based for loops"

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin requested changes to this revision. cor3ntin added a comment. This revision now requires changes to proceed. Marking that as needing revision while the author explores the direction outlined by Richard :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D154761: [clang][Interp] Diagnose callsite for implicit functions

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154761/new/ https://reviews.llvm.org/D154761 __

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks for working on this! I left some comments, i hope it helps! Comment at: clang/include/clang/Sema/Sema.h:1277 +/// as PotentiallyEvaluated. +RuntimeEvaluated }; I think I'd prefer a boolean for that, `ExpressionEvaluat

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 3 inline comments as done. cor3ntin added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:110-111 + +static_assert(false, Leaks{}); //expected-error {{the message in a static assertion must be produced by a constant expression}} \ +

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin reopened this revision. cor3ntin added a comment. This revision is now accepted and ready to land. @nathanchance Thanks for letting me know. I did revert the patch and plan to investigate later this week Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

2023-07-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. The issue is that `VerifyIndirectOrAsmJumps` does not consider that label may be local label at all, I'm not exactly sure how to improve that. The fact it works currently seems kind of brittle, what prevent incorrect jumps to be ill-formed is that we looked if a label

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

2023-07-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman @shafik Thanks! I'll address the small nitpicks one we all agree on a direction on whether to have a warning/error/nothing on `static_assert(true, non_constant_expression)` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

2023-07-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision. Herald added a project: All. cor3ntin requested review of this revision. Herald added subscribers: cfe-commits, wangpc. Herald added a project: clang. This patch does a few things: - Fix aggregate initialization. When an aggregate has an initializer that is immedi

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

2023-07-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // that are not referenced later. e.g.: if (int var = init()); + if (!T->isAggregateType()) +ConditionVar->se

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

2023-07-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // that are not referenced later. e.g.: if (int var = init()); + if (!T->isAggregateType()) +ConditionVar->se

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

2023-07-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/Parse/Parser.h:578-582 + bool isTokenConcatenable() const { +return isTokenStringLiteral() || + getLangOpts().MicrosoftExt && + tok::isMSPredefinedMacro(Tok.getKind()); + } -

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

2023-07-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:1300 case tok::kw___PRETTY_FUNCTION__: // primary-expression: __P..Y_F..N__ [GNU] -Res = Actions.ActOnPredefinedExpr(Tok.getLocation(), SavedKind); -ConsumeToken(); +Res = ParsePredefinedExp

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

2023-07-14 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D155175#4498788 , @efriedma wrote: > Do we need CodeGen testcases here for full coverage? The testcases from the > issue passed with -fsyntax-only. > > > > With this patch, the following cases produce errors that do

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

2023-07-14 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. I found further issues with aggregates, I think not supporting aggregates for now would be fine, the high bit is to fix the regressions I did add some more info there https://github.com/cplusplus/CWG/issues/354#issuecomment-1636129255 Repository: rG LLVM Github Mon

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

2023-07-14 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. This looks reasonable to me now. I'll wait next week before approving it so other get a chance to look at it :) Comment at: clang/lib/Sema/SemaExprCXX.cpp:4016-4019 + // Ensure that `-Wunused-variable` will be emitted for condition variables + // tha

[PATCH] D153914: [clang-cl] Enable concatenation of predefined identifiers

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:3288-3291 + assert( + (StringToks.size() != 1 || + !tok::isUnexpandableMsMacro(StringToks[0].getKind())) && + "single predefined identifiers shall be handled by ActOnPredefinedExpr"); -

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks a lot for working on this. This probably needs a release note entry Comment at: clang/lib/Sema/JumpDiagnostics.cpp:790 // reach this label scope. for (auto [JumpScope, JumpStmt] : JumpScopes) { + // This unnecessary copy is becaus

[PATCH] D155342: [clang][JumpDiagnostics] ignore non-asm goto target scopes

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/JumpDiagnostics.cpp:791-800 + // This unnecessary copy is because: + // warning: captured structured bindings are a C++20 extension + // [-Wc++20-extensions] + LabelDecl *TL = TargetLabel; + // Is

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/Interp/Function.h:180 +// FIXME: Is there a better way to get this information? +if (getName() == "__builtin_isnan") + return true; You can compare getBuiltinID(), which is better than using s

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/Interp/Function.h:180 +// FIXME: Is there a better way to get this information? +if (getName() == "__builtin_isnan") + return true; tbaeder wrote: > cor3ntin wrote: > > You can compare getBuil

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 540773. cor3ntin added a comment. Fix the evaluation context of member initializers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155175/new/ https://reviews.llvm.org/D155175 Files: clang/include/clang/Sema

[PATCH] D154795: [clang][Interp] Check pointers for live-ness when returning them

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. LGTM. It may be worth submitting an issue for the current behavior which is wrong. Do you want to add a quote to the standard? (https://eel.is/c++draft/expr.const#13.3) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154795

[PATCH] D155367: [clang][Interp] Implement __builtin_inf() etc.

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:119 +static bool interp__builtin_inf(InterpState &S, CodePtr OpPC, +const InterpFrame *Frame, const Function *F) { why does that return a bool? yo

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/Sema/Lookup.h:197-200 +DiagnoseAccess(std::move(Other.DiagnoseAccess)), +DiagnoseAmbiguous(std::move(Other.DiagnoseAmbiguous)), AllowHidden(std::move(Other.AllowHidden)), Shadowed(st

[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1546 def warn_consteval_if_always_true : Warning< + "consteval if is always %select{true|false}0 in this context">, To be consistent with other tautological warnings

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D155175#4498788 , @efriedma wrote: > Do we need CodeGen testcases here for full coverage? The testcases from the > issue passed with -fsyntax-only. You have something specific in mind? The examples in the issues now lead to

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 540809. cor3ntin added a comment. Assert if we try to generate code for a CallExpr to an immediate function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155175/new/ https://reviews.llvm.org/D155175 Files:

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/Sema/Lookup.h:228-229 Other.Paths = nullptr; -Other.Diagnose = false; +Other.DiagnoseAccess = false; +Other.DiagnoseAmbiguous = false; return *this; rsmith wrote: > cor3ntin wro

[PATCH] D155367: [clang][Interp] Implement __builtin_inf() etc.

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:119 +static bool interp__builtin_inf(InterpState &S, CodePtr OpPC, +const InterpF

[PATCH] D155401: [clang][Interp] Implement __builtin_fmax

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155401/new/ https://reviews.llvm.org/D155401 ___

[PATCH] D155400: [clang][Interp] Implement __builtin_fabs()

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155400/new/ https://reviews.llvm.org/D155400 ___

[PATCH] D155371: [clang][Interp] Implement __builtin_isinf

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155371/new/ https://reviews.llvm.org/D155371 ___ cfe-commits mailing list cfe-commi

[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:166-168 + // FIXME: We are not going through getParam() here, because the function + // doesn't have any paramete

[PATCH] D155374: [clang][Interp] Implement __builtin_isnormal

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155374/new/ https://reviews.llvm.org/D155374 ___

[PATCH] D155410: [clang][Interp] Fix comparing nan/inf floating point values

2023-07-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155410/new/ https://reviews.llvm.org/D155410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

2023-07-17 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 540902. cor3ntin added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Add more tests for aggregates and a codegen test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155175/new

[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

2023-07-17 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 541057. cor3ntin added a comment. Revert inadvertently committed cmake changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155175/new/ https://reviews.llvm.org/D155175 Files: clang/include/clang/Sema/Sem

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @ChuanqiXu Thanks. Do you think this needs to be backported for coroutine support? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158601/new/ https://reviews.llvm.org/D158601 __

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Gotcha. Given there are other stuffs I need to backport, this will have to wait clang 18! I don't want to give undue work to tobias and testers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158601/new/ https://reviews.ll

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 553038. cor3ntin added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158601/new/ https://reviews.llvm.org/D158601 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/ExprConstant.cpp clan

[PATCH] D158718: [Clang] Handle ellipsises in abiguous cast expressions.

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision. Herald added a project: All. cor3ntin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. `(T())(foo)` is a cast expression because `T()` is treated as a type-id, and the expression is valid. However, `(T())(foo

[PATCH] D158433: [Clang] Do not change the type of captured vars when checking lambda constraints

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 553047. cor3ntin added a comment. Missing curlies Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158433/new/ https://reviews.llvm.org/D158433 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaConce

[PATCH] D158733: [Clang] Fix a crash when an invalid immediate function call appears in a cast

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision. Herald added a project: All. cor3ntin requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Fixes #64949 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D158733 Files: clang/d

[PATCH] D158733: [Clang] Fix a crash when an invalid immediate function call appears in a cast

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 553109. cor3ntin added a comment. Remove accidentyally commited change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158733/new/ https://reviews.llvm.org/D158733 Files: clang/docs/ReleaseNotes.rst clang/l

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:911-915 if (X->getNumParams() != Y->getNumParams()) return false; for (unsigned I = 0; I < X->getNumParams(); ++I) if (!Ctx.hasSameUnqualifiedType(X->getParamDecl(I)->getType(),

[PATCH] D158433: [Clang] Do not change the type of captured vars when checking lambda constraints

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG158f4f30adb4: [Clang] Do not change the type of captured vars when checking lambda constraints (authored by cor3ntin). Repository: rG LLVM Github

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3f98cdc815bd: [Clang] Always constant-evaluate operands of comparisons to nullptr (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1586

[PATCH] D158808: [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check whether the lambda-declarator is valid

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. LGTM modulo: - No release notes - I think you need to write in the commit Fixes https://github.com/llvm/llvm-project/issues/64962 Fixes https://github.com/llvm/llvm-project/issues/28679 for github not to be confused - You need to clang-format

[PATCH] D158591: Add support of Windows Trace Logging macros

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a subscriber: tbaeder. cor3ntin added a comment. Looks mostly good to me, thanks! In D158591#4613868 , @RIscRIpt wrote: > The current implementation of `getPredefinedExprDeclContext` could be easily > moved to a static function inside `cl

[PATCH] D158502: [clang][Interp] Actually consider ConstantExpr result

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. FYI there are build failures. you probably need to rebase (visitAPValue does not seem to actually exist) Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:767-772 + std::optional T = classify(E->getType()); + if (T && E->hasAPValueResult() && +

[PATCH] D158733: [Clang] Fix a crash when an invalid immediate function call appears in a cast

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 553381. cor3ntin added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158733/new/ https://reviews.llvm.org/D158733 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaExpr.cpp

[PATCH] D158733: [Clang] Fix a crash when an invalid immediate function call appears in a cast

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 553385. cor3ntin added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158733/new/ https://reviews.llvm.org/D158733 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaExpr.cpp clang/t

[PATCH] D158733: [Clang] Fix a crash when an invalid immediate function call appears in a cast

2023-08-24 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6824d156d5dd: [Clang] Fix a crash when an invalid immediate function call appears in a cast (authored by cor3ntin). Repository: rG LLVM Github Mon

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-08-25 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/CXX/drs/dr25xx.cpp:104-106 +struct D : B { + void f(this D&); // expected-error {{an explicit object parameter cannot appear in a virtual function}} +}; aaron.ballman wrote: > I'd like to see two other test

[PATCH] D140828: [C++] Implement "Deducing this" (P0847R7)

2023-08-25 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 4 inline comments as done. cor3ntin added inline comments. Comment at: clang/test/SemaCXX/cxx2b-deducing-this.cpp:19 + +void g(this auto) const; // expected-error{{a function with an explicit object parameter cannot be const}} +void h(this auto) &; // exp

<    6   7   8   9   10   11   12   13   14   15   >