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

eandrews updated this revision to Diff 548789. eandrews added a comment. Applied review comments to use an assert instead CHANGES SINCE LAST ACTION https://reviews.llvm.org/D157454/new/ https://reviews.llvm.org/D157454 Files: clang/lib/CodeGen/CGObjC.cpp Index: clang/lib/CodeGen/CGObjC.c

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

eandrews created this revision. eandrews added reviewers: tahonermann, aaron.ballman. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware. Herald added a project: All. eandrews requested review of this revision. Herald added a subscriber: wa

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

eandrews added a comment. In D157554#4576478 , @aaron.ballman wrote: > This feels a bit more like a functional change than a non-functional change > because it seems like we should be able to test this case (whereas, if we > think `TC` can never be nul

[PATCH] D141375: [SYCL][OpenMP] Fix compilation errors for unsupported __bf16 intrinsics

eandrews added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1978-1979 !Context.getTargetInfo().hasInt128Type()) || +(Ty->isBFloat16Type() && !Context.getTargetInfo().hasBFloat16Type() && + !LangOpts.CUDAIsDevice) || LongDoubleMismatched)

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

eandrews created this revision. eandrews added reviewers: erichkeane, tahonermann, aaron.ballman, zsrkmyn. Herald added a project: All. eandrews requested review of this revision. Currently target_clones attribute results in a linker error when there are no multi-versioned function declarations i

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

eandrews added a comment. In D158666#4611481 , @erichkeane wrote: > I think the .ifunc spelling was an oversight on my part when I implemented > this, I didn't spend enough time investigating GCC's behavior when > implementing this feature. I think th

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

eandrews added a comment. In D158666#4611497 , @erichkeane wrote: > In D158666#4611494 , @eandrews > wrote: > >> In D158666#4611481 , @erichkeane >> wrote: >> >>> I thi

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

eandrews created this revision. eandrews added reviewers: aaron.ballman, tahonermann. Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware. Herald added a reviewer: ributzka. Herald added a project: All. eandrews requested review of this revi

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

eandrews added a comment. aarch64 has different mangling for default functions when using target_clones. Sigh `int __attribute__((target_clones("ls64_v+fp16", "default"))) foo_ovl(int) { return 1; }` results in `@_Z7foo_ovli._Mfp16Mls64_v` and `@_Z7foo_ovlv`. Note there is no ".default" as in

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

eandrews updated this revision to Diff 553534. eandrews added a comment. Thanks for the reviews! Requested changes have been made in latest patch CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158671/new/ https://reviews.llvm.org/D158671 Files: clang/include/clang/ExtractAPI/ExtractAP

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG847186eb94bd: [NFC][Clang] Fix static analyzer concern (authored by eandrews). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

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

eandrews updated this revision to Diff 554063. eandrews added a comment. Changed a dyn_cast to cast and handled ObjCAtFinallyStmt similar to how ObjCAtTryStmt is handled, i.e. just check that it exists before calling PrintRawCompoundStmt CHANGES SINCE LAST ACTION https://reviews.llvm.org/D15

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG39191c457715: [NFC][Clang] Fix static analyzer concerns (authored by eandrews). Herald added a project: clang. Repository: rG LLVM Github Monorepo

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

