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

tahonermann added inline comments. Comment at: clang/lib/Basic/TargetInfo.cpp:287-288 FloatModeKind ExplicitType) const { + if (getHalfWidth() == BitWidth) +return FloatModeKind::Half; if (getFloatWidth() == BitWidth) ---

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

tahonermann added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:60 + Ibm128, + Half }; The existing enumerators were ordered according to precision. Consider moving `Half` to before `Float` if doing so doesn't cause any problems (I would

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

tahonermann added a comment. I wonder if I'm reading (temp.friend)p9` sentence 2 correctly. Which of these should it be parsed as? 1. A (friend function template with a constraint) that depends on a template parameter from an enclosing template

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

tahonermann added a comment. > Implementation modules are never serialized (-emit-module-interface for an > implementation unit is diagnosed and rejected). Never? Or just not via -emit-module-interface? I would expect to be able to serialize via -emit-ast. Repository: rG LLVM Github Monorep

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

tahonermann added a comment. > Note we might be confused, the parens there aren't completely clear as to > what your intent is. Well, I know that I'm confused and not clear what my intent is :) I asked the question because there appears to be an asymmetry between (temp.friend)p9 sentence 1

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

tahonermann added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:222 unsigned HasAlignMac68kSupport : 1; - unsigned RealTypeUsesObjCFPRet : 3; + unsigned RealTypeUsesObjCFPRet : 6; unsigned ComplexLongDoubleUsesFP2Ret : 1; Good find.

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

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. When submitting patches, please create the diff with context so that the code can be navigated in Phabricator. See https://llvm.org/docs/Phabricator.html#phabricator-reques

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

tahonermann added a comment. @cor3ntin, any concerns or suggestions per my recent updates? I'll plan to land this in the next couple of days otherwise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64087/new/ https://reviews.llvm.org/D64087 _

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

tahonermann updated this revision to Diff 556970. tahonermann added a comment. Addressed review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64087/new/ https://reviews.llvm.org/D64087 Files: clang/docs/ReleaseNotes.rst clang/lib/Sem

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG256a0b298c68: [clang] Correct source locations for instantiations of function templates. (authored by tahonermann). Repository: rG LLVM Github Mon

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

tahonermann added a comment. @codemzs, I'm sorry for the very long delay following up on this review. I've been struggling to keep up, but expect to be able to devote some time to this next week. I'm committed to helping to ensure we land this before Phabricator stops accepting new diffs (propo

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

tahonermann added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:147-149 +def ext_vla_cxx_static_assert : ExtWarn< + "variable length arrays in C++ are a Clang extension; did you mean to use " + "'static_assert'?">, InGroup; I fin

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

tahonermann added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:147-149 +def ext_vla_cxx_static_assert : ExtWarn< + "variable length arrays in C++ are a Clang extension; did you mean to use " + "'static_assert'?">, InGroup; aaron

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

tahonermann 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 ''?' == '#''}} +//

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

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/TokenKinds.h:87-93 +/// Return true if this token is a predefined macro +/// unexpandable by MSVC preprocessor.

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

tahonermann 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 == t

