[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 436133. cor3ntin added a comment. - Rebase - The generator code is more consistent with LLVM style guides. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: c

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @tstellar I saw you say in another path that you got confirmation from lawyers that it's okay to include UnicodeData.txt (which this patch doesn't do) and derived data (which this patch does). Can you confirm? Thanks Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 436137. cor3ntin added a comment. Use utohexstr and revert the changes that put to_hexString in StringExtras. I'll remove to_hexString entierly in a separate NFC change to avoid duplication and further confusion. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 436142. cor3ntin added a comment. Do not hardcode the list of generated code points ranges in the generator code to ease maintainance burden. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://r

[PATCH] D122768: [Clang][C++20] Support capturing structured bindings in lambdas

2022-06-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 436212. cor3ntin added a comment. - Rebase - Cleanup codegen test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122768/new/ https://reviews.llvm.org/D122768 Files: clang-tools-extra/clang-tidy/modernize/Loo

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: llvm/lib/Support/UnicodeNameToCodepoint.cpp:49 +std::string s; +s.reserve(64); +auto n = this; aaron.ballman wrote: > aaron.ballman wrote: > > Any particular reason for 64? > Still wondering why 64 bytes spe

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 436661. cor3ntin marked an inline comment as done. cor3ntin added a comment. Address Aaron's comments - `{}` => `llvm::None` in Lexer.cpp - Fix casing in UnicodeNameToCodepoint.cpp to match the style, and a couple I missed in UnicodeNameMappingGenerator.cpp

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-14 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 436682. cor3ntin marked 12 inline comments as done. cor3ntin added a comment. Regenerate UnicodeNameToCodepointGenerated.cpp to fix the formatting of its header. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-14 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked an inline comment as done. cor3ntin added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:3237-3239 + if (C != '{') { +if (Diagnose) + Diag(StartPtr, diag::warn_ucn_escape_incomplete); aaron.ballman wrote: > Is this a case where we

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-14 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 436703. cor3ntin added a comment. Fix more style issue (variable casing) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang/include/clang/Basic/Diagnosti

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 437160. cor3ntin marked 17 inline comments as done. cor3ntin added a comment. Address more style issues found by Aaron Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 437184. cor3ntin added a comment. Add unicode license notice to generated file derived fromn the unicode data Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files:

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 437186. cor3ntin added a comment. Fix wrapping in file header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang/include/clang/Basic/DiagnosticLexKinds.t

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D123064#3585074 , @aaron.ballman wrote: > I had a discussion about the license file on IRC (but not with @tstellar) and > the thinking there was: add the license file. It seems to either be required > or would be harmless t

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-17 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision. Herald added a subscriber: hiraditya. Herald added a project: All. cor3ntin requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Introduce an off-by default `-Winvalid-utf8` warning that detects

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

2022-06-20 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. So that this doesn't get lost, I'm opposed to this change because it does not actually follow the TR39 guidance or algorithm. For an input string X, define skeleton(X) to be the following transformation on the string: 1. Convert X to NFD format, as described in

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 438926. cor3ntin marked 3 inline comments as done. cor3ntin added a comment. - Address style comments - Improve commit message - Enable the warning in -pedantic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2399 + getSourceLocation(CurPtr)); + bool UnicodeDecodeFailed = false; + aaron.ballman wrote: > It looks like this can move into the `while` loop and we can

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 438984. cor3ntin added a comment. s/UnicodeDecodeFailed/UnicodeDecodingAlreadyDiagnosed/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128059/new/ https://reviews.llvm.org/D128059 Files: clang/docs/ReleaseN

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 438990. cor3ntin added a comment. Make sure the warning is off by default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128059/new/ https://reviews.llvm.org/D128059 Files: clang/docs/ReleaseNotes.rst cla

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 439047. cor3ntin added a comment. Vectorizes comment skipping even when -Winvalid-utf8 is enabled. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128059/new/ https://reviews.llvm.org/D128059 Files: clang/doc

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 439057. cor3ntin added a comment. Further optimize Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128059/new/ https://reviews.llvm.org/D128059 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/D

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman I created a 1.3GB file containing 50 comments of random size and content, with the following script import string import random for i in range(0, 50): print("/*{}*/".format(''.join(random.choices('\n' + string.ascii_uppercase + str

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2717 +__m128i Bytes = +_mm_loadu_si128(reinterpret_cast(CurPtr)); +int Mask = _mm_movemask_epi8(Bytes); This crashes when using `_mm_load_si128` which suprises me beca

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2717 +__m128i Bytes = +_mm_loadu_si128(reinterpret_cast(CurPtr)); +int Mask = _mm_movemask_epi8(Bytes); aaron.ballman wrote: > aaron.ballman wrote: > > cor3ntin wrote:

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 439177. cor3ntin added a comment. - Add a comment referencing the (to be) C++ wording and unicode discussion on replacemet characters - Do not try to skip utf8 checks when the warning is not enabled as checking whether the warning is enabled is more expensi

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

2022-06-23 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 439276. cor3ntin added a comment. - Remove explicit load instruction - cleanup extra braces Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128059/new/ https://reviews.llvm.org/D128059 Files: clang/docs/Relea

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-23 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @tahonermann gentle ping (Aaron told me you might have further comments) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 ___ cfe-commi

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-25 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 rGc92056d03881: [Clang][C++23] P2071 Named universal character escapes (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

2022-06-25 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman @tahonermann Thanks for the review. I landed the change after confirming with Aaron he was happy with it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 _

[PATCH] D108469: Improve handling of static assert messages.

2022-06-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 440063. cor3ntin marked 4 inline comments as done. cor3ntin added a comment. Herald added a subscriber: steakhal. Herald added a project: All. - Rebase - Address Aaron's comments. - Fix tests - Keep dumping wide/utf16/utf32 strings escaped until we can ensure

[PATCH] D64087: [clang] Correct source locations for instantiations of out-of-line defaulted special member functions. (PR25683)

2023-09-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. 1 nit but still LGTM Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4993 - // Copy the inner loc start from the pattern. + // Copy source locations from the pattern. + Function->setLocation(PatternDecl-

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-09-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/static-assert.cpp:287 + static_assert((char16_t)L'ゆ' == L"C̵̭̯̠̎͌ͅť̺"[1], ""); // expected-error {{failed}} \ + // expected-note {{evaluates to ''ゆ' (0x3086) == '̵'

[PATCH] D159522: [Clang][C] Fixed a bug where we reject an _Atomic qualified integer in a switch statment

2023-09-20 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG624c13057874: [Clang][C] Fixed a bug where we reject an _Atomic qualified integer in a switch… (authored by to268, committed by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D159522: [Clang][C] Fixed a bug where we reject an _Atomic qualified integer in a switch statment

2023-09-20 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @to268 as discussed, i landed the change on your behalf. Thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159522/new/ https://reviews.llvm.org/D159522 ___ cfe

[PATCH] D156064: [SemaCXX] Recognise initializer_list injected-class-name types as initializer_lists

2023-09-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @MitalAshok Do you need me to land that for you> If so, what email should i use? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156064/new/ https://reviews.llvm.org/D156064 ___ c

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

2023-09-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. @hazohelet Nah, feel free to merge that first! Thanks CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155064/new/ https://reviews.llvm.org/D155064 ___ cfe-commits mailing list cfe-commi

[PATCH] D158071: [clang] Remove rdar links; NFC

2023-09-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin closed this revision. cor3ntin added a comment. This was landed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158071/new/ https://reviews.llvm.org/D158071 ___ cfe-commits mailing list cfe-commit

[PATCH] D156890: [clang] Remove outdated INSTALL.txt

2023-09-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @stephanlachnit do you need me to land this on your behalf? What email should we use? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156890/new/ https://reviews.llvm.org/D156890 ___

[PATCH] D69763: [Clang][Test]: Remaining "lld-link2" -> "lld-link"

2023-09-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Herald added a project: All. @rnk can we close this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69763/new/ https://reviews.llvm.org/D69763 ___ cfe-commits mailing list cfe-co

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. I had a chat with @hubert.reinterpretcast and @tahonermann. We reached consensus on wanting to make sure the codepoint value is formatted in a future-proof way so that if we ever implement escaping of lone combining codepoint, this case would be handled as well. To do

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

2023-09-27 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/AST/ASTLambda.h:45 + return isLambdaCallOperator(DC) && + !cast(DC)->getType().isNull() && + !cast(DC)->isExplicitObjectMemberFunction(); aaron.ballman wrote: > cor3ntin wrote: > > e

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

2023-09-28 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 7 inline comments as done. cor3ntin added inline comments. Comment at: clang/include/clang/Sema/Sema.h:3775 bool UseMemberUsingDeclRules, bool ConsiderCudaAttrs = true, - bool ConsiderRequiresClauses = true); +

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-10-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @hazohelet Please keep this patch on Phab. It's not going to be shutdown. The current consensus is that we will reconsider shutting down phab on November 15 https://discourse.llvm.org/t/update-on-github-pull-requests/71540/125 CHANGES SINCE LAST ACTION https://review

[PATCH] D155610: [Clang][Sema] Fix display of characters on static assertion failure

2023-10-02 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! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155610/new/ https://reviews.llvm.org/D155610 ___ cfe-commits mailing list

[PATCH] D156565: Diagnose use of VLAs in C++ by default

2023-10-02 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/D156565/new/ https://reviews.llvm.org/D156565 ___ cfe-commits mailing list cfe-comm

[PATCH] D155548: [clang][ExprConst] Short-circuit ConstantExpr evaluation

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. I think this does make sense. Have you notice an impact on performance? I wonder if there are places where we should cache the result of evaluating a ConstantExpr, like in `ExprEvaluatorBase::VisitConstantExpr` How often do we construct a ConstantExpr without an initial

[PATCH] D155552: [clang][Interp] Support __null

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/AST/Interp/literals.cpp:16 +static_assert(__null == __null, ""); static_assert(1 == 1, ""); static_assert(1 == 3, ""); // expected-error{{failed}} ref-error{{failed}} Can you add a test along the lines of

[PATCH] D155546: [clang][Interp] Implement __builtin_fmin

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Looks fine but where are the tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155546/new/ https://reviews.llvm.org/D155546 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D155573: [Clang] Fix the location of default init expressions

2023-07-18 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. Default member initializations constructed from a parenthesized aggregate initialization should be constructed at the loca

[PATCH] D155580: [trivial-auto-var-init] Do not emit initialization code for empty class

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:1915-1924 LangOptions::TrivialAutoVarInitKind trivialAutoVarInit = (D.isConstexpr() ? LangOptions::TrivialAutoVarInitKind::Uninitialized : (D.getAttr() ?

[PATCH] D155584: [clang][NFC] Simplify SourceLocExpr::EvaluateInContext

2023-07-18 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 agree, this looks better Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155584/new/ https://reviews.llvm.org/D155584 _

[PATCH] D155573: [Clang] Fix the location of default init expressions

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/source_location.cpp:796 +static_assert(S(0).i == S{0}.i); +static_assert(S(0).j == S{0}.i); +} aaron.ballman wrote: > Shouldn't this test fail because `i != j`? > > Can you add a test that demonstrat

[PATCH] D155573: [Clang] Fix the location of default init expressions

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 541473. cor3ntin added a comment. Address Aaron's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155573/new/ https://reviews.llvm.org/D155573 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/Sema

[PATCH] D155573: [Clang] Fix the location of default init expressions

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/source_location.cpp:796 +static_assert(S(0).i == S{0}.i); +static_assert(S(0).j == S{0}.i); +} tbaeder wrote: > aaron.ballman wrote: > > cor3ntin wrote: > > > aaron.ballman wrote: > > > > Shouldn't th

[PATCH] D155580: [trivial-auto-var-init] Do not emit initialization code for empty class

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:1913-1923 + if (D.isConstexpr()) +// Constexpr already initializes everything correctly. +trivialAutoVarInit = LangOptions::TrivialAutoVarInitKind::Uninitialized; + else if (D.getAttr()) +trivi

[PATCH] D155627: [clang][Interp] Handle SourceLocExprs

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Note that you are probably need something like `SourceLocExprScopeGuard` to make source_location actually work, , it might be worth doing in this patch. The idea is to remember the location of a call when evaluating default arguments / default member initializers Repo

[PATCH] D155627: [clang][Interp] Handle SourceLocExprs

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:153 + static_assert(__builtin_LINE() == 152, ""); +} what is missing to enable the existing source location tests with the new intterpreter? Repository: rG LLVM Github M

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. I agree with Aaron that in the current state, the common case diagnostics are made worse. But there is room for improvement! I think what we want to do here is modify `ConvertAPValueToString` so that it applies the same escape logic to `char` as we do in `pushEscapedSt

[PATCH] D155573: [Clang] Fix the location of default init expressions

2023-07-18 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG64cfcde31a48: [Clang] Fix the location of default init expressions (authored by cor3ntin). Changed prior to commit: https://reviews.llvm.org/D155573?vs=541473&id=541853#toc Repository: rG LLVM Github

[PATCH] D154368: [Clang] Fix constraint checking of non-generic lambdas.

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

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. FYI, this can be relanded after https://reviews.llvm.org/D155342 is merged Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154696/new/ https://reviews.llvm.org/D154696 ___ cfe-com

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 7 inline comments as done. cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:2441 bool Sema::CheckImmediateEscalatingFunctionDefinition( -FunctionDecl *FD, bool HasImmediateEscalatingExpression) { - if (!FD->hasBody() || !getLangOpts

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 541983. cor3ntin added a comment. Address Mariya & Aaron's feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155175/new/ https://reviews.llvm.org/D155175 Files: clang/include/clang/AST/RecursiveASTVisi

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 5 inline comments as done. cor3ntin added inline comments. Comment at: clang/test/SemaCXX/cxx2b-consteval-propagate.cpp:245-246 +void test() { +[[maybe_unused]] SS s; // expected-error {{call to immediate function 'DefaultedUse::SS::SS' is not a constant expr

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 542022. cor3ntin marked 17 inline comments as done. cor3ntin added a comment. - Display a warning, defaulted as error, if the message cannot be evaluated in a static assertion that does not fail - Address Shafik's, Timm's and Aaron's feedbacks Repository:

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked an inline comment as done. cor3ntin added inline comments. Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:2730-2734 +DEF_TRAVERSE_STMT(CXXDefaultInitExpr, { + if (getDerived().shouldVisitImplicitCode()) +TRY_TO(TraverseStmt(S->getExpr())); +}) + --

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. This looks good to me modulo nitpicks. When you land that, please make an issue on github for the missing narrowing warning, it seems important. I'll wait before approving in case @aaron.ballman spot things i missed Comment at: clang/lib/Sema/SemaEx

[PATCH] D155627: [clang][Interp] Handle SourceLocExprs

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:153 + static_assert(__builtin_LINE() == 152, ""); +} tbaeder wrote: > tbaeder wrote: > > cor3ntin wrote: > > > what is missing to enable the existing source location tests wi

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 542049. cor3ntin added a comment. Fix test name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155175/new/ https://reviews.llvm.org/D155175 Files: clang/include/clang/AST/RecursiveASTVisitor.h clang/includ

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:16408 +Char) || +!Char.isInt()) + return false; aaron.ballman wrote: > shafik wrote: > > Why are we specifically checking `!isInt()` wh

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 542102. cor3ntin marked an inline comment as done. cor3ntin added a comment. - Add test for dependent message - always dump the message even if it's not a string literal - remove superflous isInt - fix diag message Repository: rG LLVM Github Monorepo CHA

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:269 + //expected-error {{static assertion failed}} \ + // expected-note {{in call to 'InvalidPtr{}.data()'}} aaron.ballman

[PATCH] D155610: [Clang][ExprConstant] Print integer instead of character on static assertion failure

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:24 // expected-error@11 {{}} expected-warning@11 {{trigraph ignored}} -// expected-error@13 {{failed}} expected-warning@13 {{trigraph ignored}} expected-note@13 {{evaluates to ''?' == '#''}} +// ex

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

2023-07-19 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor. +inline bool isUnexpandableMsMacro(TokenKind K) { + return K == tok::kw___FUNCTION__ || K == tok:

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

2023-07-19 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 rG47ccfd7a89e2: [Clang] Implement P2741R3 - user-generated static_assert messages (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANG

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

2023-07-20 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 542389. cor3ntin added a comment. Correctly track constructor location. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155175/new/ https://reviews.llvm.org/D155175 Files: clang/include/clang/AST/RecursiveAST

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

2023-07-20 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 542435. cor3ntin added a comment. Remove an assert (added in a previous iteration of this PR). Unfortunately, we sometimes do emit the address of an immediate function, after we establish the program is ill-formed - as we still proceed to code gen. cpp

[PATCH] D154475: [clang][Interp] Fix ignoring MaterializeTemporaryExprs

2023-07-20 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman any opinion on that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154475/new/ https://reviews.llvm.org/D154475 ___ cfe-commits mailing list cfe-commits@lists.llv

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

2023-07-20 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. FYI, my current aim is to land that early in the Clang 18 cycle, that should give us time to find any remaining issue and maybe improve debugger support! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153536/new/ https://r

[PATCH] D155714: [clang] Fix diagnostics for defaulted, implicitly deleted 'operator=='.

2023-07-20 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Looks generally good to me. can you add an entry in clang/docs/ReleaseNotes.rst? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155714/new/ https://reviews.llvm.org/D155714

[PATCH] D154368: [Clang] Fix constraint checking of non-generic lambdas.

2023-07-21 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGf9caa12328b2: [Clang] Fix constraint checking of non-generic lambdas. (authored by cor3ntin). Changed prior to commit: https://reviews.llvm.org/D154368?vs=541858&id=542806#toc Repository: rG LLVM Git

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

2023-07-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 542813. cor3ntin added a comment. Rebase + add test 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/clang/Basic/

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

2023-07-21 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 rG202191060602: [Clang] Diagnose jumps into statement expressions (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D155955: [Clang] Improve the handling of large arrays evaluation.

2023-07-21 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. This is a temporary fix (for clang 17) that caps the size of any array we try to constant evaluate: There are 2 limits:

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

2023-07-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. > Should we be marking the init expression/declaration as invalid to force > codegen not to run on the expression? So it's not removing the init, but is > marking the declaration its attached to as invalid. We should remove the init from the declaration entirely, the t

[PATCH] D155457: [clang] Skip tautological comparison if the comparison involves the 'size_t' type

2023-07-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. I'm not sure I understand the motivation for this change. Sure, people do that but they also might do the same thing for ssize_t, intmax_t, or to compare int to int32_t. I think a better heuristic would be to not emit a warning for any integral (and floating point?) ty

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

2023-07-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D155064#4523024 , @hazohelet wrote: > In D155064#4514893 , @cor3ntin > wrote: > >> This looks good to me modulo nitpicks. >> When you land that, please make an issue on github for th

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

2023-07-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D154696#4525100 , @nathanchance wrote: > The error added by this patch triggers on some recently added code to the > Linux kernel's -next tree, which I failed to test above due to its generally > unstable nature: > > driv

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

2023-07-23 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:101-112 + for (;;) { +const Pointer &ElemPtr = StrPtr.atIndex(I); + +if (!CheckRange(S, OpPC, ElemPtr, AK_Read)) + return false; + +uint8_t Val = ElemPtr.deref();

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

2023-07-23 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/vartemplate-lambda.cpp:17 +// expected-note{{cannot be used in a constant expression}} \ +// expected-er

[PATCH] D155707: [clang][Interp] Handle CXXNoexceptExprs

2023-07-23 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/AST/Interp/literals.cpp:1037-1045 +namespace NE { + constexpr int foo() noexcept { +return 1; + } + static_assert(noexcept(foo()), ""); + constexpr int foo2() { +return 1; Can you either add tests

[PATCH] D156045: [clang][Interp] Enable existing source_location tests

2023-07-23 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/test/SemaCXX/source_location.cpp:764-768 +#if defined(USE_CONSTEVAL) && !defined(NEW_INTERP) static_assert(test_init_capture() == __LINE__ - 4); #

[PATCH] D155714: [clang] Fix diagnostics for defaulted, implicitly deleted 'operator=='.

2023-07-23 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. Thanks for the patch, it looks good to me. I'll commit on your behalf Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155714/new/ https://revi

[PATCH] D155714: [clang] Fix diagnostics for defaulted, implicitly deleted 'operator=='.

2023-07-23 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 rG02bb2beeef3d: [clang] Fix diagnostics for defaulted, implicitly deleted 'operator=='. (authored by AMP999, committed by cor3ntin). Changed prior to

[PATCH] D156053: [Clang] Fix crash in CIndex, when visiting a static_assert without message

2023-07-23 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/tools/libclang/CIndex.cpp:1297 return true; - if (auto *Message = dyn_cast(D->getMessage())) + if (auto *Message = dyn_cast_if_present(D->getMessage())) if (Visit(MakeCXCursor(Message, StmtParent, TU, RegionOfInterest)

[PATCH] D156063: [Clang] Reject programs declaring namespace std to be inline

2023-07-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Can you add a test for `foo::std` ? I suspects it warns, which is incorrect ? Can you add additional tests for `inline std::foo` ? (which should warn) Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156063/new/ htt

[PATCH] D156053: [Clang] Fix crash in CIndex, when visiting a static_assert without message

2023-07-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. There is a failed unit test, can you look into it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156053/new/ https://reviews.llvm.org/D156053 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

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

2023-07-24 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/vartemplate-lambda.cpp:17 +// expected-note{{cannot be used in a constant expression}} \ +// expected-er

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

2023-07-24 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/D154716/new/ https://reviews.llvm.org/D154716 __

<    3   4   5   6   7   8   9   10   11   12   >