Could somebody review this?

Sorry, I don't understand the issue and can't write a better description.
Since this test triggers an assert in getCommonPtr that it "Cannot retrieve a NULL type pointer" of Ty->getPointeeType(), the fix just checks that the pointee is not null before using isVolatileQualified.

It seems that Yandex is using their own patch for the same behavior, so we can still remove it from llvm as unused?

FWIW raw pointer iterators are used at Yandex [1] and sometimes uncover issues like [2] (although othertimes they hide issues that would have been prevented by wrapped iterators).

The char * iterator feature could have been enabled in libcxx via _LIBCPP_RAW_ITERATORS config definition until https://reviews.llvm.org/D79323.

I suppose that the newer versions are generally more restrictive than the older ones. Old libstdc++ used to bring std names into the global scope.

My C++ library does not allow even this:

#include <algorithm>
#include <string>

If you approve please merge on my behalf.

I do not have commit access, could you please commit on my behalf?

I have reproduced the error with Clang 7 and 10. I am compiling with a slightly customized version of a fresh libcxx.

I have tried that first, but I should have dereferenced too. Done.

Could you commit this on my behalf? Please edit the comment if you wish.

Add a comment that this affects only MSVC >= 19.15, < 19.20.

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

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.)

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…

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 {}; }

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

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

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.)

Remove unrelated change.

Use relative line offset.

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

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…

Fix typo

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

Thank you! This patch works as intended.

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

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

I have slightly extended the description.

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

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

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

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

Renamed %i32as64 to %i64next.

This fixes the issue. Thank you!

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

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

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

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

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.

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.

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