[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

tahonermann added a comment. It is unclear to me why/when we would ever want the substitute drive expansion; the modified tests aren't very elucidating. My naive expectation is that we effectively want to restore the Python 3.7 behavior. Comment at: clang/test/ExtractAPI/rela

[PATCH] D155776: [NFC] Add checks for self-assignment.

tahonermann added a comment. Hmm, it seems exceptionally unlikely for a move assignment operator to be invoked on an object with itself as the source argument. That would require an explicit use of `std::move()` as in: Value x = ...; x = std::move(x); Rather than protecting against that an

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

tahonermann accepted this revision. tahonermann added a comment. Looks good; the default implementations would definitely do the wrong thing, so this is a good find. My suggested edit is just to add a blank line. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1823-1826 + OMPT

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

tahonermann added a comment. > there's no bug being fixed which makes me think this should go back to the > static analysis vendor to report this as a false positive. I agree. I'll try to isolate a reproducible test case and report it to the vendor. Repository: rG LLVM Github Monorepo CHAN

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

tahonermann accepted this revision. tahonermann added a comment. This is a good change since the default copy constructor would do bad things when `ownsOutputFile` is true. The copy assignment operator is already implicitly deleted because of the presence of a data member of reference type, but

[PATCH] D155776: [NFC] Add checks for self-assignment.

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. On second thought, and after having read https://stackoverflow.com/a/9322542/11634221, I think this change is fine. There are legitimate scenarios for which self move assignment shou

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

tahonermann added a comment. > I agree. I'll try to isolate a reproducible test case and report it to the > vendor. Done. I'm ambivalent with regard to keeping or discarding the proposed change, but the static analysis issue should be triaged as a false positive. Repository: rG LLVM Github

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

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. I think this is looking good. The only big thing I noticed is some code that looks like it should have been removed with the last set of changes. Otherwise, just some minor

[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

tahonermann added inline comments. Comment at: clang/test/Lexer/case-insensitive-include-win.c:5-9 +// Note: We must use the real path here, because the logic to detect case +// mismatch relies on resolving the real path and checking that casing differs. +// If we use %t and we a

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

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:2010-2013 +OS << "R\"EFLPM(" + << PredefinedExpr::ComputeName(getPredefinedExprKind(Tok.getKind()), + currentDecl) + << ")EFLPM\""; RIs

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

tahonermann added inline comments. Comment at: clang/test/Sema/ms_predefined_expr.cpp:156 +static_assert(__FUNCTION__ ""_len == 14); // expected-warning{{string literal concatenation of predefined identifier '__FUNCTION__' is a Microsoft extension}} } Here

[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

tahonermann added a comment. > Marking comments as resolved per my reply. I'm not sure if that's best > practice! Yes, that is fine. Comment at: clang/test/Lexer/case-insensitive-include-win.c:5-9 +// Note: We must use the real path here, because the logic to detect case +//

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

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:2010-2013 +OS << "R\"EFLPM(" + << PredefinedExpr::ComputeName(getPredefinedExprKind(Tok.getKind()), + currentDecl) + << ")EFLPM\""; cor

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

tahonermann added a comment. I think I'm happy with this. I noted two code changes that I think are no longer needed and offered some final suggestions on some names. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:118-120 +def ext_expand_predef_ms : ExtWarn< + "

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

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:1981 + // parsed yet). + assert(getLangOpts().MicrosoftExt); + RIscRIpt wrote: > I think I should remove this assertion so this function would be usable > without MS ext, on the other h

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

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. I'm happy with this. I noted one last rename suggestion to maintain consistency, but am accepting. Please give Corentin a final chance to raise any concerns. Thank you for being so r

[PATCH] D156247: [Clang] Add a warning on uses of coroutine keywords

tahonermann added a comment. At first, it sounded to me like the option that would best suit Google's needs is `-fno-coroutines`. However: - I see that Google does have some limited use of coroutines; perhaps that could be enabled on a per-TU or project basis by the build system? However... - T

[PATCH] D156247: [Clang] Add a warning on uses of coroutine keywords

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. > You are absolutely right, -fno-coroutines would totally work for us, had it > been available. Good. Gcc handles `-fcoroutines` and `-fno-coroutines` as I would expect (h

[PATCH] D155081: Specify the developer policy around links to external resources

tahonermann added a comment. This looks good to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155081/new/ https://reviews.llvm.org/D155081 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D157885: [NFC][Clang] Fix static analyzer concern about null value derefence

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

[PATCH] D157554: [NFC][Clang] Fix static analyzer concern about null pointer dereference

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/D157554/new/ https://reviews.llvm.org/D157554 ___ cfe-commits mai

[PATCH] D157888: [NFC][Clang] Fix static analyzer concern about null value dereference

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/D157888/new/ https://reviews.llvm.org/D157888 ___ cfe-commits mai

[PATCH] D157989: [NFC] Initialize pointer field

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good to me. I see that `OpenMPIRBuilder::emitTargetKernel()` can return without having assigned the `Return` parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

tahonermann added inline comments. Comment at: clang/test/Lexer/case-insensitive-include-win.c:5-9 +// Note: We must use the real path here, because the logic to detect case +// mismatch relies on resolving the real path and checking that casing differs. +// If we use %t and we a

[PATCH] D158293: [NFC][CLANG] Fix potential dereferencing of null return values

tahonermann added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2009-2010 (Line.MightBeFunctionDecl || Line.InPPDirective) && - Current.NestingLevel == 0 && + Current.NestingLevel == 0 && Current.Previous &&

[PATCH] D158285: [NFC][CLANG] Fix wrong orders of function arguments positions

tahonermann added a comment. > To me, an important metric of a quality patch is if the title and the summary > describe why we have this patch, and what we do about it. @steakhal, I'll work with the people submitting these patches to add more context to the title, summary, and commit comments.

[PATCH] D155776: [NFC] Add checks for self-assignment.

tahonermann accepted this revision. tahonermann added a comment. Thank you for following up, Sindhu! Looks good! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155776/new/ https://reviews.llvm.org/D155776 ___ cfe-commits mailing list cfe-commi

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

tahonermann added inline comments. Comment at: clang/include/clang/AST/Stmt.h:2606-2607 enum { INIT, CONDVAR, COND, INC, BODY, END_EXPR }; - Stmt* SubExprs[END_EXPR]; // SubExprs[INIT] is an expression or declstmt. + Stmt *SubExprs[END_EXPR] = { + nullptr}; // SubExprs[

[PATCH] D158285: [NFC][CLANG] Fix wrong orders of function arguments positions

tahonermann added inline comments. Comment at: clang/lib/AST/Interp/InterpBlock.cpp:94-95 DeadBlock::DeadBlock(DeadBlock *&Root, Block *Blk) -: Root(Root), B(Blk->Desc, Blk->IsStatic, Blk->IsExtern, /*isDead=*/true) { +: Root(Root), B(Blk->Desc, Blk->IsExtern, Blk->IsS

[PATCH] D158488: [NFC] Initialize member pointers to nullptr.

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Looks good to me. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:934 // Return value of the runtime offloading call. - Value *Return; + Value *Return =

[PATCH] D158372: [Clang] Treat invalid UDL as two tokens

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. Given Richard's comments, it seems that changes are needed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158372/new/ https://reviews.llvm.org/D158372 ___

[PATCH] D158293: [NFC][Clang] Fix static code analyzer concern about null value dereference

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Lex/PPDirectives.cpp:494 ++NumSkipped; assert(!CurTokenLexer && CurPPLexer && "Lexing a macro, not a file?"); --

[PATCH] D158522: [NFC][CLANG] Fix static analyzer bugs about large copy by values

tahonermann accepted this revision. tahonermann added a comment. Looks fine to me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158522/new/ https://reviews.llvm.org/D158522 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

tahonermann added a comment. > @tahonermann are all your comments addressed at this point? Apologies for the late response; I was on vacation last week. No. I'm still skeptical that there is ever a desire to expand substitute drives. Comment at: clang/test/Lexer/case-insensit

[PATCH] D157118: [NFC][Clang] Fix static analyzer concerns

tahonermann accepted this revision. tahonermann added a comment. I think this change is good as is, but I added some additional thoughts to Aaron's earlier comment. Comment at: clang/lib/AST/StmtPrinter.cpp:178 void StmtPrinter::PrintRawCompoundStmt(CompoundStmt *Node) { + a

[PATCH] D157129: [NFC] Fix unnecessary copy with auto.

tahonermann requested changes to this revision. tahonermann added a comment. I agree with the analysis done by @steakhal. The static analyzer being used (which I'm not allowed to name) is being too noisy in these cases; it complains about every case where an object of class type is copied regard

[PATCH] D157454: [NFC][Clang] Fix static analyzer concern about null value dereference

tahonermann added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:222-223 Selector Sel = MethodWithObjects->getSelector(); - QualType ResultType = E->getType(); - const ObjCObjectPointerType *InterfacePointerType -= ResultType->getAsObjCInterfacePointerType();

[PATCH] D157454: [NFC][Clang] Fix static analyzer concern about null value dereference

tahonermann added inline comments. Comment at: clang/lib/CodeGen/CGObjC.cpp:222-223 Selector Sel = MethodWithObjects->getSelector(); - QualType ResultType = E->getType(); - const ObjCObjectPointerType *InterfacePointerType -= ResultType->getAsObjCInterfacePointerType();

[PATCH] D157454: [NFC][Clang] Fix static analyzer concern about null value dereference

tahonermann accepted this revision. tahonermann added a comment. Looks good to me. Thanks, Elizabeth! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157454/new/ https://reviews.llvm.org/D157454 ___ cfe-commits mailing list cfe-commits@lists.ll

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

tahonermann updated this revision to Diff 556592. tahonermann edited the summary of this revision. tahonermann added a comment. Rebased patch uploaded. This retains the same code change, but includes additional test updates for tests added since the first patch was submitted, as well as an addit

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

tahonermann added a comment. @Endill, thank you for the reminder about this old patch! @cor3ntin, I added an additional test and updated a few other recently (relative to the original patch!) added tests. Would you be so kind as to give this another quick review? Repository: rG LLVM Github

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

tahonermann updated this revision to Diff 556604. tahonermann added a comment. Moved the added release note to the correct section. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64087/new/ https://reviews.llvm.org/D64087 Files: clang/docs/Releas

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

tahonermann added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:304 +static_assert('\u{9}' == (char)1, ""); // expected-error {{failed}} \ + // expected-note {{evaluates to ''\t' (0x09, 9) == '' (0x01, 1)'}} +static

[PATCH] D159474: [NFC][CLANG] Fix static analyzer bugs about unnecessary object copies with auto

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Thanks, @Manna, these changes look good to me! Comment at: clang/include/clang/ExtractAPI/ExtractAPIVisitor.h:175 SmallVector Bases; -for (const auto BaseS

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

tahonermann accepted this revision. tahonermann added a comment. These changes look fine to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153589/new/ https://reviews.llvm.org/D153589 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D153926: [NFC] Initialize class member pointers to nullptr.

tahonermann accepted this revision. tahonermann added a comment. Herald added a subscriber: wangpc. These changes look find to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153926/new/ https://reviews.llvm.org/D153926 ___ cfe-commits mail

[PATCH] D153892: [NFC] Initialize class member pointers to nullptr.

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Herald added a subscriber: wangpc. Comment at: clang/lib/Frontend/ASTConsumers.cpp:192-198 + bool HandleTopLevelDecl(DeclGroupRef D) override {

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

tahonermann requested changes to this revision. tahonermann added a comment. This revision now requires changes to proceed. Changes look good; I added a number of suggested edits for minor issues. Comment at: clang/docs/ReleaseNotes.rst:203-204 +- Implemented `WG14 N3124

[PATCH] D153418: Adding iconv support to CharSetConverter class

tahonermann added a subscriber: hubert.reinterpretcast. tahonermann added a comment. > Please correct me if I'm wrong, I'm not too familiar with icu4c, but I think > adding support for ICU would be the better long-term solution since it seems > to allow the same behaviour across different platfo

[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

tahonermann added a comment. > 95% of the %>t are around clang modulemap files, because that code resolves > real paths in C++ by design, so I can't avoid it. Can you link to evidence that this behavior is by design? It isn't obvious to me why modulemap files would demand different behavior; es

[PATCH] D153892: [NFC] Initialize class member pointers to nullptr.

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Thanks, Sindhu! This looks good to me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153892/new/ https://reviews.llvm.org/D153892 __

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

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. > I'll probably merge that EoW unless you scream! Why wait? :) Looks good! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153621/new/

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

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:118-120 +def ext_concat_predef_ms : ExtWarn< + "concatenation of predefined identifier '

[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

tahonermann added a comment. >> In D154130#4486898 , @tahonermann >> wrote: >> >>> 95% of the %>t are around clang modulemap files, because that code resolves >>> real paths in C++ by design, so I can't avoid it. >> >> Can you link to evidence that this

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

tahonermann added a comment. A couple of observations with regard to compatibility: gcc,, at least by default, emits the TC implementations as local functions, the resolver as a weak global function, and the undecorated name as an ifunc. When only a TC declaration (not a definition) is present,

[PATCH] D158666: [Clang] Fix linker error for function multiversioning

tahonermann added a comment. I think it would also be a good idea to exercise the test case for a target that lacks ifunc support to ensure that inter-TU calls work as expected. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158666/new/ https://reviews.llvm.org/D158666

[PATCH] D158869: [clang] Fix timing of propagation of MSInheritanceAttr for template instantiation declarations.

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. This change addresses two issues: 1. A failure to propagate

[PATCH] D158869: [clang] Fix timing of propagation of MSInheritanceAttr for template instantiation declarations.

tahonermann added reviewers: rnk, zequanwu. tahonermann added a comment. Adding Erich as attributes owner, and Reid and Zequan for MS ABI. This patch slightly modifies the changes made via D94646 and committed as rG4fffbc150cca1638051b8ad2a20f4b8240df0869

[PATCH] D158869: [clang] Fix timing of propagation of MSInheritanceAttr for template instantiation declarations.

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:10112 + Specialization->addAttr(PrevDecl->getAttr()); + Consumer.AssignInheritanceModel(Specialization); +} erichkeane wrote: > tahonermann wrote: > > I am concerned that

[PATCH] D158293: [NFC][Clang] Fix static code analyzer concern about null value dereference

tahonermann requested changes to this revision. tahonermann added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Lex/PPDirectives.cpp:494-495 ++NumSkipped; - assert(!CurTokenLexer && CurPPLexer && "Lexing a macro, not a file?"); + asser

[PATCH] D158869: [clang] Fix timing of propagation of MSInheritanceAttr for template instantiation declarations.

tahonermann marked 2 inline comments as done. tahonermann added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:10110-10111 +// propagated to the new node prior to instantiation. +if (PrevDecl && PrevDecl->hasAttr()) { + Specialization->addAttr(PrevDecl-

[PATCH] D158869: [clang] Fix timing of propagation of MSInheritanceAttr for template instantiation declarations.

tahonermann updated this revision to Diff 554413. tahonermann marked an inline comment as done. tahonermann edited the summary of this revision. tahonermann added a comment. Address prior review feedback. This update also modifies the propagation of the `MSInheritanceAttr` attribute for explicit

[PATCH] D158293: [NFC][Clang] Fix static code analyzer concern about null value dereference

tahonermann added inline comments. Comment at: clang/lib/Lex/PPDirectives.cpp:494-495 ++NumSkipped; assert(!CurTokenLexer && CurPPLexer && "Lexing a macro, not a file?"); + assert(CurLexer && "Invalid lexer value"); Would you mind splitting out all of th

[PATCH] D158293: [NFC][Clang] Fix static code analyzer concern about null value dereference

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/D158293/new/ https://reviews.llvm.org/D158293 ___

[PATCH] D158869: [clang] Fix timing of propagation of MSInheritanceAttr for template instantiation declarations.

tahonermann updated this revision to Diff 555076. tahonermann added a comment. Added a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158869/new/ https://reviews.llvm.org/D158869 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/S

[PATCH] D158869: [clang] Fix timing of propagation of MSInheritanceAttr for template instantiation declarations.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6658db5e5014: [clang] Fix timing of propagation of MSInheritanceAttr for template… (authored by tahonermann). Repository: rG LLVM Github Monorepo

[PATCH] D159345: [Clang] Handle non-ASCII after line splicing

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. This looks good to me modulo a couple of nits. Comment at: clang/include/clang/Lex/Lexer.h:806-807 /// Try to consume an identifier character encoded in UTF-8.

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

tahonermann added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:304 +static_assert('\u{9}' == (char)1, ""); // expected-error {{failed}} \ + // expected-note {{evaluates to ''\t' (0x09, 9) == '' (0x01, 1)'}} +static

[PATCH] D159345: [Clang] Handle non-ASCII after line splicing

tahonermann accepted this revision. tahonermann added a comment. Even better than I asked for. I held back on suggesting the change of `Tok` to `Result` to match `tryConsumeIdentifierUCN()`, but you made that change anyway! You must have read my mind! :) Repository: rG LLVM Github Monorepo

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

tahonermann added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:304 +static_assert('\u{9}' == (char)1, ""); // expected-error {{failed}} \ + // expected-note {{evaluates to ''\t' (0x09, 9) == '' (0x01, 1)'}} +static

[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; tahonermann wrote: > A

[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 added a comment. I apologize for the delay Corentin, I hope to get through a number of my outstanding code reviews tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137051/new/ https://reviews.llvm.org/D137051 __

[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. Per added comments, I think we should look for a guarantee that `Initializer` is non-null earlier in the function. If there is, then we could remove a bunch of the current

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

tahonermann added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:5824-5828 // Handle default initialization. if (Kind.getKind() == InitializationKind::IK_Default) { TryDefaultInitialization(S, Entity, Kind, *this); return; } schittir

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

tahonermann added a comment. The change to add the assert looks good. I'm eager to see what the testsuite has to say about it :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139148/new/ https://reviews.llvm.org/D139148 ___ cfe-commits maili

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

tahonermann added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:5890-5892 + TargetOpts.EABIVersion = llvm::EABI::Default; + // Initialize CodeObjectVersion with default i.e., 4 + TargetOpts.CodeObjectVersion = TargetOptions::CodeObjectVersionKind::COV_4; -

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

tahonermann added inline comments. Comment at: clang/include/clang/Basic/TargetOptions.h:48 /// The EABI version to use - llvm::EABI EABIVersion; + llvm::EABI EABIVersion = llvm::EABI::Default; Looks good; this is consistent with the default initialization

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

tahonermann added a comment. @cor3ntin, do you have any thoughts about this? CWG 1871 (Non-identifier characters in ud-suffix) is somewhat related. I think the changed behavior makes sense when `-fdollars-in-identifiers` is in effect (which is the default). ==

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

tahonermann added a comment. > It's not in the range of any of the universal-character-names. I assume you mean that it isn't included in the range of UCNs that are allowed in identifiers. If so, that is correct; it is neither in XID_Start

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

tahonermann added inline comments. Comment at: lldb/source/Commands/CommandObjectTarget.cpp:2182-2183 -clang::DumpModuleInfoAction dump_module_info; -dump_module_info.OutputStream = &result.GetOutputStream().AsRawOstream(); +std::shared_ptr Out( +&result.Get

[PATCH] D146604: [NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

tahonermann accepted this revision. tahonermann added a comment. This revision is now accepted and ready to land. Changes look good to me. Thanks, Sindhu! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146604/new/ https://reviews.llvm.org/D146604

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

tahonermann accepted this revision. tahonermann added a comment. > is it ok to land this patch? No objection from me! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150744/new/ https://reviews.llvm.org/D150744 ___ cfe-commits mailing list cfe

[PATCH] D150931: [NFC][Clang] 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. These changes look fine to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150931/new/ https://reviews.llvm.org/D150931 ___ cf

<    1   2   3   4   >