eandrews added subscribers: erichkeane, aaron.ballman. eandrews added a comment. Functionally this looks ok to me. However I am not sure if CodeGenTypes is the 'right' place for this function to live, considering that other functions with similar functionality are in ASTContext - including overl

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews created this revision. eandrews added reviewers: rnk, gribozavr, gribozavr2, Szelethus, erichkeane, riccibruno. This patch reapplies D61027 . When D61027 was previously committed (76945821b9cad3), buildbots failed due t

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews added a subscriber: mibintc. eandrews added a comment. Thank you for taking a look Reid and Dmitri. There were no fails with check-all. My commit rights have not been transferred from SVN to Github yet, so Melanie (@mibintc) has kindly agreed to commit this patch for me. CHANGES SINCE

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews added a comment. @rnk @gribozavr2 clang-tidy test "bugprone-string-integer-assignment.cpp" caused a bot fail on windows. From the logs it looks like the 'CHECK-FIXES' I added in that test is failing because the fix isn't applied. I am not sure why. I don't have a windows build set up a

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews added a comment. Thank you for letting me know. The patch has been reverted. The fail is a result of different diagnostics thrown on Windows and Linux. It looks like clang passes -fdelayed-template-parsing by default on Windows, which I assume is the reason why the diagnostic is not ge

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews added a comment. The patch changes type dependency of field 'std::string s' and sets it based on field type (i.e. not type-dependent). I do not know how clang-tidy works internally, but I assume checker used to skip the field earlier since it was set as dependent (based on containing

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews added a comment. In D69950#1751859 , @gribozavr2 wrote: > > hence not throwing the warning on any platform? > > The way I read the buildbot breakage, an existing ClangTidy test passed > before and after this change, but broke on Windows. The brea

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews added a comment. In D69950#1752044 , @gribozavr2 wrote: > In D69950#1751894 , @eandrews wrote: > > > In D69950#1751859 , @gribozavr2 > > wrote: > > > > > > hence no

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews added a comment. In D69950#1770138 , @mstorsjo wrote: > This (when reapplied in > https://reviews.llvm.org/rG878a24ee244a24c39d1c57e9af2) broke compilation of > code that earlier built fine. A reduced example: > > namespace glslang { > class

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

eandrews added a comment. In D69950#1771340 , @mstorsjo wrote: > In D69950#1771133 , @eandrews wrote: > > > In D69950#1770138 , @mstorsjo > > wrote: > > > > > This (when rea

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

eandrews updated this revision to Diff 243049. eandrews edited the summary of this revision. eandrews added a comment. Thanks for taking a look Richard. This patch adds the required value-dependency check and sets type-dependency accordingly. CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

eandrews added a comment. Thanks! I'll push it shortly. Just running a final ninja check-all after updating workspace. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe-commits mailing list cf

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

eandrews added a comment. Actually I just noticed it still says 'Needs review' above. I'm not sure what the protocol is. Can I push the patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

eandrews added a comment. In D72242#1873017 , @smeenai wrote: > I'm not sure why Phabricator is still showing Needs Review, but @rnk's > approval should make this count as accepted. Ok. Thank you. I've pushed the patch. CHANGES SINCE LAST ACTION htt

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGa58017e5cae5: Fix type-dependency of bitfields in templates (authored by eandrews). Herald added a project: clang. Reposi

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

eandrews added a comment. In D72242#1820908 , @rnk wrote: > I think I liked the first version of this patch better. I would say, if the > AST after instantiation remains the same as it was before D69950 > , then the first

[PATCH] D39210: Add default calling convention support for regcall.

eandrews updated this revision to Diff 120284. eandrews added a comment. Updated patch to set default calling convention for main() in CheckMain() https://reviews.llvm.org/D39210 Files: include/clang/Basic/LangOptions.h include/clang/Driver/CC1Options.td include/clang/Driver/CLCompatOptio

[PATCH] D39210: Add default calling convention support for regcall.

eandrews added inline comments. Comment at: lib/Sema/SemaType.cpp:3269-3273 + bool IsMain = false; + if (D.getIdentifier() && D.getIdentifier()->isStr("main") && + S.CurContext->getRedeclContext()->isTranslationUnit() && + !S.getLangOpts().Freestanding) +IsMain =

[PATCH] D39210: Add default calling convention support for regcall.

eandrews added a comment. *ping* https://reviews.llvm.org/D39210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39210: Add default calling convention support for regcall.

eandrews added a comment. No problem! Thanks! https://reviews.llvm.org/D39210 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

eandrews added a comment. Herald added a subscriber: jplehr. This patch causes an assertion when the attribute argument is an integer constant expression - https://godbolt.org/z/osKx5ejMb and has resulted in test fails downstream since any attribute which uses `VerifyIntegerConstantExpression`

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

eandrews added a comment. In D124351#4208842 , @cor3ntin wrote: > In D124351#4207501 , @eandrews > wrote: > >> This patch causes an assertion when the attribute argument is an integer >> constant expression - ht

[PATCH] D146535: [Clang] Fix evaluation of parameters of lambda call operator attributes

eandrews added a comment. > Attributes of lambda call operator were evaluated in the context of the > closure object type rather than its operator, Just for my understanding, what did this affect with regards to the if guard for assert? CurContext or something else? I suspected it had something

[PATCH] D146535: [Clang] Fix evaluation of parameters of lambda call operator attributes

eandrews accepted this revision. eandrews added a comment. This revision is now accepted and ready to land. In D146535#4210268 , @cor3ntin wrote: > In D146535#4210198 , @eandrews > wrote: > >>> Attributes of lamb

[PATCH] D146535: [Clang] Fix evaluation of parameters of lambda call operator attributes

eandrews added a comment. Can we merge this patch? It will fix regressions introduced by D124351 I see downstream Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146535/new/ https://reviews.llvm.org/D146535 __

[PATCH] D151837: [Clang][Parser] Accept GNU attributes preceding C++ style attributes on templates

eandrews created this revision. eandrews added reviewers: aaron.ballman, erichkeane, compnerd. Herald added a project: All. eandrews requested review of this revision. Clang was rejecting valid code where GNU style attributes preceded C++ style attributes in template declarations as follows: tem

[PATCH] D151837: [Clang][Parser] Accept GNU attributes preceding C++ style attributes on templates

eandrews updated this revision to Diff 527382. eandrews added a comment. Thanks for the review! I added a couple more tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151837/new/ https://reviews.llvm.org/D151837 Files: clang/lib/Parse/ParseTemplate.cpp clang/test/Parser/attr-or

[PATCH] D151837: [Clang][Parser] Accept GNU attributes preceding C++ style attributes on templates

eandrews updated this revision to Diff 527406. eandrews added a comment. Thanks for the review! I've added a release note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151837/new/ https://reviews.llvm.org/D151837 Files: clang/docs/ReleaseNotes.rst clang/lib/Parse/ParseTemplate.cpp

[PATCH] D151837: [Clang][Parser] Accept GNU attributes preceding C++ style attributes on templates

eandrews added inline comments. Comment at: clang/docs/ReleaseNotes.rst:416 + handled, resulting in compilation error. This has been corrected to match the + behaviour exhibited by GCC. erichkeane wrote: > I'll make this change when I commit the patch. Thank

[PATCH] D151837: [Clang][Parser] Accept GNU attributes preceding C++ style attributes on templates

This revision was automatically updated to reflect the committed changes. Closed by commit rGcecd8471e499: [Clang][Parser] Accept GNU attributes preceding C++ attributes on templates (authored by eandrews). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D151837

[PATCH] D152195: [Clang] Fix access of friend function in local class

eandrews created this revision. eandrews added reviewers: erichkeane, aaron.ballman. Herald added a project: All. eandrews requested review of this revision. Clang currently emits an error when a friend of a local class tries to access it's private data members. This patch fixes the bug. https:

[PATCH] D152195: [Clang] Fix access of friend function in local class

eandrews added a comment. In D152195#4399251 , @erichkeane wrote: > I think the patch looks fine, but this needs a release note. Just so I know - Does every bug fix need a release note now? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152195/

[PATCH] D152195: [Clang] Fix access of friend function in local class

eandrews updated this revision to Diff 528883. eandrews added a comment. Thanks for the review! I added a release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152195/new/ https://reviews.llvm.org/D152195 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaDecl.cpp clang/t

[PATCH] D152195: [Clang] Fix access of friend function in local class

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc1401e9f3e7c: [Clang][Sema] Fix access of friend class in local class (authored by eandrews). Herald added a project: clang. Changed prior to commit

[PATCH] D124351: [Clang][WIP] Implement Change scope of lambda trailing-return-type - Take 2

eandrews added a comment. I came across a strange error when capturing arguments in a lambda inside another lambda. I filed an issue here - https://github.com/llvm/llvm-project/issues/59549 Short reproducer: void foo () { constexpr int i = 2; [&]() { [=]() [[clang::annotate_

[PATCH] D35259: Complex Long Double classification In RegCall calling convention

eandrews created this revision. This change is part of the RegCall calling convention support for LLVM. Existing RegCall implementation was extended to include correct handling of Complex Long Double type. Complex long double types should be returned/passed in memory and not register stack. Thi

[PATCH] D35259: Complex Long Double classification In RegCall calling convention

eandrews added a comment. In https://reviews.llvm.org/D35259#805415, @erichkeane wrote: > In https://reviews.llvm.org/D35259#805409, @rnk wrote: > > > In https://reviews.llvm.org/D35259#805284, @erichkeane wrote: > > > > > Oren discovered this miss to the original implementation. I'd reviewed >

[PATCH] D35259: Complex Long Double classification In RegCall calling convention

eandrews updated this revision to Diff 107343. eandrews added a comment. As per revision comments, I moved the condition for extended precision floating type to isX86VectorTypeForVectorCall. This update will now alter behavior for complex long double type under vectorcall calling convention as w

[PATCH] D35259: Complex Long Double classification In RegCall calling convention

eandrews updated this revision to Diff 107667. eandrews added a comment. Regcall-specific checks for Lin64 now occur only if CXXABI returns false. An existing test has also been modified to verify behavior with non trivial destructors. https://reviews.llvm.org/D35259 Files: lib/CodeGen/Targ

[PATCH] D40925: Add option -fkeep-static-consts

eandrews added a comment. That makes sense. Is it not possible to implement the required functionality using a flag vs an attribute? In an earlier comment you mentioned adding the global to @llvm.used to prevent GlobalDCE from removing it. Can you not do that when using a flag? https://review

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews added inline comments. Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:636-638 +unsigned AS = PointeeType->isFunctionTy() + ? getDataLayout().getProgramAddressSpace() + : Context.getTargetAddressSpace(ETy); rj

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews updated this revision to Diff 398723. eandrews added a comment. Implemented review comment to move logic into function (`getTargetAddressSpace`) as opposed to call site. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111566/new/ https://reviews.llvm.org/D111566 Files: clang/

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews added a comment. In D111566#3228949 , @rjmccall wrote: > Block pointers are actually data pointers and should stay in the generic > address space if they don't have an address space qualifier. That might > require new logic. Thanks! I kept th

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:11494 + const Type *TypePtr = T.getTypePtr(); + return TypePtr->isFunctionType() + ? llvm::DataLayout(getTargetInfo().getDataLayoutString()) rjmccall wrote: > You can just do `

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews updated this revision to Diff 398784. eandrews added a comment. Implement review comments - add a comment + remove unnecessary code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111566/new/ https://reviews.llvm.org/D111566 Files: clang/include/clang/AST/ASTContext.h clang/

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews marked 2 inline comments as done. eandrews added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:11496 + .getProgramAddressSpace() + : getTargetAddressSpace(T.getQualifiers()); +} rjmccall wrote: > Oh, I'm sorry I

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews updated this revision to Diff 399080. eandrews added a comment. Add program address space to TargetInfo. Targets with non-default address space for functions should explicitly set value. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111566/new/ https://reviews.llvm.org/D111566

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews marked 3 inline comments as done. eandrews added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:763 + /// Set the address space for functions for the given target. + virtual void setProgramAddressSpace() { ProgramAddrSpace = 0; } +

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews updated this revision to Diff 399484. eandrews added a comment. Herald added a subscriber: Jim. Implemented review comments - Remove unnecessary set method and set default value during construction instead - Changed default address space for function pointer for avr target to 1. CHANG

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews added inline comments. Comment at: clang/test/CodeGen/avr/functionptr-addrspace.c:3 + +int main() { + int (*p)(); When writing the test I noticed an existing crash in the compiler. I am not sure if I am doing something wrong but if you try to assign to

[PATCH] D111566: [SYCL] Fix function pointer address space

This revision was automatically updated to reflect the committed changes. Closed by commit rG4eaf5846d0e7: [clang] Fix function pointer address space (authored by eandrews). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D97941: [Reland] "Do not apply calling conventions to MSVC entry points"

This revision was automatically updated to reflect the committed changes. eandrews marked an inline comment as done. Closed by commit rGd8b8f544d9de: [Reland] "Do not apply calling conventions to MSVC entry points" (authored by eandrews). Herald added a project: clang. Repository: rG LLVM Githu

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews created this revision. eandrews added reviewers: vlastik, dylanmckay, bader. Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl. eandrews requested review of this revision. Functions pointers should be created with program address space. This patch fixes a crash on lvalue ref

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews updated this revision to Diff 379562. eandrews added a comment. Added a test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111566/new/ https://reviews.llvm.org/D111566 Files: clang/lib/CodeGen/CodeGenTypes.cpp clang/test/CodeGenSYCL/functionptr-addressspace.cpp Index: cl

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111566/new/ https://reviews.llvm.org/D111566 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111566: [SYCL] Fix function pointer address space

eandrews added inline comments. Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:636-638 +unsigned AS = PointeeType->isFunctionTy() + ? getDataLayout().getProgramAddressSpace() + : Context.getTargetAddressSpace(ETy); aa

<    1   2