This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Refactor IncludeInserter
ClosedPublic

Authored by njames93 on Jul 13 2020, 6:27 AM.

Details

Summary

Simplified how IncludeInserter is used in Checks by abstracting away the SourceManager and PPCallbacks inside the method registerPreprocessor.
Changed checks that use IncludeInserter to no longer use a std::unique_ptr, instead the IncludeInserter is just a member of the class thats initialized with an IncludeStyle.
Saving an unnecessary allocation.

This results in the removal of the field IncludeSorter::IncludeStyle from the checks, as its wrapped in the IncludeInserter.
No longer need to create an instance of the IncludeInserter in the registerPPCallbacks, now that method only needs to contain:

Inserter.registerPreprocessor(PP);

Also added a helper method to IncludeInserter called createMainFileInclusionInsertion, purely sugar but does better express intentions.

Diff Detail

Event Timeline

njames93 created this revision.Jul 13 2020, 6:27 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2020, 6:27 AM
aaron.ballman added inline comments.Jul 17 2020, 7:45 AM
clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp
55

I think it might be useful to add something like ; did you remember to call registerPreprocessor()? to the assertion message, as that seems like it would be the most likely issue if SourceMgr was null.

clang-tools-extra/clang-tidy/utils/IncludeInserter.h
59

retrived -> retrieved
ClangTidyOptios -> ClangTidyOptions

70

or if inclusion -> or if the inclusion

76

or if inclusion -> or if the inclusion

91

In-class initialize this to nullptr instead of leaving it indeterminate?

njames93 updated this revision to Diff 278807.Jul 17 2020, 9:33 AM
njames93 marked 5 inline comments as done.

Address reviewer comments

This revision is now accepted and ready to land.Jul 17 2020, 9:38 AM
This revision was automatically updated to reflect the committed changes.