This is an archive of the discontinued LLVM Phabricator instance.

[clang-tools-extra] Fix switch coverage warning
ClosedPublic

Authored by echristo on Dec 13 2019, 12:22 PM.

Details

Summary

... I think this is right?

Add support in the switch statement for addr spaces. I couldn't get a magical API combination to get just the address space off the top of my head, but wanted to get this out.

Diff Detail

Event Timeline

echristo created this revision.Dec 13 2019, 12:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 13 2019, 12:22 PM

Unit tests: pass. 60867 tests passed, 0 failed and 726 were skipped.

clang-format: pass.

Build artifacts: console-log.txt, CMakeCache.txt, test-results.xml, diff.json

It should be possible to test this by adapting a test case from the original ed8dadb, but I'm not certain...

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
525

Not very familiar with this code, but it looks like this is feeding the arg back into a builder, and ed8dadb added support for sending LangAS itself to the builder.

So I don't think we should stringify it, but just send the LangAS

526

I think the stored thing is a LangAS (an enum), rather than a pointer to one.

echristo updated this revision to Diff 233857.Dec 13 2019, 12:53 PM

Fix for some slightly better API uses.

echristo marked 2 inline comments as done.Dec 13 2019, 12:58 PM
This revision was not accepted when it landed; it landed in state Needs Review.Dec 13 2019, 1:00 PM
This revision was automatically updated to reflect the committed changes.

It should be possible to test this by adapting a test case from the original ed8dadb, but I'm not certain...

This concern was not addressed/answered.

The commit https://reviews.llvm.org/rG5623bd52acd34db2e9cfc11d1510407610a14db0 claims that this was reviewed by rsmith, but I don't see rsmith's LGTM here..