Remove intentional change to unrelated comment

after looking at this for a bit, I think this is probably the way to go. Using a template in ScopedPrinter.h has some problems, because there are types like ulittle32_t that won't satisfy is_integral. Using conversion also has its issues, like making some calls ambiguous.

@abhina.sreeskantharajan would you be able to test this out on AIX?

Sorry, I've disabled that portion of the test for now. https://reviews.llvm.org/rGfa56e362af475e0758cfb41c42f78db50da7235c. Please let me know if there are lingering issues. I'm going to try to dig in and get this working properly later today.

git clang-format HEAD~ formatted it that way

Remove most commented out code. Specifically leaving the PrintNumber ones, since they are clear about what should be re-enabled.

clang-format

Fix enable_if and update getInteger to allow more unsigned types

Fix typo in test code.

remove unused yaml input files for testing

I've split this into smaller patch sets by project:

Restore check lines in test.

Remove unrelated changes, due to bad commit.

ugh. I accidentally commited some temporary work here.

Address comments.

also, forward fix in https://reviews.llvm.org/D146739

@abhina.sreeskantharajan is there a way to test AIX things locally?

Ugh, I see this issue. I missed that the "INF/inf" discrepancy also happens for JSON output, but only changed the non-JSON test string. I should have a forward fix very soon. If that causes issues, we'll either need to revert the original patch, the 2 fixes and the HasHistogram patch, or disable the test for float/double,until we sort it out.

@jhenderson sure, I can set aside some time for that. I'm a bit under the weather, so it may take me a day or so to get to it, though.

In D146492#4211751, @junhee-yoo wrote:

In D146492#4210239, @paulkirth wrote:

@junhee-yoo can you try https://reviews.llvm.org/D146544? I think that may be a good solution here, so long as @jhenderson agrees it won't run into problems w/ overflow.

I'd checked your change and I think it will work for ELFDumper.cc but in my humble opinion, calling printNumber() with size_t still have a potential problem who doesn't know about.

By the way, @paulkirth if you mind, would you tell me your Mac build env and command to me for reproducing success build? If this problem can be solved by build config, Maybe the proper way to solve this is changing build configs rather than changing the code.

In D146492#4208618, @paulkirth wrote:

Hi, sorry this is causing you a problem. I'm surprised our builders haven't picked this up. I'm also unable to reproduce this locally on a Mac or Linux machine.

@junhee-yoo you may want to try your code w/ top-of-tree again. looks like someone added some static_cast to those APIs earlier today, so you may no longer have an issue. I didn't even notice that had landed earlier when I proposed changing to uint32_t.

In D137091#4209313, @hans wrote:

This broke some Chromium builds: https://bugs.chromium.org/p/chromium/issues/detail?id=1426287

It turns out we have a tool that consumes this JSON. Maybe that one can be made to accept the encoding both before and after this change, but there may be others.

That raises the question of what the compatibility story is here. Are we okay with breaking all consumers of this JSON?

Split out save/restore tests for SCS into its own file

@junhee-yoo can you try https://reviews.llvm.org/D146544? I think that may be a good solution here, so long as @jhenderson agrees it won't run into problems w/ overflow.

fix up in D146542.

so it looks like there are additional platform specific behaviors to how things like infinite are printed (inf vs INF, etc.). @abhina.sreeskantharajan I should be able to forward fix this fairly soon, probably in an hour(commute + meeting), but all that should be required is to adjust the test string the same way we do for max doubles/floats so that it's dynamically created and will thus match on all platforms.

Sorry for the trouble. looking now

Hi, sorry this is causing you a problem. I'm surprised our builders haven't picked this up. I'm also unable to reproduce this locally on a Mac or Linux machine.

Remove outdated diagnostics and add tests for SCS + save/restore.

Fix comment and add FIXME

Update test to avoid implementation specific output.

• fix comment string for /*IsGnu=*/

Address comments.

Rebase

arc diff git merge-base HEAD origin --update D137095

Fix Rebase

• remove const qualifiers from StringRef

Add missing blank line.

Add back funcion removed when rebasing

Rebase and remove const qualifier from StringRef.

Add missing blank line between functions.

Rebase

Address comments.

Add comment about the rationale for how we handle JSON output.

Address comments

Minor update to tests.

@jrtc27 @asb Any feedback on this? I'd like to land it, but wanted RISC-V maintainers to take a look. If there are no objections, I'll land this tomorrow.

Address comments.

Thanks for the fix. That change looks good to me. Its unfortunate that we can express that ordering is unimportant in lit for some range of code. The only reason I found the issue here was that I had a similar problem when testing stack layout analysis, and never updated my build flags.