[PATCH] D151606: [NFC][CLANG] Fix Static Code Analyzer Concerns with bad bit right shift operation in getNVPTXLaneID()

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. The static analysis results look suspect since there does not appear to be a way for `Log2_32` to return a value larger than 31. Regardless, the change looks safe to me; `GV_Warp_Siz

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. Sorry for the long delay reviewing again. I noted some mostly minor issues. I'm on vacation next week, but I'll try to watch for updates. I'm still concerned that the chang

[PATCH] D146412: [NFC] Fix potential for use-after-free in DumpModuleInfoAction

tahonermann accepted this revision. tahonermann added a comment. Thank you, Mariya! Looks good to me! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146412/new/ https://reviews.llvm.org/D146412 ___ cfe-co

[PATCH] D143099: [clang][lex] Expose findBeginningOfLine()

tahonermann added a comment. Sorry for the delayed response. I managed to miss seeing this review request. Comment at: clang/include/clang/Lex/Lexer.h:598-599 + /// Returns the pointer that points to the beginning of line that contains + /// the given offset, or null if the

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

tahonermann added subscribers: cor3ntin, tahonermann. tahonermann added a comment. Adding Corentin for awareness. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144331/new/ https://reviews.llvm.org/D144331 __

[PATCH] D144331: [libc++][format] Implements formatter thread::id.

tahonermann added a comment. > It adds an include of #include <__format/formatter_integral.h> which ends up > including which has internal definitions of isupper/islower causing > clang to complain. > > Any suggestions on what would be the right fix here? It sounds like `safe-ctype.h` should a

[PATCH] D119291: [Clang] Add support for STDC CX_LIMITED_RANGE pragma.

tahonermann resigned from this revision. tahonermann added a comment. Herald added a project: All. Resigning to remove this old review from my review queue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119291/new/ https://reviews.llvm.org/D119291

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

tahonermann added a comment. Thanks for the update, @ahatanak. Please note that myself and Corentin are both preoccupied with the WG21 meeting this week, so it might be a while before either of us is able to take a good look at your update. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D143670: Stop claiming we support [[carries_dependency]]

tahonermann added a comment. > The guidance from EWG this week and in the past was that we are always > required to 'parse and diagnose appertainment' of standard attributes, but > not to enable __has_cpp_attribute unless we actually 'do' something with it. > I intend/suggest we add a condition

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

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Sema/SemaExpr.cpp:19085-19092 + // If the variable is used in a default argument expression of a lambda call + // operator, sw

[PATCH] D144334: [Clang] Add C++2b attribute [[assume(expression)]]

tahonermann added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1448 +def Assume : StmtAttr { + let Spellings = [CXX11<"", "assume", 202302>]; + let Documentation = [AssumeDocs]; philnik wrote: > Izaron wrote: > > Honestly I don't have a clue w

[PATCH] D143142: [clang][lex] Enable Lexer to grow its buffer

tahonermann added a comment. The changes made (from what I've seen, I haven't reviewed every line) make sense to me. The amount of change does make me a bit nervous though. In D143142#4142212 , @aaron.ballman wrote: > In terms of whether to use `unsign

[PATCH] D149461: [NFC] ][CLANG] Fix static code analyzer concerns

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. These changes look good to me and I agree with not making a change for the `KnownHeaders` case. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1131-1132

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. This looks good with one exception; I think the changes for class `SemaDiagnosticBuilder` are not what was intended. Comment at: clang/include/clang/Anal

