[PATCH] D104975: Implement P1949

tahonermann added a comment. > Since it is a C++23 feature, can we at least have it only if the user > requests C++23 standard compliance? WG21 adopted P1949 as a defect report (DR) against prior standards with the intent that the changes be applied retroactive

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

tahonermann added inline comments. Comment at: clang/lib/Lex/PPExpressions.cpp:413 Val.setIsUnsigned(!TargetInfo::isTypeSigned(TI.getWCharType())); -else if (!Literal.isUTF16() && !Literal.isUTF32()) +else if (!Literal.isUTF8() && !Literal.isUTF16() && !Literal.isU

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

tahonermann added a comment. Thanks for your continued work on this, Tim! I think this is close. I did spot one issue and added a few other comments. Comment at: clang/lib/Lex/PPExpressions.cpp:417-418 +else if (Literal.isUTF8()) + Val.setIsUnsigned(PP.getLangOpts().C

[PATCH] D124719: [docs] PCH usage documentation update

tahonermann closed this revision. tahonermann added a comment. Re-closing since this review has already landed. @akyrtzi, did you see my last comment? If I don't hear back from you, I'll submit a new doc change that covers both forms of PCH use. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D129373: [NFC] Minor cleanup of usage of FloatModeKind with bitmask enums

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good, thank you for following up @jolanta.jensen! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129373/new/ https://reviews.llvm

[PATCH] D129373: [NFC] Minor cleanup of usage of FloatModeKind with bitmask enums

tahonermann added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:894 bool useObjCFPRetForRealType(FloatModeKind T) const { -return RealTypeUsesObjCFPRetMask & llvm::BitmaskEnumDetail::Underlying(T); +return (int)((FloatModeKind)RealTypeUsesObjCFPRe

[PATCH] D129689: [limits.h] USHRT_MAX fix for 16 bit architectures

tahonermann added a comment. > Sorry about this, I actually tried this but the patch showed a lot more than > what I've changed, not sure what I did wrong. You didn't do anything wrong; the `-U9` option will effectively include the entire file for each changed file. That is actually the int

[PATCH] D127363: [Lex] Fix for char32_t literal truncation on 16 bit architectures

tahonermann added a comment. >> Additionally, the type of a character constant in C is int. > > This means that char32_t c4 = U'\U00064321'; is invalid in C No. A character constant that does not have an encoding prefix has type `int` in C. Character constants that have an encoding prefix have t

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

tahonermann added a comment. I'm not following what the root problem is. You stated: > When we write the attribute preferred_name(foo) in ASTWriter, the compiler > would try to write the type for the argument foo. Then when the compiler > write the type for foo, the compiler find the type for f

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

tahonermann added a comment. Thank you for the detailed explanation, @ChuanqiXu, that was very helpful. It looks to me like the problem may be that the initial declaration of the `basic_string_view` class template is non-defining, but when serializing that declaration, we serialize a definition

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

tahonermann added a comment. I neglected to explicitly mention in conjunction with my last comment, but @ChuanqiXu, can you check to see if we are indeed serializing class template specializations "too early" and, if so, whether delaying such serialization until a defining point resolves the is

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

tahonermann added a comment. > Personally, the problem is not serializing class template specializations > "too early". The problem is that we are deserializing class template > specializations "too early". Yes, I agree. > The key point here is that Modules would read declarations lazily for

[PATCH] D130331: [C++20] [Modules] Disable preferred_name when writing a C++20 Module interface

tahonermann added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2925-2926 for (unsigned I = 0, E = readInt(); I != E; ++I) -Attrs.push_back(readAttr()); +if (auto *Attr = readAttr()) + Attrs.push_back(Attr); } Interesting

[PATCH] D125167: [WIP] Fix member access of anonymous struct/union fields in C

tahonermann added inline comments. Comment at: clang/docs/ReleaseNotes.rst:152-157 +- When forming a member expression, now consider any qualifiers written on an + anonymous structure or union as also applying to the field being referenced. + This fixes an issue where qualifier

[PATCH] D125167: [WIP] Fix member access of anonymous struct/union fields in C

tahonermann added inline comments. Comment at: clang/test/Sema/anonymous-struct-union.c:137-140 + // It's the access path that picks up the qualifiers, not the direct + // declaration of the field itself. So 'i' and 'j' are both 'int'. + _Static_assert(_Generic(x.i, int : 1, d

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

tahonermann added inline comments. Comment at: clang/test/Lexer/utf8-char-literal.cpp:5-7 +// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++17 -fsyntax-only -fchar8_t -DCHAR8_T -verify %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++20 -fsyntax-only -verify %s

[PATCH] D125259: [C11] Diagnose unreachable generic selection associations

tahonermann accepted this revision. tahonermann added a comment. Looks good to me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125259/new/ https://reviews.llvm.org/D125259 ___ cfe-commits mailing list

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

tahonermann added inline comments. Comment at: clang/test/Lexer/utf8-char-literal.cpp:3-4 // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c2x -x c -fsyntax-only -verify %s -// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++1z -fsyntax-only -verify %s +// RUN: %clang_c

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

tahonermann added inline comments. Comment at: clang/test/Lexer/utf8-char-literal.cpp:54-56 +# if !(u8'\xff' == 0xff) +#error u8 char literal is not unsigned +# endif The C++ case looks good now, but the condition doesn't look right for the C case. The exp

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

tahonermann added inline comments. Comment at: clang/test/Lexer/utf8-char-literal.cpp:54-56 +# if !(u8'\xff' == 0xff) +#error u8 char literal is not unsigned +# endif tbaeder wrote: > tahonermann wrote: > > The C++ case looks good now, but the condition doe

[PATCH] D124996: [clang][preprocessor] Fix unsigned-ness of utf8 char literals

tahonermann accepted this revision as: tahonermann. tahonermann added a comment. This revision is now accepted and ready to land. Looks good, @tbaeder! Thank you for sticking with me through all these iterations! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124996/new/ https://reviews

[PATCH] D125259: [C11] Diagnose unreachable generic selection associations

tahonermann added a comment. > I'm questioning the benefit to supporting _Generic in C++ and starting to > wonder if perhaps we should pull this extension back. I imagine it may be used in shared system headers. E.g., implementations of `tgmath.h` that don't use the builtins. Repository: rG

[PATCH] D125882: Correct the diagnostic behavior for unreachable _Generic associations in C++

tahonermann accepted this revision. tahonermann added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125882/new/ https://reviews.llvm.org/D125882 ___ cfe-commits mailing list cfe-commits@

[PATCH] D125773: [Driver] Do not auto-enable header modules with -std=c++20

tahonermann added a comment. > So the proposal is that -fheader-modules=parse would parse #include of header > unit in the same TU, and import .pcm on import, right? Per cpp.includep7 , "If the header identified by the //header-name// denotes an importable

[PATCH] D72053: [RFC] Handling implementation limits

tahonermann added inline comments. Comment at: clang/docs/ImplementationQuantities.rst:11 +This page lists the limits implemented in the Clang compiler. The available +resources on the system running Clang may imposse other limits. For example, +the system may have insufficient m

[PATCH] D93031: Enable fexec-charset option

tahonermann added inline comments. Comment at: clang/include/clang/Driver/Options.td:3583-3584 +def fexec_charset : Separate<["-"], "fexec-charset">, MetaVarName<"">, + HelpText<"Set the execution for string and character literals">; def target_cpu : Separate<["-"], "target

[PATCH] D93031: Enable fexec-charset option

tahonermann added inline comments. Comment at: clang/include/clang/Driver/Options.td:3583-3584 +def fexec_charset : Separate<["-"], "fexec-charset">, MetaVarName<"">, + HelpText<"Set the execution for string and character literals">; def target_cpu : Separate<["-"], "target

[PATCH] D93031: Enable fexec-charset option

tahonermann added inline comments. Comment at: clang/test/CodeGen/systemz-charset.c:4 + +char *UpperCaseLetters = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; +// CHECK: c"\C1\C2\C3\C4\C5\C6\C7\C8\C9\D1\D2\D3\D4\D5\D6\D7\D8\D9\E2\E3\E4\E5\E6\E7\E8\E9\00" `const char *` please

[PATCH] D93031: Enable fexec-charset option

tahonermann added inline comments. Comment at: clang/include/clang/Lex/LiteralSupport.h:244 bool Pascal; + ConversionState TranslationState; + abhina.sreeskantharajan wrote: > tahonermann wrote: > > Same concern here with respect to persisting the conversion

[PATCH] D93031: Enable fexec-charset option

tahonermann added a comment. Hi, Abhina. Sorry for the delay getting back to you. I added some more comments. Comment at: clang/include/clang/Lex/LiteralSupport.h:191 +Preprocessor &PP, tok::TokenKind kind, +ConversionState Translation

[PATCH] D136120: [Clang] follow-up D128745, remove ClangABICompat checks

tahonermann added a comment. The code changes look good to me. I offered a suggested rewording of the release note. Comment at: clang/docs/ReleaseNotes.rst:499-501 +- Implemented DR692, DR1395 and DR1432. Note that the fix for DR1432 is speculative + that there is no wording

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

tahonermann added a comment. > In a pre-scanned world, the build system does know the info for each source > file (published and dependent modules) [which ought to dispel some of the > concerns raised about not knowing about possible outputs for > implementation/interface cases]. > > In a disco

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

tahonermann added a comment. > Give some time for Aaron/Tom to have the chance to raise further comment but > I'm happy with it otherwise. I'm sorry for the delay getting back to this and I see this has already been pushed (and a fix then pushed). The changes look good to me too. Repository:

[PATCH] D135919: [Clang] Correct when Itanium ABI guard variables are set for non-block variables with static or thread storage duration.

tahonermann added a comment. > Please add a comment something like this: Done. Thank you for the prior review comments. This is again ready for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135919/new/ https://reviews.llvm.org/D135919 ___

[PATCH] D135919: [Clang] Correct when Itanium ABI guard variables are set for non-block variables with static or thread storage duration.

tahonermann updated this revision to Diff 472050. tahonermann retitled this revision from "[Clang] Set thread_local Itanium ABI guard variables before calling constructors." to "[Clang] Correct when Itanium ABI guard variables are set for non-block variables with static or thread storage duratio

[PATCH] D135919: [Clang] Correct when Itanium ABI guard variables are set for non-block variables with static or thread storage duration.

tahonermann updated this revision to Diff 472306. tahonermann added a comment. Rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135919/new/ https://reviews.llvm.org/D135919 Files: clang/lib/CodeGen/ItaniumCXXABI.cpp clang/test/CodeGenCXX

[PATCH] D136554: Implement CWG2631

tahonermann added a comment. > There is a bug with default parameter in blocks so that can't be tested Block expressions don't actually support default arguments due to the type erasure that is fundamental to how they are implemented. Clang should diagnose default arguments in block expressions

[PATCH] D132111: [clang][Interp] Implement pointer (de)ref operations and DeclRefExprs

tahonermann added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:613-614 case UO_LNot: // !x +if (!this->Visit(SubExpr)) + return false; return this->emitInvBool(E); I don't love that the `Visit()` calls are now repeated fo

[PATCH] D132111: [clang][Interp] Implement pointer (de)ref operations and DeclRefExprs

tahonermann added inline comments. Comment at: clang/test/AST/Interp/cxx20.cpp:2 +// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s +// RUN: %clang_cc1 -std=c++20 -verify %s -DREFERENCE + tbaeder wrote: > tahonermann wrote: > > Is

[PATCH] D132111: [clang][Interp] Implement pointer (de)ref operations and DeclRefExprs

tahonermann added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:662 + return this->emitGetPtrParam(It->second, E); + } + tbaeder wrote: > tahonermann wrote: > > Perhaps add: > > else { > > assert(0 && "Unhandled declaration kin

[PATCH] D132111: [clang][Interp] Implement pointer (de)ref operations and DeclRefExprs

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Thanks, Timm. The changes look reasonable to me, so I'll accept. Given my unfamiliarity with this code though, it would be great if someone else also gave it a nod. CHANGES SINCE L

[PATCH] D132913: [HLSL] Preserve vec3 for HLSL.

tahonermann added inline comments. Comment at: clang/test/CodeGenHLSL/float3.hlsl:1 +// RUN: %clang --driver-mode=dxc -Tlib_6_7 -fcgl -Fo - %s | FileCheck %s + Does this need to use driver mode? That is odd for a code gen test. Repository: rG LLVM Github Mon

[PATCH] D133807: Update Unicode to 15.0

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Thank you, Corentin, and apologies for the delayed review. This looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133807

[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

tahonermann added a comment. This change appears to be responsible for the following test case now being rejected. That seems right and is consistent with the intent, but use of `-fclang-abi-compat=14` doesn't seem to restore the prior behavior. https://godbolt.org/z/ad1TPjTza struct map {

[PATCH] D134874: [Concepts] Fix Concepts on generic lambda in a VarTemplateSpecDecl

tahonermann added a comment. The refactoring looks like a good improvement to me. I think there may be some opportunities to make it more robust. I did some spot checking that previous behavior is retained, but didn't exhaustively do such checking; it does look like many of the changes were mec

[PATCH] D133499: [clang]: Add DeclContext::dumpAsDecl().

tahonermann updated this revision to Diff 464487. tahonermann retitled this revision from "[clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext." to "[clang]: Add DeclContext::dumpAsDecl().". tahonermann edited the summary of this revision. tahonermann adde

[PATCH] D133499: [clang]: Add DeclContext::dumpAsDecl().

tahonermann updated this revision to Diff 464488. tahonermann marked 9 inline comments as done. tahonermann added a comment. Addressed a suggested edit I originally failed to notice. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133499/new/ https:/

[PATCH] D133499: [clang]: Add DeclContext::dumpAsDecl().

tahonermann marked an inline comment as done. tahonermann added inline comments. Comment at: clang/lib/AST/ASTDumper.cpp:203 +LLVM_DUMP_METHOD void DeclContext::dumpDecl() const { + if (const Decl *D = dyn_cast(this)) shafik wrote: > Interesting ` DeclContext:

[PATCH] D133500: [clang] Correct handling of lambdas in lambda default arguments in dependent contexts.

tahonermann updated this revision to Diff 464489. tahonermann added a comment. Rebased and removed redundant asserts pointed out in code review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133500/new/ https://reviews.llvm.org/D133500 Files: cl

[PATCH] D133500: [clang] Correct handling of lambdas in lambda default arguments in dependent contexts.

tahonermann marked 5 inline comments as done. tahonermann added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1159 +Expr *UninstExpr = PVD->getUninstantiatedDefaultArg(); +// FIXME: Obtain the source location for the '=' token. +S

[PATCH] D134874: [Concepts] Fix Concepts on generic lambda in a VarTemplateSpecDecl

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. I think this looks good. I took more time to compare with the current code and it looks to me like behavioral consistency is maintained where desired. I added one comments requesting

[PATCH] D134874: [Concepts] Fix Concepts on generic lambda in a VarTemplateSpecDecl

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:255-258 +/// \param ForConstraintInstantiation when collecting arguments, +/// ForConstraintInstantiation indicates we should continue looking when +/// encountering a lambda generic call op

[PATCH] D133499: [clang]: Add DeclContext::dumpAsDecl().

tahonermann closed this revision. tahonermann marked 2 inline comments as done. tahonermann added a comment. Pushed, but I forgot to add the `Differential Revision` tag to the commit, so closing manually. The changes landed as commit 4247cdb568eca4c31b14d91105fe5ee140225036

[PATCH] D135161: [clang][Lex] Fix a crash on malformed string literals

tahonermann added subscribers: cor3ntin, tahonermann. tahonermann added a reviewer: cor3ntin. tahonermann added a comment. I think this looks right, but it would be good for @cor3ntin to take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135

[PATCH] D133500: [clang] Correct handling of lambdas in lambda default arguments in dependent contexts.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. tahonermann marked an inline comment as done. Closed by commit rG4409a83c2935: [clang] Correct handling of lambdas in lambda default arguments in dependent… (authored b

[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

tahonermann added a comment. Is it known what gcc and Microsoft maintainers are going to do with these DRs? I don't see any recent changes in gcc, but the following is interesting... For the test below, Clang has historically accepted it but now rejects it. MSVC has historically rejected it, bu

[PATCH] D135500: [Clang] reject bit-fields as instruction operands in Microsoft style inline asm blocks.

tahonermann created this revision. tahonermann added reviewers: erichkeane, aaron.ballman. Herald added a project: All. tahonermann requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. MSVC allows bit-fields to be specified as instruction operan

[PATCH] D135500: [Clang] reject bit-fields as instruction operands in Microsoft style inline asm blocks.

tahonermann updated this revision to Diff 466589. tahonermann added a comment. Rebased and addressed code review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135500/new/ https://reviews.llvm.org/D135500 Files: clang/docs/ReleaseNotes.

[PATCH] D135500: [Clang] reject bit-fields as instruction operands in Microsoft style inline asm blocks.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0982db188b66: [Clang] reject bit-fields as instruction operands in Microsoft style inline asm… (authored by tahonermann). Repository: rG LLVM Gith

[PATCH] D135500: [Clang] reject bit-fields as instruction operands in Microsoft style inline asm blocks.

tahonermann marked 2 inline comments as done. tahonermann added inline comments. Comment at: clang/lib/Sema/SemaStmtAsm.cpp:937 for (uint64_t I = 0; I < NumOutputs + NumInputs; ++I) { -if (Exprs[I]->getType()->isBitIntType()) - return StmtError( - Diag(Exprs[

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

tahonermann added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:445-446 +const CharacterLiteral *E) { + if (Optional T = classify(E->getType())) +return this->emitConst(E, E->getValue()); + `T` isn't used and `classify()` already

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

tahonermann added inline comments. Comment at: clang/test/AST/Interp/arrays.cpp:143 + +}; cor3ntin wrote: > tbaeder wrote: > > cor3ntin wrote: > > > tahonermann wrote: > > > > As others already noted, additional testing of multicharacter literals > > > > and UCN

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

tahonermann added inline comments. Comment at: clang/test/AST/Interp/arrays.cpp:143 + +}; tbaeder wrote: > cor3ntin wrote: > > tahonermann wrote: > > > cor3ntin wrote: > > > > tbaeder wrote: > > > > > cor3ntin wrote: > > > > > > tahonermann wrote: > > > > > > > A

[PATCH] D135919: [Clang] Set thread_local Itanium ABI guard variables before calling constructors.

tahonermann created this revision. tahonermann added reviewers: erichkeane, aaron.ballman, hubert.reinterpretcast, rjmccall. Herald added a project: All. tahonermann requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously, for thread_lo

[PATCH] D134507: [Clang] add missing ClangABICompat check

tahonermann added a comment. > Per further discussions in D128745 , this > is not needed. I agree that the code change is not needed. I think it is worth keeping the test though (modified to verify the expected ambiguity). Repository: rG LLVM Github Monorep

[PATCH] D135919: [Clang] Set thread_local Itanium ABI guard variables before calling constructors.

tahonermann added a comment. > the case from https://github.com/llvm/llvm-project/issues/57828 is not for a > block-scope variable, but the case in the patch description is... Thanks, Hubert. Yes, I found that the reported issue occurred for any case where thread safe guard variables are not re

[PATCH] D134269: [docs] Document the one-phase style compilation to c++ standard modules

tahonermann added inline comments. Comment at: clang/docs/StandardCPlusPlusModules.rst:228-229 + +Also if when we compile an importable module unit to an object file, +the compiler will generate the corresponding BMI implicitly. + When an importable module unit i

[PATCH] D134507: [Clang] add missing ClangABICompat check

tahonermann added a comment. > Agreed. It turns out the ambiguous case is already in place. > (https://github.com/llvm/llvm-project/blob/d8af31ecede0c54ec667ab687784149e806c9e4c/clang/test/CXX/drs/dr6xx.cpp#L1104) Ah, perfect! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D140598: [Clang] Add sanity check in Sema::getDestructorName to prevent nullptr dereference

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:393 // reasonably apply this fallback for dependent nested-name-specifiers. -if (SS.getScopeRep()->getPrefix()) { +if (SS.isSet() && SS.getScopeRep()->getPrefix()) { if (ParsedType T

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

tahonermann added a comment. > I don't think this patch is no longer necessary given that we merged the math > profile I agree; we can revisit this if complaints from users due to use of characters not in the math profile materializes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good! Sorry, Corentin, this review had fallen off my radar! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139889/new/ https://re

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

tahonermann added a comment. I don't know the driver code very well, but as best I can tell, this appears to match the design agreed to at the last Clang Modules WG meeting. Comment at: clang/test/Driver/module-output.cppm:5 +// +// Tests that the .pcm file will be generated i

[PATCH] D140984: [clang] Correct -frewrite-includes generation of line control directives with mixed EOL forms.

tahonermann created this revision. tahonermann added reviewers: rnk, cor3ntin, shafik, erichkeane, aaron.ballman. Herald added a project: All. tahonermann requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously, if a header file and a so

[PATCH] D140984: [clang] Correct -frewrite-includes generation of line control directives with mixed EOL forms.

tahonermann added inline comments. Comment at: clang/lib/Frontend/Rewrite/InclusionRewriter.cpp:292-303 // Output the file one line at a time, rewriting the line endings as we go. StringRef Rest = TextToWrite; while (!Rest.empty()) { - StringRef LineText; -

[PATCH] D140984: [clang] Correct -frewrite-includes generation of line control directives with mixed EOL forms.

tahonermann updated this revision to Diff 486384. tahonermann added a comment. Addressed Corentin's review feedback. Thank you, Corentin! I'm going to remember to update the release notes doc all on my own one of these days, I'm sure of it! :) Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D140984: [clang] Correct -frewrite-includes generation of line control directives with mixed EOL forms.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3b1d45518910: [clang] Correct -frewrite-includes generation of line control directives with… (authored by tahonermann). Repository: rG LLVM Github

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

tahonermann added a comment. > Header units have even more need to be involved in the build graph than named > units. ODR violations and cache invalidation problems await anyone just > winging it on header units (at least that's the understanding I've gotten > from SG15 meetings). I think that

[PATCH] D135919: [Clang] Correct when Itanium ABI guard variables are set for non-block variables with static or thread storage duration.

tahonermann added a comment. @rjmccall, could you please take a look at the updated changes? I think I've addressed the issues you raised. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135919/new/ https://reviews.llvm.org/D135919

[PATCH] D137488: [clang][Interp] Array initialization via string literal

tahonermann added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1098-1099 + +unsigned N = SL->getLength(); +for (size_t I = 0; I != NumElems; ++I) { + uint32_t CodePoint = I < N ? SL->getCodeUnit(I) : 0; Aren't `N` and `NumEle

[PATCH] D137826: [clang] Allow comparing pointers to string literals

tahonermann added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12951-12954 + // ObjC's @encode() + if (isa(E->getLHS()->IgnoreParenImpCasts()) || + isa(E->getRHS()->IgnoreParenImpCasts())) return Error(E); A comment to e

[PATCH] D135919: [Clang] Correct when Itanium ABI guard variables are set for non-block variables with static or thread storage duration.

This revision was automatically updated to reflect the committed changes. Closed by commit rG3e25ae605edd: [Clang] Correct when Itanium ABI guard variables are set for non-block… (authored by tahonermann). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D142639: [clang] Warn by default that implicit capture of 'this' is deprecated in C++20 and later.

tahonermann created this revision. tahonermann added reviewers: aaron.ballman, erichkeane, rsmith, shafik. Herald added a project: All. tahonermann requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously, a warning that C++20 deprecated

[PATCH] D142639: [clang] Warn by default that implicit capture of 'this' is deprecated in C++20 and later.

tahonermann added a comment. > You'll need a rebase to get the pre-commit CI to work. Yes, thank you. I'll wait for https://reviews.llvm.org/D142578 to land and then rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142639/new/ https://review

[PATCH] D142639: [clang] Warn by default that implicit capture of 'this' is deprecated in C++20 and later.

tahonermann updated this revision to Diff 493955. tahonermann added a comment. Rebased. This now targets Clang 17. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142639/new/ https://reviews.llvm.org/D142639 Files: clang/docs/ReleaseNotes.rst cl

[PATCH] D142639: [clang] Warn by default that implicit capture of 'this' is deprecated in C++20 and later.

This revision was automatically updated to reflect the committed changes. Closed by commit rGecd0be100bed: [clang] Warn by default that implicit capture of 'this' is deprecated in C++20… (authored by tahonermann). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D143109: [Sema] Push a LambdaScopeInfo before calling SubstDefaultArgument

tahonermann added a comment. > I'm still don't understand what the problem is about cleaning up the lambda > scope. Me either. It looks to me like the pushed lambda scope will get properly cleaned up. Incidentally, it looks like that LSI copy (https://github.com/llvm/llvm-project/blob/main/cl

[PATCH] D143109: [Sema] Push a LambdaScopeInfo before calling SubstDefaultArgument

tahonermann added a comment. > Incidentally, it looks like that LSI copy > (https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/TreeTransform.h#L13469-L13478) > has not been needed since commit bf5fe2dbba0899bee4323f5eaa075acc43a18e2e > (see > https://github.com/llvm/llvm-project/blo

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

tahonermann added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5736 + C.getArgs().hasArg(options::OPT_fmodule_output) && + C.getArgs().hasArg(options::OPT_o)) { +SmallString<128> OutputPath; dblaikie wrote: > ChuanqiXu wrote: > > dblaiki

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:377-380 + SubstitutedExpression = ImplicitCastExpr::Create( + S.Context, SubstitutedExpression.get()->getType(), + CK_LValueToRValue, SubstitutedExpression.get(), +

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:377-380 + SubstitutedExpression = ImplicitCastExpr::Create( + S.Context, SubstitutedExpression.get()->getType(), + CK_LValueToRValue, SubstitutedExpression.get(), +

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

tahonermann accepted this revision as: tahonermann. tahonermann added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: clang/lib/Sema/SemaConcept.cpp:377-380 + SubstitutedExpression = ImplicitCastExpr::Create( + S.C

[PATCH] D126479: [Clang] Allow 'Complex float __attribute__((mode(HC)))'

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/TargetInfo.h:223-224 unsigned HasAlignMac68kSupport : 1; - unsigned RealTypeUsesObjCFPRet : 3; + unsigned R

[PATCH] D126479: [Clang] Allow 'Complex float __attribute__((mode(HC)))'

tahonermann added a comment. The code changes and test updates all look good to me. Please add a release note to `clang/docs/ReleaseNotes.rst`; I'll be happy to approve after that is done. It would be great to get an additional approval on this; I'm not an expert here. Repository: rG LLVM

[PATCH] D126479: [Clang] Allow 'Complex float __attribute__((mode(HC)))'

tahonermann added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:885 bool useObjCFPRetForRealType(FloatModeKind T) const { +assert(T <= FloatModeKind::Last); return RealTypeUsesObjCFPRet & (1 << (int)T); aaron.ballman wrote: > Thi

[PATCH] D126479: [Clang] Allow 'Complex float __attribute__((mode(HC)))'

tahonermann added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:223-224 unsigned HasAlignMac68kSupport : 1; - unsigned RealTypeUsesObjCFPRet : 3; + unsigned RealTypeUsesObjCFPRet : (1 << (int)FloatModeKind::Float) | + (

[PATCH] D126479: [Clang] Allow 'Complex float __attribute__((mode(HC)))'

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good to me! Thanks Jolanta! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126479/new/ https://reviews.llvm.org/D126479

[PATCH] D127363: [Lex] Fix for char32_t literal truncation on 16 bit architectures

tahonermann added inline comments. Comment at: clang/lib/Lex/LiteralSupport.cpp:1600 - llvm::APInt LitVal(PP.getTargetInfo().getIntWidth(), 0); + llvm::APInt LitVal(PP.getTargetInfo().getChar32Width(), 0); sammccall wrote: > tahonermann wrote: > > I don't t

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

tahonermann added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:116-117 "source file is not valid UTF-8">; +def warn_invalid_utf8_in_comment : Warning< + "invalid UTF-8 in comment">, InGroup>, DefaultIgnore; def err_character_not_allowed : Err

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

tahonermann added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2405-2406 // Skip over characters in the fast loop. -while (C != 0 &&// Potentially EOF. - C != '\n' && C != '\r') // Newline or DOS-style newline. +// Warn on invalid U

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

tahonermann accepted this revision as: tahonermann. tahonermann added a comment. This revision is now accepted and ready to land. > @tahonermann gentle ping (Aaron told me you might have further comments) I'm sorry for the delay. I ran out of time to do the thorough review I would have liked to

<    1   2   3   4   >