Page MenuHomePhabricator

orivej (Orivej Desh)
User

Projects

User does not belong to any projects.

User Details

User Since
May 15 2017, 3:07 PM (109 w, 3 d)

Recent Activity

Dec 9 2018

orivej added a comment to rL343285: [cxx2a] P0641R2: (Some) type mismatches on defaulted functions only.

Cross reference to a bug in diagnostics observed after this change: https://bugs.llvm.org/show_bug.cgi?id=39933

Dec 9 2018, 5:21 PM
orivej added a comment to D45898: [SemaCXX] Mark destructor as referenced .

I have noticed that this change breaks seemingly valid code:

class A { protected: ~A(); };
struct B : A {};
B f() { return B(); }
B g() { return {}; }

f compiles, but g fails with temporary of type 'A' has protected destructor. (g++ 8.2 compiles this file.)

Dec 9 2018, 4:26 AM

Dec 8 2018

orivej added a comment to D45898: [SemaCXX] Mark destructor as referenced .

Actually, arc-list-init-destruct.mm crashes Clang 7 with the same backtrace as this test case, and Clang trunk generates similar assembly (to the the point that I could essentially copy the // CHECK comments from the .mm test to a .cpp test), so I'm not sure if adding a .cpp test is valuable…

Dec 8 2018, 4:29 PM
orivej added a comment to D45898: [SemaCXX] Mark destructor as referenced .

The committed test does not crash Clang 7, but the following test does, yet it compiles without any warnings by the current Clang trunk thanks to this fix.

struct A { ~A(); };
struct B : A {};
struct C { C();  B b; };
struct D { C c, d; };
D f() { return {}; }
Dec 8 2018, 3:49 PM

Nov 22 2018

orivej added a comment to D52142: [DebugInfo] Fix build when std::vector::iterator is a pointer.

Do you have an example of a c++ std library where this happens?

Nov 22 2018, 6:07 PM

Nov 19 2018

orivej added a comment to D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end.

@vsk, I think this is ready, but I don't have the merge rights. Could you commit it and D53231 for me?

Nov 19 2018, 1:11 AM

Oct 31 2018

orivej added a comment to D53231: [Sema] Fix PR38987: keep end location of a direct initializer list.

D41921 was the last big change necessary to fix clang crashes on code coverage, but without the fixup in this review clang crashes on about 1.5 times more projects that I maintain than before. It seems useful to include this in the 7.0.1 release. (After the fixup the only crasher is scipy, being dealt with in D53244.)

Oct 31 2018, 4:28 PM
orivej updated the diff for D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end.

Remove unrelated change.

Oct 31 2018, 4:15 PM
orivej updated the diff for D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end.

Use relative line offset.

Oct 31 2018, 4:13 PM

Oct 15 2018

orivej added inline comments to D52904: [hot-cold-split] fix static analysis of cold regions.
Oct 15 2018, 3:37 PM
orivej added a comment to D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end.

Thanks! Could you merge this (after a while to let others review)?

Oct 15 2018, 12:12 AM

Oct 14 2018

orivej added a comment to D53231: [Sema] Fix PR38987: keep end location of a direct initializer list.

I don't have the complete picture yet. (This is why the summary is so short.)
I have looked up that IK_DirectList Kind was known not to carry a valid ParenRange since https://github.com/llvm-mirror/clang/commit/188158db29f50443b6e412f2a40c800b2669c957, and that PerformConstructorInitialization acquired BraceLoc arguments specifically to support CXXTemporaryObjectExpr: https://github.com/llvm-mirror/clang/commit/1245a54ca6e9c5b14196461dc3f84b24ea6594b1#diff-d7cc8293491a9fdddee7ba857c028256R5921 , but then it appears that CXXTemporaryObjectExpr Kind has acquired a valid ParenRange, except when it is instantiated from a template…

Oct 14 2018, 12:02 AM

Oct 13 2018

orivej updated subscribers of D53231: [Sema] Fix PR38987: keep end location of a direct initializer list.
Oct 13 2018, 11:56 PM
orivej updated the diff for D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end.

Fix typo

Oct 13 2018, 9:51 AM
orivej updated the diff for D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end.

This looks better to me. (There is no difference in check-clang test results.)

Oct 13 2018, 7:23 AM
orivej created D53244: [Coverage] Fix PR39258: support coverage regions that start deeper than they end.
Oct 13 2018, 6:52 AM

Oct 12 2018

orivej created D53231: [Sema] Fix PR38987: keep end location of a direct initializer list.
Oct 12 2018, 5:19 PM
orivej added inline comments to D41921: [Parse] Forward brace locations to TypeConstructExpr.
Oct 12 2018, 4:12 PM

Oct 3 2018

orivej added inline comments to rL334246: Expose a single global file open function..
Oct 3 2018, 5:31 AM
orivej added inline comments to rL334246: Expose a single global file open function..
Oct 3 2018, 5:28 AM

Oct 2 2018

orivej accepted D52724: [ELF] - Do not forget to include to .dymsym symbols that were converted to Defined..

Thank you! This patch works as intended.

Oct 2 2018, 5:22 PM
orivej abandoned D52698: Revert r330966: Replace SharedSymbols with Defined when creating copy relocations..

Discarded in favor of https://reviews.llvm.org/D52724

Oct 2 2018, 5:20 PM

Sep 29 2018

orivej created D52698: Revert r330966: Replace SharedSymbols with Defined when creating copy relocations..
Sep 29 2018, 4:35 PM

Sep 17 2018

orivej added a comment to D52142: [DebugInfo] Fix build when std::vector::iterator is a pointer.

Do you have an example of a c++ std library where this happens?

Sep 17 2018, 3:48 AM

Sep 16 2018

orivej added a comment to D52142: [DebugInfo] Fix build when std::vector::iterator is a pointer.

I have slightly extended the description.

Sep 16 2018, 2:46 PM
orivej updated the summary of D52142: [DebugInfo] Fix build when std::vector::iterator is a pointer.
Sep 16 2018, 2:40 PM

Sep 15 2018

orivej added a comment to D52142: [DebugInfo] Fix build when std::vector::iterator is a pointer.

OK, please commit this into trunk first on my behalf!

Sep 15 2018, 11:55 PM
orivej added a comment to D52142: [DebugInfo] Fix build when std::vector::iterator is a pointer.

Without this, the error (abbreviated) is:

lib/CodeGen/AsmPrinter/DwarfDebug.cpp: error: 'llvm::MapVector<>::const_iterator' (aka 'const std::pair<> *') is not a class, namespace, or enumeration
Sep 15 2018, 4:41 PM
orivej created D52142: [DebugInfo] Fix build when std::vector::iterator is a pointer.
Sep 15 2018, 4:36 PM

Sep 4 2018

orivej added a comment to D51639: [LV] Fix PR38786 - consider first order recurrence phis non-uniform.

I think this is ready. Would someone like to commit it?

Sep 4 2018, 12:23 PM
orivej added inline comments to D51639: [LV] Fix PR38786 - consider first order recurrence phis non-uniform.
Sep 4 2018, 9:40 AM
orivej updated the diff for D51639: [LV] Fix PR38786 - consider first order recurrence phis non-uniform.

Specify in the test comment that this is not about all phis, but only about first order recurrence phis.

Sep 4 2018, 9:34 AM
orivej updated the diff for D51639: [LV] Fix PR38786 - consider first order recurrence phis non-uniform.

Renamed %i32as64 to %i64next.

Sep 4 2018, 9:15 AM
orivej created D51639: [LV] Fix PR38786 - consider first order recurrence phis non-uniform.
Sep 4 2018, 8:57 AM

Aug 24 2018

Herald updated subscribers of D47751: [lsan] Do not check for leaks in the forked process.
Aug 24 2018, 10:35 PM

Sep 14 2017

orivej added inline comments to D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead..
Sep 14 2017, 1:37 PM
orivej accepted D37863: Fix bug 34608 by moving private header out of public header.WindowsManifestMerger.h should not include llvm/Config/config.h, since it is private. The include has been moved to the source instead..

This fixes the issue. Thank you!

Sep 14 2017, 1:25 PM
orivej added a comment to D35425: Implement parsing and writing of a single xml manifest file..

WindowsManifestMerger.h should not include llvm/Config/config.h: https://bugs.llvm.org/show_bug.cgi?id=34608

Sep 14 2017, 12:08 PM
orivej added a comment to D36255: Integrate manifest merging library into LLD..

This broke building LLD with an installed LLVM: https://bugs.llvm.org/show_bug.cgi?id=34608

Sep 14 2017, 12:07 PM

Jul 24 2017

orivej added a comment to D35509: Covnert .[cd]tors to .{init,fini}_array using synthetic section..

@ruiu I'd appreciate if you continue with this review.

Jul 24 2017, 4:50 AM

Jul 18 2017

orivej added a comment to D35487: [ELF] - Represent .init_array/.fini_array via synthetic sections..

It should be possible to implement this, but I do not think that is what bfd do when mixing them. So why LLD should?

I do not know how bfd interprets this script, but its test checks that they are mixed: https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=ld/testsuite/ld-elf/init-mixed.c;h=f401ded4d702be49669ecdc7893f65cc70e9fa7c;hb=HEAD
The need to mix .ctors with .init_array is the reason why gcc could not just add support for .init_array in crtbegin.o and had to change the linker.

Probably it worth to push it on review to phab then.

@ruiu has proposed another approach: https://bugs.llvm.org/show_bug.cgi?id=31224#c26, https://reviews.llvm.org/D35509

Jul 18 2017, 2:44 AM

Jul 17 2017

orivej added a comment to D35487: [ELF] - Represent .init_array/.fini_array via synthetic sections..

The other approach I can think of is to convert .{dtors,ctors} to .{init,fini}_array internally so that they are processed as if they were .{init,fini}_array from the beginning.

Jul 17 2017, 11:24 AM
orivej added a comment to D35487: [ELF] - Represent .init_array/.fini_array via synthetic sections..

Idea is next: we can do the same for .ctors/.dtors in another patch.
Synthetic section for .ctors/.dtors should know how to change sorting rules on fly depending on output section name.

Jul 17 2017, 9:27 AM

Jul 6 2017

orivej added a comment to D33680: [ELF] - Resolve references properly when using .symver directive.

lld incorrectly resolves versioned symbols to zeros when they are in a static library. Here is an example that should loop, but it crashes: https://gist.github.com/orivej/8379fbd550022e966f77d1516e31702a

Jul 6 2017, 6:46 PM

May 15 2017

orivej added inline comments to D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+.
May 15 2017, 3:16 PM