[clang-doc] Use DiagnosticsEngine to handle diagnostic output #170219
+344 −220
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Contributor Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Member
| @llvm/pr-subscribers-clang-tools-extra Author: Paul Kirth (ilovepi) Changes[clang-doc] Use DiagnosticsEngine to handle diagnostic output Right now we use a combination of outs() and errs() to handle tool [clang-doc] Use static functions over the anonymous namespace [clang-doc] Reorder struct fields to have less padding Patch is 73.43 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170219.diff 21 Files Affected:
diff --git a/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp index 4efbbd34730cf..b83cd0bb8ef69 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.cpp +++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp @@ -9,6 +9,7 @@ #include "BitcodeReader.h" #include "llvm/ADT/IndexedMap.h" #include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TimeProfiler.h" #include "llvm/Support/raw_ostream.h" #include <optional> @@ -16,6 +17,8 @@ namespace clang { namespace doc { +static llvm::ExitOnError ExitOnErr("clang-doc error: "); + using Record = llvm::SmallVector<uint64_t, 1024>; // This implements decode for SmallString. @@ -717,8 +720,8 @@ llvm::Error addReference(FriendInfo *Friend, Reference &&R, FieldId F) { template <typename T, typename ChildInfoType> static void addChild(T I, ChildInfoType &&R) { - llvm::errs() << "invalid child type for info"; - exit(1); + ExitOnErr(llvm::createStringError(llvm::inconvertibleErrorCode(), + "invalid child type for info")); } // Namespace children: @@ -767,8 +770,9 @@ template <> void addChild(BaseRecordInfo *I, FunctionInfo &&R) { // parameters) or TemplateSpecializationInfo (for the specialization's // parameters). template <typename T> static void addTemplateParam(T I, TemplateParamInfo &&P) { - llvm::errs() << "invalid container for template parameter"; - exit(1); + ExitOnErr( + llvm::createStringError(llvm::inconvertibleErrorCode(), + "invalid container for template parameter")); } template <> void addTemplateParam(TemplateInfo *I, TemplateParamInfo &&P) { I->Params.emplace_back(std::move(P)); @@ -780,8 +784,8 @@ void addTemplateParam(TemplateSpecializationInfo *I, TemplateParamInfo &&P) { // Template info. These apply to either records or functions. template <typename T> static void addTemplate(T I, TemplateInfo &&P) { - llvm::errs() << "invalid container for template info"; - exit(1); + ExitOnErr(llvm::createStringError(llvm::inconvertibleErrorCode(), + "invalid container for template info")); } template <> void addTemplate(RecordInfo *I, TemplateInfo &&P) { I->Template.emplace(std::move(P)); @@ -799,8 +803,9 @@ template <> void addTemplate(FriendInfo *I, TemplateInfo &&P) { // Template specializations go only into template records. template <typename T> static void addTemplateSpecialization(T I, TemplateSpecializationInfo &&TSI) { - llvm::errs() << "invalid container for template specialization info"; - exit(1); + ExitOnErr(llvm::createStringError( + llvm::inconvertibleErrorCode(), + "invalid container for template specialization info")); } template <> void addTemplateSpecialization(TemplateInfo *I, @@ -809,8 +814,8 @@ void addTemplateSpecialization(TemplateInfo *I, } template <typename T> static void addConstraint(T I, ConstraintInfo &&C) { - llvm::errs() << "invalid container for constraint info"; - exit(1); + ExitOnErr(llvm::createStringError(llvm::inconvertibleErrorCode(), + "invalid container for constraint info")); } template <> void addConstraint(TemplateInfo *I, ConstraintInfo &&C) { I->Constraints.emplace_back(std::move(C)); diff --git a/clang-tools-extra/clang-doc/BitcodeReader.h b/clang-tools-extra/clang-doc/BitcodeReader.h index 4947721f0a06d..2144b77e7631d 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.h +++ b/clang-tools-extra/clang-doc/BitcodeReader.h @@ -29,7 +29,8 @@ namespace doc { // Class to read bitstream into an InfoSet collection class ClangDocBitcodeReader { public: - ClangDocBitcodeReader(llvm::BitstreamCursor &Stream) : Stream(Stream) {} + ClangDocBitcodeReader(llvm::BitstreamCursor &Stream, DiagnosticsEngine &Diags) + : Stream(Stream), Diags(Diags) {} // Main entry point, calls readBlock to read each block in the given stream. llvm::Expected<std::vector<std::unique_ptr<Info>>> readBitcode(); @@ -74,6 +75,7 @@ class ClangDocBitcodeReader { llvm::BitstreamCursor &Stream; std::optional<llvm::BitstreamBlockInfo> BlockInfo; FieldId CurrentReferenceField; + DiagnosticsEngine &Diags; }; } // namespace doc diff --git a/clang-tools-extra/clang-doc/BitcodeWriter.cpp b/clang-tools-extra/clang-doc/BitcodeWriter.cpp index 3a7ac6e2abcdd..650501d1d7606 100644 --- a/clang-tools-extra/clang-doc/BitcodeWriter.cpp +++ b/clang-tools-extra/clang-doc/BitcodeWriter.cpp @@ -769,7 +769,9 @@ bool ClangDocBitcodeWriter::dispatchInfoForWrite(Info *I) { emitBlock(*static_cast<FriendInfo *>(I)); break; case InfoType::IT_default: - llvm::errs() << "Unexpected info, unable to write.\n"; + unsigned ID = Diags.getCustomDiagID(DiagnosticsEngine::Error, + "Unexpected info, unable to write."); + Diags.Report(ID); return true; } return false; diff --git a/clang-tools-extra/clang-doc/BitcodeWriter.h b/clang-tools-extra/clang-doc/BitcodeWriter.h index 688f886b45308..1d81ece521826 100644 --- a/clang-tools-extra/clang-doc/BitcodeWriter.h +++ b/clang-tools-extra/clang-doc/BitcodeWriter.h @@ -17,6 +17,7 @@ #include "Representation.h" #include "clang/AST/AST.h" +#include "clang/Basic/Diagnostic.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" @@ -179,7 +180,8 @@ enum class FieldId { class ClangDocBitcodeWriter { public: - ClangDocBitcodeWriter(llvm::BitstreamWriter &Stream) : Stream(Stream) { + ClangDocBitcodeWriter(llvm::BitstreamWriter &Stream, DiagnosticsEngine &Diags) + : Stream(Stream), Diags(Diags) { emitHeader(); emitBlockInfoBlock(); emitVersionBlock(); @@ -264,6 +266,7 @@ class ClangDocBitcodeWriter { SmallVector<uint32_t, BitCodeConstants::RecordSize> Record; llvm::BitstreamWriter &Stream; AbbreviationMap Abbrevs; + DiagnosticsEngine &Diags; }; } // namespace doc diff --git a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp index 1e757101549c6..fc466e243c4a5 100644 --- a/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp +++ b/clang-tools-extra/clang-doc/HTMLMustacheGenerator.cpp @@ -15,6 +15,7 @@ #include "Generators.h" #include "Representation.h" #include "support/File.h" +#include "clang/Basic/Diagnostic.h" #include "llvm/Support/Error.h" #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/Mustache.h" @@ -187,11 +188,13 @@ Error MustacheHTMLGenerator::generateDocs( } auto File = MemoryBuffer::getFile(Path); - if (EC = File.getError(); EC) - // TODO: Buffer errors to report later, look into using Clang - // diagnostics. - llvm::errs() << "Failed to open file: " << Path << " " << EC.message() - << '\n'; + if (EC = File.getError(); EC) { + unsigned ID = CDCtx.Diags.getCustomDiagID(DiagnosticsEngine::Warning, + "Failed to open file: %0 %1"); + CDCtx.Diags.Report(ID) << Path << EC.message(); + JSONIter.increment(EC); + continue; + } auto Parsed = json::parse((*File)->getBuffer()); if (!Parsed) diff --git a/clang-tools-extra/clang-doc/Mapper.cpp b/clang-tools-extra/clang-doc/Mapper.cpp index 497b80cf97463..24e798af52e62 100644 --- a/clang-tools-extra/clang-doc/Mapper.cpp +++ b/clang-tools-extra/clang-doc/Mapper.cpp @@ -95,10 +95,10 @@ bool MapASTVisitor::mapDecl(const T *D, bool IsDefinition) { // this decl for some reason (e.g. we're only reporting public decls). if (Child) CDCtx.ECtx->reportResult(llvm::toHex(llvm::toStringRef(Child->USR)), - serialize::serialize(Child)); + serialize::serialize(Child, CDCtx.Diags)); if (Parent) CDCtx.ECtx->reportResult(llvm::toHex(llvm::toStringRef(Parent->USR)), - serialize::serialize(Parent)); + serialize::serialize(Parent, CDCtx.Diags)); } return true; } diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp index 929112f01fb43..4cc94fa167545 100644 --- a/clang-tools-extra/clang-doc/Representation.cpp +++ b/clang-tools-extra/clang-doc/Representation.cpp @@ -82,12 +82,11 @@ llvm::StringRef commentKindToString(CommentKind Kind) { llvm_unreachable("Unhandled CommentKind"); } -namespace { const SymbolID EmptySID = SymbolID(); template <typename T> -llvm::Expected<std::unique_ptr<Info>> +static llvm::Expected<std::unique_ptr<Info>> reduce(std::vector<std::unique_ptr<Info>> &Values) { if (Values.empty() || !Values[0]) return llvm::createStringError(llvm::inconvertibleErrorCode(), @@ -102,7 +101,7 @@ reduce(std::vector<std::unique_ptr<Info>> &Values) { // Return the index of the matching child in the vector, or -1 if merge is not // necessary. template <typename T> -int getChildIndexIfExists(std::vector<T> &Children, T &ChildToMerge) { +static int getChildIndexIfExists(std::vector<T> &Children, T &ChildToMerge) { for (unsigned long I = 0; I < Children.size(); I++) { if (ChildToMerge.USR == Children[I].USR) return I; @@ -111,7 +110,7 @@ int getChildIndexIfExists(std::vector<T> &Children, T &ChildToMerge) { } template <typename T> -void reduceChildren(std::vector<T> &Children, +static void reduceChildren(std::vector<T> &Children, std::vector<T> &&ChildrenToMerge) { for (auto &ChildToMerge : ChildrenToMerge) { int MergeIdx = getChildIndexIfExists(Children, ChildToMerge); @@ -123,7 +122,6 @@ void reduceChildren(std::vector<T> &Children, } } -} // namespace // Dispatch function. llvm::Expected<std::unique_ptr<Info>> @@ -402,7 +400,7 @@ BaseRecordInfo::BaseRecordInfo() : RecordInfo() {} BaseRecordInfo::BaseRecordInfo(SymbolID USR, StringRef Name, StringRef Path, bool IsVirtual, AccessSpecifier Access, bool IsParent) - : RecordInfo(USR, Name, Path), IsVirtual(IsVirtual), Access(Access), + : RecordInfo(USR, Name, Path), Access(Access), IsVirtual(IsVirtual), IsParent(IsParent) {} llvm::SmallString<16> Info::extractName() const { @@ -481,10 +479,11 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx, StringRef RepositoryUrl, StringRef RepositoryLinePrefix, StringRef Base, std::vector<std::string> UserStylesheets, + clang::DiagnosticsEngine &Diags, bool FTimeTrace) - : ECtx(ECtx), ProjectName(ProjectName), PublicOnly(PublicOnly), - FTimeTrace(FTimeTrace), OutDirectory(OutDirectory), - UserStylesheets(UserStylesheets), Base(Base) { + : ECtx(ECtx), ProjectName(ProjectName), OutDirectory(OutDirectory), + SourceRoot(std::string(SourceRoot)), UserStylesheets(UserStylesheets), + Base(Base), Diags(Diags), PublicOnly(PublicOnly), FTimeTrace(FTimeTrace) { llvm::SmallString<128> SourceRootDir(SourceRoot); if (SourceRoot.empty()) // If no SourceRoot was provided the current path is used as the default diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h index 79e9bfc291c3a..a3e779aa39bc4 100644 --- a/clang-tools-extra/clang-doc/Representation.h +++ b/clang-tools-extra/clang-doc/Representation.h @@ -15,11 +15,10 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_DOC_REPRESENTATION_H #include "clang/AST/Type.h" +#include "clang/Basic/Diagnostic.h" #include "clang/Basic/Specifiers.h" -#include "clang/Tooling/StandaloneExecution.h" -#include "llvm/ADT/APSInt.h" +#include "clang/Tooling/Execution.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringExtras.h" #include <array> #include <optional> #include <string> @@ -87,29 +86,29 @@ struct CommentInfo { // the vector. bool operator<(const CommentInfo &Other) const; - CommentKind Kind = CommentKind:: - CK_Unknown; // Kind of comment (FullComment, ParagraphComment, - // TextComment, InlineCommandComment, HTMLStartTagComment, - // HTMLEndTagComment, BlockCommandComment, - // ParamCommandComment, TParamCommandComment, - // VerbatimBlockComment, VerbatimBlockLineComment, - // VerbatimLineComment). - SmallString<64> Text; // Text of the comment. - SmallString<16> Name; // Name of the comment (for Verbatim and HTML). + std::vector<std::unique_ptr<CommentInfo>> + Children; // List of child comments for this CommentInfo. SmallString<8> Direction; // Parameter direction (for (T)ParamCommand). + SmallString<16> Name; // Name of the comment (for Verbatim and HTML). SmallString<16> ParamName; // Parameter name (for (T)ParamCommand). SmallString<16> CloseName; // Closing tag name (for VerbatimBlock). - bool SelfClosing = false; // Indicates if tag is self-closing (for HTML). - bool Explicit = false; // Indicates if the direction of a param is explicit - // (for (T)ParamCommand). + SmallString<64> Text; // Text of the comment. llvm::SmallVector<SmallString<16>, 4> AttrKeys; // List of attribute keys (for HTML). llvm::SmallVector<SmallString<16>, 4> AttrValues; // List of attribute values for each key (for HTML). llvm::SmallVector<SmallString<16>, 4> Args; // List of arguments to commands (for InlineCommand). - std::vector<std::unique_ptr<CommentInfo>> - Children; // List of child comments for this CommentInfo. + CommentKind Kind = CommentKind:: + CK_Unknown; // Kind of comment (FullComment, ParagraphComment, + // TextComment, InlineCommandComment, HTMLStartTagComment, + // HTMLEndTagComment, BlockCommandComment, + // ParamCommandComment, TParamCommandComment, + // VerbatimBlockComment, VerbatimBlockLineComment, + // VerbatimLineComment). + bool SelfClosing = false; // Indicates if tag is self-closing (for HTML). + bool Explicit = false; // Indicates if the direction of a param is explicit + // (for (T)ParamCommand). }; struct Reference { @@ -120,13 +119,13 @@ struct Reference { // "GlobalNamespace" as the name, but an empty QualName). Reference(SymbolID USR = SymbolID(), StringRef Name = StringRef(), InfoType IT = InfoType::IT_default) - : USR(USR), Name(Name), QualName(Name), RefType(IT) {} + : USR(USR), RefType(IT), Name(Name), QualName(Name) {} Reference(SymbolID USR, StringRef Name, InfoType IT, StringRef QualName, StringRef Path = StringRef()) - : USR(USR), Name(Name), QualName(QualName), RefType(IT), Path(Path) {} + : USR(USR), RefType(IT), Name(Name), QualName(QualName), Path(Path) {} Reference(SymbolID USR, StringRef Name, InfoType IT, StringRef QualName, StringRef Path, SmallString<16> DocumentationFileName) - : USR(USR), Name(Name), QualName(QualName), RefType(IT), Path(Path), + : USR(USR), RefType(IT), Name(Name), QualName(QualName), Path(Path), DocumentationFileName(DocumentationFileName) {} bool operator==(const Reference &Other) const { @@ -146,6 +145,10 @@ struct Reference { SymbolID USR = SymbolID(); // Unique identifier for referenced decl + InfoType RefType = InfoType::IT_default; // Indicates the type of this + // Reference (namespace, record, + // function, enum, default). + // Name of type (possibly unresolved). Not including namespaces or template // parameters (so for a std::vector<int> this would be "vector"). See also // QualName. @@ -156,9 +159,6 @@ struct Reference { // Name. SmallString<16> QualName; - InfoType RefType = InfoType::IT_default; // Indicates the type of this - // Reference (namespace, record, - // function, enum, default). // Path of directory where the clang-doc generated file will be saved // (possibly unresolved) llvm::SmallString<128> Path; @@ -278,21 +278,21 @@ struct MemberTypeInfo : public FieldTypeInfo { Other.Description); } + std::vector<CommentInfo> Description; + // Access level associated with this info (public, protected, private, none). // AS_public is set as default because the bitcode writer requires the enum // with value 0 to be used as the default. // (AS_public = 0, AS_protected = 1, AS_private = 2, AS_none = 3) AccessSpecifier Access = AccessSpecifier::AS_public; - - std::vector<CommentInfo> Description; // Comment description of this field. bool IsStatic = false; }; struct Location { Location(int StartLineNumber = 0, int EndLineNumber = 0, StringRef Filename = StringRef(), bool IsFileInRootDir = false) - : StartLineNumber(StartLineNumber), EndLineNumber(EndLineNumber), - Filename(Filename), IsFileInRootDir(IsFileInRootDir) {} + : Filename(Filename), StartLineNumber(StartLineNumber), + EndLineNumber(EndLineNumber), IsFileInRootDir(IsFileInRootDir) {} bool operator==(const Location &Other) const { return std::tie(StartLineNumber, EndLineNumber, Filename) == @@ -310,40 +310,24 @@ struct Location { std::tie(Other.StartLineNumber, Other.EndLineNumber, Other.Filename); } - int StartLineNumber = 0; // Line number of this Location. + SmallString<32> Filename; + int StartLineNumber = 0; int EndLineNumber = 0; - SmallString<32> Filename; // File for this Location. - bool IsFileInRootDir = false; // Indicates if file is inside root directory + bool IsFileInRootDir = false; }; /// A base struct for Infos. struct Info { Info(InfoType IT = InfoType::IT_default, SymbolID USR = SymbolID(), StringRef Name = StringRef(), StringRef Path = StringRef()) - : USR(USR), IT(IT), Name(Name), Path(Path) {} + : Path(Path), Name(Name), USR(USR), IT(IT) {} Info(const Info &Other) = delete; Info(Info &&Other) = default; - virtual ~Info() = default; Info &operator=(Info &&Other) = default; - SymbolID USR = - SymbolID(); // Unique identifier for the decl described by this Info. - InfoType IT = InfoType::IT_default; // InfoType of this particular Info. - SmallString<16> Name; // Unqualified name of the decl. - llvm::SmallVector<Reference, 4> - Namespace; // List of parent namespaces for this decl. - std::vector<CommentInfo> Description; // Comment description of this decl. - llvm::SmallString<128> Path; // Path of directory where the clang-doc - // generated file will be saved - - // The name used for the file that this info is documented in. - // In the JSON generator, infos are documented in files with mangled names. - // Thus, we keep track of the physical filename for linking purposes. - SmallString<16> DocumentationFileName; - void mergeBase(Info &&I); bool mergeable(const Info &Other); @@ -354,6 +338,29 @@ struct Info { /// Returns the basename that should be used for this Info. llvm::SmallString<16> getFileBaseName() const; + + // Path of directory where the clang-doc generated file will be saved. + llvm::SmallString<128> Path; + + // Unqualified name of the decl. + SmallString<16> Name; + + // The name used for the file that this info is documented in. + // In the JSON generator, infos are documented in files with mangled names. + // Thus, we keep track of the physical filename for linking purposes. + SmallString<16> DocumentationFileName; + + // List of parent namespaces for this decl. + llvm::SmallVector<Reference, 4> Namespace; + + // Unique identifier for the decl described by this Info. + SymbolID USR = SymbolID(); + + // InfoType of this particular Info. + InfoType IT = InfoType::IT_default; + + // Comment description of this decl. + std::vector<CommentInfo> Description; }; // Info for namespaces. @@ -427,21 +434,21 @@ struct FunctionInfo : public SymbolInfo { void merge(... [truncated] |
ilovepi force-pushed the users/ilovepi/cd-diagnostics branch from 31d597d to 10f110c Compare This was referenced Dec 1, 2025
Right now we use a combination of outs() and errs() to handle tool output. Instead, we can use existing diagnostic support in clang and LLVM to ensure our tool has a consistent behavior with other tools.
ilovepi force-pushed the users/ilovepi/cd-diagnostics branch from 10f110c to 34e1ed1 Compare 
🐧 Linux x64 Test Results
Failed Tests(click on a test name to see its output) Clang ToolsClang Tools.clang-doc/invalid-options.cppIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
[clang-doc] Use DiagnosticsEngine to handle diagnostic output
Right now we use a combination of outs() and errs() to handle tool
output. Instead, we can use existing diagnostic support in clang and
LLVM to ensure our tool has a consistent behavior with other tools.