LGTM

FWIW, this change broke building libaom: https://bugs.chromium.org/p/aomedia/issues/detail?id=1945

LGTM, thanks!

Sounds sensible to me; I've tested the patch in the setup that showed the issue this original patch tried to fix, and it seems to work fine there.

It seems like this started triggering assertions in builds, I was about to file a bug about it, but it seems someone else already beat me to it, in https://bugs.llvm.org/show_bug.cgi?id=37686.

LGTM, thanks!

In D47002#1102688, @mstorsjo wrote:

In D47002#1102687, @xbolva00 wrote:

Just for info.. Windows has also some kind of unlocked stdio via _nolock functions :)

Yes, I saw that as well :-) However it doesn't seem to be available in the system DLL "msvcrt.dll" which mingw targets by default. (Nowadays one can target the more modern ucrtbase.dll by default as well, and there they'd be available.)

In D47002#1102687, @xbolva00 wrote:

Just for info.. Windows has also some kind of unlocked stdio via _nolock functions :)

Changed String into IntOrString and changed the test to exercise both value types.

Back to the original form of the check, but using sizeof(BitmapInfoHeader) instead of a separate constant.

Changed to check specifically for the PNG magic header, giving credit where it's due in the commit message :-)

In D46818#1098090, @amccarth wrote:

I kinda wish the test was broken up a bit. The stringtable bundle logic is distinctly different than just applying the keywords to other resource types, so that would be a natural place to break this apart.

Split the tests into two files.

In D46777#1097475, @javed.absar wrote:

In D46777#1096889, @mstorsjo wrote:

Separated the blocks, as requested.

I think you forgot to upload updated patch.

Actually updated the patch

Accidentally submitted initally before adding llvm-commits as subscriber, will resubmit.

Separated the blocks, as requested.

Simplified the estimate, elaborated the comment a little.

In D46752#1096183, @xbolva00 wrote:

I commited it again, you do not need to revert your revert.

In D46752#1096016, @spatel wrote:

LGTM - thanks!
Note (future patches): just glancing at the surrounding cases, it seems like we're missing similar return type checks for other calls.

Made the requested adjustments.

In D46752#1095878, @spatel wrote:

I haven't looked in detail at the failure or discussion in D46285 or https://bugs.llvm.org/show_bug.cgi?id=37408 , so I might be missing something.
Why are we getting this far in the 1st place? Shouldn't we have avoided this problem in TargetLibraryInfoImpl::isValidProtoForLibFunc() by checking that the return type is wrong?

In D45788#1095683, @grimar wrote:

Sorry, but I had to revert this in r332085 "[ELF] - Revert of: r332038, r332054, r332060, r332061, r332062, r332063"

In D46285#1094989, @xbolva00 wrote:

In D46285#1094917, @mstorsjo wrote:

This caused lots of assertion errors in building code with mingw-w64 headers (where snprintf can be a static inline function), see https://bugs.llvm.org/show_bug.cgi?id=37408. I would like to revert this change to unbreak things until this can be fixed.

C standard says:
int snprintf(char * restrict s, size_t n, const char * restrict format, ...);

This caused lots of assertion errors in building code with mingw-w64 headers (where snprintf can be a static inline function), see https://bugs.llvm.org/show_bug.cgi?id=37408. I would like to revert this change to unbreak things until this can be fixed.

Ping @ruiu

Adapted to the review comments; using std::vector<StringRef> + llvm::join to queue up the output, excluding *.c as well, added a reference to the documentation on the matter, simplified the line parsing significantly using @zturner's hints about better StringRef helpers to use.

Reverted in SVN r331858.

In D46520#1092233, @rnk wrote:

Please don't do this, this is actually really problematic, since #line directives lose information about what's a system header. That means the result of -E usually won't compile, since Windows headers are typically full of warnings and default-error warnings.