[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by values

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good. Thanks again for catching our oops @HazardyKnusperkeks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149647/new/ https://reviews.llvm.org/D149647 _

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Sema/Sema.h:1786 SemaDiagnosticBuilder(SemaDiagnosticBuilder &&D); +SemaDiagnosticBuilder &operator=(SemaDiag

[PATCH] D150140: [NFC][CLANG] Fix Static Code Analysis Concerns

tahonermann added inline comments. Comment at: clang/utils/TableGen/SveEmitter.cpp:302 unsigned Shift = llvm::countr_zero(Mask); + assert(Shift >= 64 && "Shift is out of encodable range"); return (V << Shift) & Mask; Manna wrote: > sdesmalen wr

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names and introduce Bfloat16 arithmetic type.

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. I reviewed about a third of this, but then stopped due to the `__bf16` vs `std::bfloat16_t` naming issues. I think the existing names that use "bfloat16" to support the `__

[PATCH] D150140: [NFC][CLANG] Fix Static Code Analysis Concerns

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good to me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150140/new/ https://reviews.llvm.org/D150140 ___ cfe-commits mai

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good. Thanks @Manna! Comment at: clang/include/clang/Sema/Sema.h:1786 SemaDiagnosticBuilder(SemaDiagnosticBuilder &&D); +SemaDiagnosticBuilder &opera

[PATCH] D150291: [Clang] Rename internal type identifier(s) for `__bf16` to `BF16Ty`

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. This looks great, thank you for doing this! Requested changes are just to undo some of the style changes. Comment at: clang/include/clang/Basic/Specifier

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

tahonermann added subscribers: lenary, foad. tahonermann added a comment. > I was following the LLVM contribution guidelines to use git clang-format, but > I understand the importance of maintaining existing code styles that may be > altered by git-clang format. The guidelines are slightly in c

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

tahonermann added a subscriber: clang-vendors. tahonermann added a comment. Heads up @clang-vendors. This patch changes the names of many internal identifiers for enumerators, class data members, and functions (including virtual functions) that are related to support for the `__bf16` type. The

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Thanks for all the updates @codemzs! I'm going to go ahead and accept. But please wait a few days for recently subscribed folks to have a chance to comment before landing this. Rep

[PATCH] D150291: [Clang] Rename internal type identifier(s) for __bf16 to BF16Ty

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. > I do wonder if we need two bfloat implementations, but for that I'll leave a > comment on D149573 . Given the discussions occurring in D

[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. I added a few comments and suggested edits, but this is mostly looking good. Comment at: clang/include/clang/Sema/Sema.h:1790 SemaDiagnosticBuilder(c

[PATCH] D150744: [NFC][CLANG] Fix uninitialized scalar field issues found by Coverity

tahonermann added a comment. I'm not opposed to these changes, but please note that these changes mean is will no longer be possible to use ubsan to discover when these data members are used before having been assigned an appropriate value. That is only a concern when an appropriate value would

[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

tahonermann added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1790 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default; +SemaDiagnosticBuilder &operator=(const SemaDiagnosticBuilder &) = delete; ~SemaDiagnosticBuilder();

[PATCH] D150744: [NFC][CLANG] Fix uninitialized scalar field issues found by Coverity

tahonermann added inline comments. Comment at: clang/include/clang/Parse/Parser.h:1190 class ParseScopeFlags { Scope *CurScope; +unsigned OldFlags = 0; shafik wrote: > @tahonermann I feel like we should have a default member initializer for any > mem

[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

tahonermann added a comment. One last minor request and then I'm happy with this. Comment at: clang/include/clang/Sema/ParsedAttr.h:705-707 + // The move assignment operator is defined as deleted pending further + // motivation. + AttributePool &operator=(AttributePool &&poo

[PATCH] D150843: [clang][Diagnostics] Refactor printableTextForNextCharacter

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. Splitting this into two patches (one to do the renames, another to perform the other changes) would simplify review. Comment at: clang/lib/Frontend/TextD

[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Thanks @Manna, this looks good to me now! Comment at: clang/include/clang/Sema/ParsedAttr.h:705-707 + // The move assignment operator is defined as deleted pending

[PATCH] D148639: [NFC][clang] Fix static analyzer concerns about AUTO_CAUSES_COPY

tahonermann added a comment. After seeing this review, I filed a support case with Synopsys with a request to stop reporting `AUTO_CAUSES_COPY` issues for small objects of class types, at least in the absence of additional evidence that a reference would be preferred. Commen

[PATCH] D148639: [NFC][clang] Fix static analyzer concerns about AUTO_CAUSES_COPY

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. This set of changes looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148639/new/ https://reviews.llvm.org/D148639 ___

[PATCH] D148812: [NFC][clang] Fix static analyzer concerns

tahonermann added a comment. With the exception of the case involving the `Policy` class, these changes all look fine to me. Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:562 if (UnMaskedPolicyScheme != PolicyScheme::SchemeNone) - for (auto P : Supported

[PATCH] D148812: [NFC][clang] Fix static analyzer concerns

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Thanks, Soumi! Looks good to me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148812/new/ https://reviews.llvm.org/D148812 ___

[PATCH] D149074: [NFC][clang] Fix Coverity bugs with AUTO_CAUSES_COPY

tahonermann added a comment. I added a comment to skip the one where the element type is a simple pair. The rest look fine to me. Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:182 // Skip methods in records. -for (auto P : Context.getParents(*Method))

[PATCH] D149074: [NFC][clang] Fix Coverity bugs with AUTO_CAUSES_COPY

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good to me, thanks @Manna! Comment at: clang/utils/TableGen/NeonEmitter.cpp:392 -for (auto Type : Types) { +for (const auto &Type : Types) {

[PATCH] D149098: [Clang] Add tests and mark as implemented WG14 N2728 (char16_t & char32_t string literals shall be UTF-16 & UTF-32)

tahonermann created this revision. tahonermann added reviewers: aaron.ballman, erichkeane, cor3ntin. Herald added a project: All. tahonermann requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This change expands testing of UTF-8, UTF-16, and

[PATCH] D149098: [Clang] Add tests and mark as implemented WG14 N2728 (char16_t & char32_t string literals shall be UTF-16 & UTF-32)

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

[PATCH] D149098: [Clang] Add tests and mark as implemented WG14 N2728 (char16_t & char32_t string literals shall be UTF-16 & UTF-32)

tahonermann updated this revision to Diff 516835. tahonermann added a comment. Thank you @cor3ntin, that was an excellent suggestion; this is much more readable now! I updated the new code to use custom verify tags (I left the existing code alone). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D149009: [Sema]Select correct lexical context during template instantiate

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:3598 + FD->isDefined(FDFriend, true) && + FDFriend->getFriendObjectKind() != Decl::FriendObjectKind::FOK_None) { +// if Function defined by inline friend, use inline fried as Dec

[PATCH] D127284: [clang-repl] Support statements on global scope in incremental mode.

tahonermann added a comment. Herald added a subscriber: jplehr. This change appears to have negatively impacted some users that were dependent on the previous `Preprocessing::enableIncrementalProcessing(true)` behavior. See https://github.com/llvm/llvm-project/issues/62413. Repository: rG LL

[PATCH] D149098: [Clang] Add tests and mark as implemented WG14 N2728 (char16_t & char32_t string literals shall be UTF-16 & UTF-32)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. tahonermann marked 2 inline comments as done. Closed by commit rGa68039c51e61: [Clang] Add tests and mark as implemented WG14-N2728 (authored by tahonermann). Reposito

[PATCH] D149163: [NFC][CLANG] Fix coverity remarks about large copy by values

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. These changes all look good to me. For the `DeclaratorChunk` case, I noted an additional removal of a comment that I think should be considered. Assuming I haven't misread the code,

[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

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/D149647/new/ https://reviews.llvm.org/D149647 ___ cfe-commits mailing list

[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

tahonermann added a comment. Thank you for catching that @HazardyKnusperkeks! I completely missed (somehow) that the changed code modified `Expanded`. I offered another suggestion. Comment at: clang/lib/Format/Format.cpp:3486-3489 + Expanded.InsertBraces = true; + Pa

[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

tahonermann added inline comments. Comment at: clang/lib/Format/Format.cpp:3486-3489 + Expanded.InsertBraces = true; + Passes.emplace_back([&](const Environment &Env) { +return BracesInserter(Env, Expanded).process(/*SkipAnnotation=*/true); }); --

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

tahonermann added a comment. > @tahonermann Just a quick note - I've made the changes you initially > suggested in the RFC. When you get a chance, could you have another look? Yes, I hope to today. If not, I'll make it a priority for tomorrow. CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names

tahonermann added a comment. This is looking good to me. I added a few comments seeking clarification. I'm concerned about the changes to the existing calls to `getFloatingTypeOrder()`. The changes look like they will preserve prior behavior for standard types just fine, but I'm not sure whethe

[PATCH] D150913: [Clang][BFloat16] Upgrade __bf16 to arithmetic type, change mangling, and extend excess precision support.

tahonermann added a comment. I'm late to review and can no longer stamp an approval on this, but I'll note for the historical record that the changes look good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150913/new/ https://reviews.llvm.o

[PATCH] D150843: [clang][Diagnostics] Refactor printableTextForNextCharacter

tahonermann added a comment. The changes look good to me. I suggested two minor edits to align with parameter name changes. The summary states that the changes are not quite NFC. In that case, we would ideally have a test that demonstrates the changed behavior. Would adding such a test be chal

[PATCH] D146924: [clang] Add support for dollar sign in ud_suffix of numeric constants

tahonermann added a comment. > I wrote a paper https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3124.pdf It appears you back ported that paper from the future: "Date: 2024-22-04" :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146924/new/ https:

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

tahonermann added a comment. @cor3ntin, sorry for failing to keep up with reviews; I know this has already been committed. I did spot a couple of typos should you feel inclined to address them. Comment at: clang/lib/Lex/Lexer.cpp:2695 + // diagnostic only once per entire ill

[PATCH] D150843: [clang][Diagnostics] Refactor printableTextForNextCharacter

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. > They are NFC, it's just not 100% only a refactoring, since it adds the new > ASCII-only case. Ok, thanks. Changes look good. I noticed one comment that appears to be incorrect; pl

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann added a comment. When committing changes to address Coverity reported issues, I think it would be useful to include the Coverity analysis in the commit message. A textual representation can be obtained using `cov-format-errors --emacs-style` from the command line with access to the

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann added a comment. > I am bit afraid about release builds: > > if (!Initializer) > > return; > > Is this semantically incorrect? I think so. And now I think I see a path where the proposed `assert` is incorrect as well. The `else` branches at lines 5919, 5921, and 5923 appear to han

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann added a comment. > I don't see any after this point. You're right! I thought I had seen some, but upon checking, I agree. That is a good sign we're on the right path! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139148/new/ https://reviews.llvm.org/D139148 __

[PATCH] D139428: Handle char{8,16,32} and wchar_t in ASTContext::getIntegerRank()

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. The direction of the changes looks right. I noted a couple of minor considerations. Comment at: clang/lib/AST/ASTContext.cpp:7109 + case BuiltinType::Ch

[PATCH] D139428: Handle char{8,16,32} and wchar_t in ASTContext::getIntegerRank()

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good to me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139428/new/ https://reviews.llvm.org/D139428 ___ cfe-commits mai

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. Hmm, it seems the `assert` doesn't work in that location. The Linux and Windows builds both failed. It would be good to identify a simple test case for that. It seems we'll

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann added a comment. Here is a simple test case that fails the assertion in the current location. struct B { B(int, int); }; struct D : B { D() : B(0, 1) {} }; I spent a bit more time looking at the code and finally realized that `Initializer` is only assigned when `Args

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

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. This looks good to me now. I'm sorry for taking so long to return to review. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1098-1099 + +unsigned N = SL->

[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); tbaeder wrote:

[PATCH] D137051: [Clang] Allow additional mathematical symbols in identifiers.

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. This looks great to me. I added a minor comment regarding the grammar of the added diagnostic, but am accepting the review regardless. Comment at: clang/include/cl

[PATCH] D137051: [Clang] Allow additional mathematical symbols in identifiers.

tahonermann added a comment. Oh, also, the review summary mentions "UAX32" (twice); those should be "UAX31". Just please make sure the commit message is correct. Also, I'm terribly sorry for being so slow to get this reviewed! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. This looks really good. I added a suggested edit for a comment that I had a hard time understanding and noted an area of code that I'm not sure is working as expected. =

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

tahonermann added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:3378-3379 - if (LooseMatch) + // If no diagnostic has been emitted yet we do not want to recover here + // to make sure this function will be called again and a diagnostic emitted then. + if (LooseMatch

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

tahonermann added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:3386 + + if (!Result || CurPtr - StartPtr == (ptrdiff_t)(Buffer.size() + 4)) StartPtr = CurPtr; cor3ntin wrote: > tahonermann wrote: > > cor3ntin wrote: > > > cor3ntin wrote: > > > > tah

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:5959 if (DestType->isRecordType()) { +assert(Initializer && "Intializer must be non-null"); // - If the initialization is direct-initialization, or if it is It looks like t

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

tahonermann added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:3323 if (Diagnose) Diag(StartPtr, diag::warn_ucn_escape_incomplete); return llvm::None; I was able to confirm that `StartPtr` does point to `N`. See the diagnostic generated

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:5966 (Context.hasSameUnqualifiedType(SourceType, DestType) || - S.IsDerivedFrom(Initializer->getBeginLoc(), SourceType, DestType + (Initilializer && + S.IsDerivedF

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann added a comment. The build failed again, but it looks like the cause is unrelated to these changes. I restarted the build. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139148/new/ https://reviews.llvm.org/D139148 ___ cfe-commits

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. This looks good. I'm accepting the change despite one remaining issue that was neither introduced nor addressed by this patch; the diagnostic caret will still be in the wrong locatio

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

tahonermann added a comment. @cor3ntin, I suspect the answer is no, but do your changes perhaps address this assertion failure? https://godbolt.org/z/1bPcPcWzv int \u1{234}; Stack dump: clang++: /root/llvm-project/clang/lib/Lex/LiteralSupport.cpp:382: void clang::expandUCNs(llvm::SmallVec

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Builds passed now. I think this looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139148/new/ https://reviews.llvm.org/D139148 _

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

tahonermann added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5541-5542 + // + // FIXME: Do we need to handle the case that the calculated output is + // conflicting with the specified output file or the input file? + if (!AtTopLevel && isa(JA) && ---

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

tahonermann added inline comments. Comment at: clang/test/SemaTemplate/default-template-arguments.cpp:12 + auto lambda1 = [] {}; // expected-error {{default argument references local variable x_constexpr of enclosing function}} + auto lambda2 = [] {}; // expected-error {{def

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

tahonermann added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5541-5542 + // + // FIXME: Do we need to handle the case that the calculated output is + // conflicting with the specified output file or the input file? + if (!AtTopLevel && isa(JA) && ---

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

tahonermann added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5541-5542 + // + // FIXME: Do we need to handle the case that the calculated output is + // conflicting with the specified output file or the input file? + if (!AtTopLevel && isa(JA) && ---

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

tahonermann added a comment. This has already been merged, but I'm commenting just to note (continued) approval of the most recent changes. Looks great, Corentin! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138861/new/ https://reviews.llvm.org/D

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

tahonermann added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5541-5545 +if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o)) + TempPath = FinalOutput->getValue(); +else + TempPath = BaseInput; + ChuanqiXu wrote: > dblaiki

[PATCH] D119221: [clang][lexer] Allow u8 character literal prefixes in C2x

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:3565 else Ty = Context.CharTy; // 'x' -> char in C++ Perhaps worth updating this comment? e.g., // 'x' -> char in C++; u8'x' -> char in C11-C17 and in C++ without char8_t.

<    1   2   3   4