Thanks everyone for the helpful review. I've also tested with -O[0-3] as suggested by @EricWF. I will land this by the end of the day. Cheers!

Rebase to trigger ci.

Rebase to trigger CI.

Fix tests with C++03 and rebase.

Enable tests when using appropriate standard versions.

Thanks again for the thoughtful review!

• Use more readable == operand order in assertions
• Remove irrelevant test case
• Guard std::shared_ptr<T[]> specialization with check for standard version greater than or equal to C++14

Re-add test I mistakenly removed.

• Bolster new and existing tests.
• Slight style fixes.
• Mark 3548 as complete in status file.
• Remove misguided addition to constraints on template parameter.

lgtm!

This introduces a very clean level of indirection imo. +1 for adapting the clang documentation as well!

Rebase to ensure patch still builds cleanly.

Rebase to retrigger CI.

Thanks for the kind words @awarzynski! Glad I could help 🙂

Apply clang-format.

Rebase on main.

Update comments and formatting in response to feedback and clang-tidy.

Update language to use cmake variables over ugly environment variable workaround.

Document use of CMake variable GCC_INSTALL_PREFIX over ugly workaround.

I've hopefully addressed your comments. Thanks for the review @jhuber6!

Remove CMake example. Use more generic language.

Add LLVM header comment to buffer test.

@awarzynski Sounds good! I'll add a few more reviewers and wait a bit before merging.

lgtm!

Update revision with supporting functionality test.

@awarzynski This is just the minimum to pull the test into gtest. I would like to add documentation and death tests for failure conditions - do you have any suggestions for this? I think once we have those two criteria met we can add more reviewers.

Notice that in PrescanAndSemaAction, the first check fails with a scanning error, and the second with a parsing error. This is not the case for PrescanAndParseAction, which makes me think the second check should fail with a parsing error, and the tests should be adjusted in the same way.

Thanks again for the rigorous review!

Pull all of previous test harness into current implementation.

LGTM!

Resolve warnings against const auto without &.

Remove erroneous static qualifiers. Register all tests with crash handler.

Merged in commit 080d48f279e2.

In D100442#2693549, @MehdiChinoune wrote:

Could someone push this change p, please.

Resolve warnings by removing utility function.

Merged in ccef0adc5968.

Use more descriptive variable names.

@awarzynski Thanks again for the review! I have another patch ready to address this.

@awarzynski Thank you for the fantastic review comments! I've tried to address most of them - please let me know if this looks good to you.

Use more descriptive variable names and better comment tests. Address clang-tidy warnings and phabricator reviews.

Break original test into two test cases. Add simple sanity-check test cases. Add death test to verify failure message.

If deemed reasonable, I'm happy to extend this to test more of the api as well - I'm open to suggestions. I'm also happy to keep this patch very short.

Use GTest EXPECT_EQ over potentially ambiguous return value, redirect SUT IO to stderr.

Fix header include order.

Fix clang-format and clang-tidy warnings.

In D98303#2636170, @klausler wrote:

In D98303#2636054, @ashermancinelli wrote:

@klausler D98652 hopefully demonstrates that runtime crashes are caught by death tests. This revision has been rebased on D98652 - I hope this sufficiently demonstrates that these tests accurately catch crashes. We could also work on another patch that intentionally crashes each of the APIs tested above not already tested in D98652.

I wasn't asking about crashes in the runtime. I was asking about catching incorrect results and reporting failures.

I will try again. If you modify the decimal or runtime library in such a way as to cause f18 to emit incorrectly formatted results, does your rewritten test fail? Let me know when you have confirmed this by observation.

@klausler D98652 hopefully demonstrates that runtime crashes are caught by death tests. This revision has been rebased on D98652 - I hope this sufficiently demonstrates that these tests accurately catch crashes. We could also work on another patch that intentionally crashes each of the APIs tested above not already tested in D98652.

Rebase on recent main branch to incorporate terminator crash tests

Remove superfluous comments

Fix header guards to conform to LLVM style guide

More thoroughly comment runtime crash tests.

I have submitted D98652 to discuss similar efforts.

@awarzynski I've actually been working on a similar patch, perhaps we could merge our efforts?

These tests seem to adequately verify the runtime crash mechanism and would support other gtest tests that must catch runtime crashes.

LGTM

@klausler I think I better understand your concerns now. How would you feel about this patch if I un-removed the original test, we refine this test, and ten

@klausler I reproduce all of the same test cases, so the test coverage should be identical and should catch all of the same bugs. I'd like to increase coverage, but the patch as-is should keep it the same.

Use fully qualified auto to address clang-tidy warnings.