[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. After much digging, it looks like the lit config is never initialized in clang-tools-extra like it is in the other projects. REQUIRES et.al. work properly once that's in there (see D44708 ). Once that lands I'll reland this and

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Huh, something weird is going on there. What about the other way around, `REQUIRES: linux` ? Repository: rL LLVM https://reviews.llvm.org/D41102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. In https://reviews.llvm.org/D41102#1041791, @lebedev.ri wrote: > Have you tried something more broad, like > `UNSUPPORTED: mingw32,win32` > ? That wasn't working either, confusingly, at least on the local windows machine I have. Repository: rL LLVM

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. In https://reviews.llvm.org/D41102#1041773, @juliehockett wrote: > I was just thinking of disabling the one test that has an issue > (class-in-function) on Windows -- the filename is only used in generating > *some* USRs, so all of the other ones are fine. We ran

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. I was just thinking of disabling the one test that has an issue (class-in-function) on Windows -- the filename is only used in generating *some* USRs, so all of the other ones are fine. We ran into some issues with that though, since `UNSUPPORTED: system-windows`

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Hm, or possibly you could just pass the triple to clang? Repository: rL LLVM https://reviews.llvm.org/D41102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. So what part is failing, specifically? The SHA1 blobs of USR's differ in the llvm-bcanalyzer dumps? The actual filenames `%t/docs/bc/` differ? I guess both? First one you should be able to handle by replacing the actual values with a regex (i'd guess ` op19=226

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:230 + prepRecordData(ID); + for (const char C : RecordIdNameMap[ID].Name) Record.push_back(C); + Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_SETRECORDNAME, Record); lebedev.ri wrote: >

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. In https://reviews.llvm.org/D41102#1034919, @lebedev.ri wrote: > Since the commit was reverted, did you mean to either recommit it, or reopen > this (with updated diff), so it does not get lost? Relanded in r327295. Comment at:

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Since the commit was reverted, did you mean to either recommit it, or reopen this (with updated diff), so it does not get lost? Repository: rL LLVM https://reviews.llvm.org/D41102 ___ cfe-commits mailing list

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Might have been better to not start landing until the all differentials are understood/accepted, but i understand that it is not really up to me to decide. Let's hope nothing in the next differentials will require changes to this initial code :)

[PATCH] D41102: Setup clang-doc frontend framework

This revision was automatically updated to reflect the committed changes. juliehockett marked 11 inline comments as done. Closed by commit rL327102: [clang-doc] Setup clang-doc frontend framework (authored by juliehockett, committed by ). Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D41102: Setup clang-doc frontend framework

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. There's a few places where we can trim some of the boilerplate, which I think is important - it's hard to find the "real code" among all the plumbing in places. Other than that, this

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Hmm, i'm missing something about the way store sha1... Comment at: clang-doc/BitcodeWriter.cpp:53 +{// 0. Fixed-size integer (length of the sha1'd USR) + llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::VBR, +

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 137457. juliehockett marked 13 inline comments as done. juliehockett added a comment. Updating bitcode writer for hashed USRs, and re-running clang-format. Also cleaning up a couple of unused fields. https://reviews.llvm.org/D41102 Files:

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. In https://reviews.llvm.org/D41102#1028995, @lebedev.ri wrote: > Some further notes based on the SHA1 nature. I'm sorry, brainfreeze, i meant `40` chars, not `20`. Updated comments... Comment at: clang-doc/BitcodeWriter.cpp:309 +

[PATCH] D41102: Setup clang-doc frontend framework

Athosvk added a comment. In https://reviews.llvm.org/D41102#1028760, @juliehockett wrote: > If you take a look at the follow-on patch to this (D43341 > ), you'll see that that is where the pointer > is added in (since it is irrelevant to the mapper portion, as

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Nice! Some further notes based on the SHA1 nature. Comment at: clang-doc/BitcodeWriter.cpp:74 + AbbrevGen(Abbrev, +{// 0. Fixed-size integer (ref type) + llvm::BitCodeAbbrevOp(llvm::BitCodeAbbrevOp::Fixed,

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 137244. juliehockett added a comment. Adding hashing to reduce the size of USRs and updating tests. https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h clang-doc/CMakeLists.txt

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. In https://reviews.llvm.org/D41102#1028228, @Athosvk wrote: > This seems like quite a decent approach! That being said, I don't see the > pointer yet? I assume you mean that you will be adding this? Additionally, a > slight disadvantage of doing this generic

[PATCH] D41102: Setup clang-doc frontend framework

Athosvk added a comment. My apologies for getting back on this so late! In https://reviews.llvm.org/D41102#1017683, @juliehockett wrote: > So, as an idea (as this diff implements), I updated the string references to > be a struct, which holds the USR of the referenced type (for serialization,

[PATCH] D41102: Setup clang-doc frontend framework

Eugene.Zelenko added inline comments. Comment at: clang-doc/BitcodeWriter.h:160 +class ClangDocBitcodeWriter { + public: + ClangDocBitcodeWriter(llvm::BitstreamWriter , Looks like Clang-format was applied incorrectly, because this is Google, not LLVM style.

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: clang-doc/Representation.h:117 + bool IsDefinition = false; + llvm::Optional DefLoc; + llvm::SmallVector Loc; lebedev.ri wrote: > I meant that `IsDefinition` controls whether `DefLoc` will be

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 136809. juliehockett marked an inline comment as done. juliehockett added a comment. Removing IsDefinition field. https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/Representation.h:117 + bool IsDefinition = false; + llvm::Optional DefLoc; + llvm::SmallVector Loc; I meant that `IsDefinition` controls whether `DefLoc` will be set/used or not. So with

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 136791. juliehockett marked 11 inline comments as done. juliehockett added a comment. Addressing comments https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h clang-doc/CMakeLists.txt

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Could some other people please review this differential, too? I'm sure i have missed things. --- Some more nitpicking. For this differential as standalone, i'we mostly run out of things to nitpick. Some things can probably be done better (the blockid/recordid stuff

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:196 +/// \brief Emits a record name to the BLOCKINFO block. +void ClangDocBitcodeWriter::emitRecordID(RecordId ID) { + assert(RecordIdNameMap[ID] && "Unknown Abbreviation"); lebedev.ri

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 136650. juliehockett marked 16 inline comments as done. juliehockett added a comment. Adding tests, fixing comments, and removing an (as-of-yet) unused element of the CommentInfo struct. https://reviews.llvm.org/D41102 Files: CMakeLists.txt

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Thank you for working on this! Some more nitpicking. //Please// consider adding even more tests (ideally, all this code should have 100% test coverage) Comment at: clang-doc/BitcodeWriter.cpp:139 + {COMMENT_NAME, {"Name", }}, +

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 136520. juliehockett marked 14 inline comments as done. juliehockett added a comment. Fixing comments and adding tests https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Thank you for working on this! Some more review notes. Please look into adding a bit more tests. Comment at: clang-doc/BitcodeWriter.cpp:179 + assert(Inits.size() == RecordIdCount); + for (const auto : Inits) RecordIdNameMap[Init.first] =

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 136303. juliehockett marked 3 inline comments as done. juliehockett added a comment. Running clang-format and fixing newlines https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: clang-doc/BitcodeWriter.h:37 + static constexpr unsigned SubblockIDSize = 4U; + static constexpr unsigned BoolSize = 1U; + static constexpr unsigned IntSize = 16U; lebedev.ri wrote: > Hmm, you build with asserts

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/BitcodeWriter.h:37 + static constexpr unsigned SubblockIDSize = 4U; + static constexpr unsigned BoolSize = 1U; + static constexpr unsigned IntSize = 16U; Hmm, you build with asserts enabled, right? I

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. In https://reviews.llvm.org/D41102#1020808, @lebedev.ri wrote: > Ok, great. > And it will also complain if you try to output a block within block? Um...no. Since you can have subblocks within blocks. Comment at: clang-doc/BitcodeWriter.cpp:191

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 136161. juliehockett marked 15 inline comments as done. juliehockett added a comment. Fixing comments https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h clang-doc/CMakeLists.txt

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Tried fixing `tooling::FrontendActionFactory::create()` in https://reviews.llvm.org/D43779/https://reviews.llvm.org/D43780, but had to revert due to gcc4.8 issues :/ Thank you for working on this, some more review notes. In https://reviews.llvm.org/D41102#1020107,

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 136010. juliehockett marked 10 inline comments as done. juliehockett added a comment. 1. Moved the serialization logic out of the Mapper class and into its own namespace 2. Updated tests 3. Addressing comments https://reviews.llvm.org/D41102 Files:

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. In https://reviews.llvm.org/D41102#1017918, @lebedev.ri wrote: > Is there some (internal to `BitstreamWriter`) logic that would 'assert()' if > trying to output some recordid > which is, according to the `BLOCKINFO_BLOCK`, should not be there? > E.g. outputting

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Thank you for working on this! Some more thoughts. Comment at: clang-doc/BitcodeWriter.cpp:191 + Record.clear(); + for (const char C : BlockIdNameMap[ID]) Record.push_back(C); + Stream.EmitRecord(llvm::bitc::BLOCKINFO_CODE_BLOCKNAME, Record);

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Could you please add a bit more tests? In particular, i'd like to see how blocks-in-blocks work. I.e. class-in-class, class-in-function, ... Is there some (internal to `BitstreamWriter`) logic that would 'assert()' if trying to output some recordid which is,

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 135682. juliehockett added a comment. Fixing CMakeLists formatting https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h clang-doc/CMakeLists.txt clang-doc/ClangDoc.h

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. In https://reviews.llvm.org/D41102#1017499, @Athosvk wrote: > Disadvantage is of course that you add complexity to certain parts of the > deserialization (/serialization) for nested types and inheritance, by either > having to do so in the correct order or having

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 135678. juliehockett marked 29 inline comments as done. juliehockett added a comment. 1. Continued refactoring the bitcode writer 2. Added a USR attribute to infos 3. Created a Reference struct to replace the string references to other infos

[PATCH] D41102: Setup clang-doc frontend framework

Eugene.Zelenko added a comment. Please run Clang-format and Clang-tidy modernize. Comment at: clang-doc/Representation.h:80 + : LineNumber(LineNumber), Filename(std::move(Filename)) {} + int LineNumber; + std::string Filename; Please separate

[PATCH] D41102: Setup clang-doc frontend framework

Athosvk added a comment. The change to USR seems like quite an improvement already! That being said, I do think that it might be preferable to opt out of the use of strings for linking things together. What we did with our clang-doc is that we directly used pointers to refer to other types. So

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Next, i suggest to look into code self-debugging, see comments. Also, i have added a few questions, it would be great to know that my understanding is correct? I'm sorry that it seems like we are going over and over and over over the same code again, this is the

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 135559. juliehockett marked 10 inline comments as done. juliehockett added a comment. Refactoring bitcode writer https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. An idea on how to further generalize/cleanup `emitBlockInfoBlock()`. While *i think* will help, i'm not sure how to further consolidate the `BlockIdNameMap`/`RecordIdNameMap` and the actual `emitBlock(*)`... Comment at:

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:219 + +void ClangDocBitcodeWriter::emitIntRecord(int Value, RecordId ID) { + if (!Value) return; lebedev.ri wrote: > Now, all these three `emit*Record` functions now have the 'same

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 135453. juliehockett marked 13 inline comments as done. juliehockett added a comment. Cleaning up bitcode writer https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp clang-doc/BitcodeWriter.h

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:149 + +/// \brief Emits a record ID in the BLOCKINFO block. +void ClangDocBitcodeWriter::emitRecordID(RecordId ID) { For me, with no prior knowledge of llvm's bitstreams, it was not

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:407 + +void ClangDocBinaryWriter::writeBitstream(const EnumInfo , + BitstreamWriter , juliehockett wrote: > jakehehrlich wrote: > > lebedev.ri

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 135342. juliehockett marked 6 inline comments as done. juliehockett added a comment. Updating location creation and adding mapping from type to BlockId https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:407 + +void ClangDocBinaryWriter::writeBitstream(const EnumInfo , + BitstreamWriter , jakehehrlich wrote: > lebedev.ri wrote: > > juliehockett

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:407 + +void ClangDocBinaryWriter::writeBitstream(const EnumInfo , + BitstreamWriter , lebedev.ri wrote: > juliehockett wrote: > > lebedev.ri

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 135305. juliehockett marked 20 inline comments as done. juliehockett added a comment. Cleaning up bitcode writer and fixing pointers for CommentInfos https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/BitcodeWriter.cpp

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/Representation.h:79 + std::string Filename; +}; + Hmm, have you tried adding a constructor here? ``` struct Location { int LineNumber; std::string Filename; Location() = default; Location(int

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:407 + +void ClangDocBinaryWriter::writeBitstream(const EnumInfo , + BitstreamWriter , juliehockett wrote: > lebedev.ri wrote: > > Hmm, common

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: clang-doc/BitcodeWriter.cpp:407 + +void ClangDocBinaryWriter::writeBitstream(const EnumInfo , + BitstreamWriter , lebedev.ri wrote: > Hmm, common pattern again > ``` > void

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Nice! Comment at: clang-doc/BitcodeWriter.cpp:33 + llvm::IndexedMap BlockIdNameMap; + BlockIdNameMap.resize(BI_LAST - BI_FIRST + 1); + So here's the thing. We know how many enumerators we

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. In https://reviews.llvm.org/D41102#1011299, @lebedev.ri wrote: > I don't know the protocol, but i think it might be a good idea > to add a new entry to `CODE_OWNERS.TXT` for `clang-doc`? > > `clang-doc` going to be quite distinctive, and bigger/complicated > than

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 135168. juliehockett marked 13 inline comments as done. juliehockett added a comment. 1. Updating mapper keys to use USRs instead of names 2. Also updating internal representation to use USRs instead of names 3. Renaming files (getting rid of the

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added a comment. > 2. I've mentioned it before as a comment, but to what extent will you be > parsing information in this frontend? Currently the links between types are > primarily stored as strings. Are you planning to have the backend that > generates the MarkDown parse those

[PATCH] D41102: Setup clang-doc frontend framework

Athosvk added a comment. The changes seem good (both mapper and additional changes)! I've added some comments, but those are primarily details. What I'm however primarily interested in is the following: 1. You seemingly output only little information for declarations that are not definition.

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/ClangDocBinary.cpp:72 + assert(Abbrevs.find(recordID) == Abbrevs.end() && + "Abbreviation already set."); + Abbrevs[recordID] = abbrevID; juliehockett wrote: > lebedev.ri wrote: > > lebedev.ri

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 135009. juliehockett marked 27 inline comments as done. juliehockett added a comment. 1. Decoupled the mapper implementation from the main program, exposing only the function to generate the action factory 2. Implemented the matchers into a

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: clang-doc/ClangDoc.cpp:32 + ECtx.reportResult( + Name, Mapper.emitInfo(D, getComment(D), Name, getLine(D), getFile(D))); +} lebedev.ri wrote: > I wonder if `Name` should be `std::move()`'d ? Or not,

[PATCH] D41102: Setup clang-doc frontend framework

sammccall added a comment. Naming conventions tend to stick around for a while - `clang-doc/ClangDocXYZ.h` seems a bit unwieldy compared to `clang-doc/XYZ.h` - might be worth considering. Comment at: clang-doc/ClangDoc.cpp:37 + Context = Result.Context; + if (const auto *M

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added inline comments. Comment at: clang-doc/ClangDocBinary.h:82 + +static std::map BlockIdNameMap = { + {NAMESPACE_BLOCK_ID, "NamespaceBlock"}, lebedev.ri wrote: > Nice! > Some thoughts: > 1. I agree it makes sense to keep it

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Great work! Some more review... Comment at: clang-doc/ClangDoc.cpp:32 + ECtx.reportResult( + Name, Mapper.emitInfo(D, getComment(D), Name, getLine(D), getFile(D))); +} I wonder if `Name` should be `std::move()`'d ? Or not,

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: clang-doc/ClangDocBinary.cpp:88 + Stream.Emit((unsigned)'C', 8); + Stream.Emit((unsigned)'S', 8); +} lebedev.ri wrote: > General comment: shouldn't the bitcode be versioned? Possibly? My understanding of the

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 134855. juliehockett marked 14 inline comments as done. juliehockett added a comment. 1. Fixing docs 2. Adding static map from bitcode block/record id to block/record name 3. Pulling magic numbers into one struct 4. Cleaning up and clarifying command

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. Nice work! It will be great to have a replacement for doxygen, which is actually modern and usable. Some review notes for some of the code below: Comment at: clang-doc/ClangDocBinary.cpp:17 + +enum BlockIds { + NAMESPACE_BLOCK_ID =

[PATCH] D41102: Setup clang-doc frontend framework

lebedev.ri added a comment. I don't know the protocol, but i think it might be a good idea to add a new entry to `CODE_OWNERS.TXT` for `clang-doc`? `clang-doc` going to be quite distinctive, and bigger/complicated than what already is in `clang-tools-extra`. https://reviews.llvm.org/D41102

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added a comment. Can we add tests for a function at the top level and for a method? https://reviews.llvm.org/D41102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 134544. juliehockett edited the summary of this revision. juliehockett added a comment. Updating tests and moving the bitcode reader out (to the next patch) https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/CMakeLists.txt

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 133726. juliehockett added a comment. Updating documentation https://reviews.llvm.org/D41102 Files: CMakeLists.txt clang-doc/CMakeLists.txt clang-doc/ClangDoc.cpp clang-doc/ClangDoc.h clang-doc/ClangDocBinary.cpp clang-doc/ClangDocBinary.h

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 133714. juliehockett marked 6 inline comments as done. juliehockett added a comment. 1. Implementing the bitstream decoder (and fixing the encoder) 2. Setting up new tests for the mapper output 3. Fixing comments https://reviews.llvm.org/D41102 Files:

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added a comment. After the comments I just made are resolved, I'm fine with the low level details of the parts of this change that are here Comment at: clang-doc/ClangDoc.cpp:47 + +comments::FullComment *ClangDocCallback::getComment(const NamedDecl *D) { +

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett added inline comments. Comment at: tools/clang-doc/ClangDoc.h:29 +struct ClangDocContext { + // Which format in which to emit representation. + OutFormat EmitFormat; sammccall wrote: > juliehockett wrote: > > sammccall wrote: > > > Is this the

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 133108. juliehockett marked 27 inline comments as done. juliehockett edited the summary of this revision. juliehockett edited projects, added clang-tools-extra; removed clang. juliehockett added a comment. 1. Moved the tool to clang-tools-extra 1.

[PATCH] D41102: Setup clang-doc frontend framework

Athosvk added inline comments. Comment at: tools/clang-doc/ClangDocReporter.h:39 // Info for named types (parameters, members). struct NamedType { std::string Type; Storing the type information seems more suitable than storing just the name and type as a

[PATCH] D41102: Setup clang-doc frontend framework

Athosvk added a comment. As we had discussed before, we're interested in the development as well! As an overall comment, I speak from experience that maintaining a large degree of documentation throughout the source code of the tool can provide an excellent test-case. We sure hope this will

[PATCH] D41102: Setup clang-doc frontend framework

sammccall added a subscriber: bkramer. sammccall added a comment. In https://reviews.llvm.org/D41102#998180, @thakis wrote: > This should be in clang-tools-extra next to clang-tidy, clang-include-fixer, > clangd etc, not in the main compiler repo, right? I agree. I see there was earlier

[PATCH] D41102: Setup clang-doc frontend framework

thakis added a comment. This should be in clang-tools-extra next to clang-tidy, clang-include-fixer, clangd etc, not in the main compiler repo, right? https://reviews.llvm.org/D41102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added a comment. Additional note: This diff is a diff from your last commit not the full diff relative to origin/master which is what should be up here. Comment at: tools/clang-doc/ClangDoc.cpp:34 -bool ClangDocVisitor::VisitNamespaceDecl(const NamespaceDecl

[PATCH] D41102: Setup clang-doc frontend framework

sammccall added a comment. In https://reviews.llvm.org/D41102#994311, @juliehockett wrote: > I'm going to take a stab at refactoring the serialization part of this next > -- rather than keeping it all in memory and dumping it at the end, it should > serialize as it goes and do some sort of

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett planned changes to this revision. juliehockett added inline comments. Comment at: tools/clang-doc/ClangDoc.cpp:60 + +comments::FullComment *ClangDocVisitor::getComment(const Decl *D) { + RawComment *Comment = Context->getRawCommentForDeclNoCache(D);

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 132306. juliehockett marked 47 inline comments as done. juliehockett added a comment. 1. Changing the traversal pattern from using `RecursiveASTVisitor` to using matchers instead. This will allow for a more flexible API (e.g. allowing access to

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added inline comments. Comment at: tools/clang-doc/ClangDoc.h:33 + +class ClangDocVisitor : public RecursiveASTVisitor { +public: sammccall wrote: > This API makes essentially everything public. Is that the intent? > > It seems like

[PATCH] D41102: Setup clang-doc frontend framework

sammccall added a comment. Really sorry about the delay in getting to this. At a high level, I'm most concerned about: - the monolithic in-memory intermediate format, which seems to put hard limits on performance and scalability - having high-level documentation and clear APIs - having multiple

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added inline comments. Comment at: tools/clang-doc/ClangDocReporter.cpp:55 +Docs.Namespaces[Name] = std::move(I); +populateBasicInfo(*Docs.Namespaces[Name], Name, D->getNameAsString(), + getParentNamespace(D)); If you

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added inline comments. Comment at: tools/clang-doc/ClangDocReporter.cpp:53-54 + if (Pair == Docs.Namespaces.end()) { +std::unique_ptr I = make_unique(); +Docs.Namespaces[Name] = std::move(I); +populateBasicInfo(*Docs.Namespaces[Name], Name,

[PATCH] D41102: Setup clang-doc frontend framework

jakehehrlich added a comment. If its possible to split VisitEnumDecl, and VisitRecordDecl into separate methods and the same is possible for VisitFunctionDecl and VisitCXXMethodDecl then I think all of your methods will look like the following VisitNamespaceDecl. That being the case you might

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 132075. juliehockett edited the summary of this revision. juliehockett added a reviewer: jakehehrlich. juliehockett added a comment. 1. Updating and expanding tests 2. Updating output options (can now write to files) 3. Cleaning up pointers and whatnot

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 131481. juliehockett added a comment. Herald added a subscriber: hintonda. Cleaning up a few unnecessary copyings https://reviews.llvm.org/D41102 Files: test/CMakeLists.txt test/Tooling/clang-doc-basic.cpp test/lit.cfg.py tools/CMakeLists.txt

[PATCH] D41102: Setup clang-doc frontend framework

juliehockett updated this revision to Diff 128641. juliehockett added a comment. 1. Adding in a basic test setup for the framework 2. Pulling the YAML specs out into their own file 3. Expanding the representation to consider different types of declarations (namespace, tag, and function) and

[PATCH] D41102: Setup clang-doc frontend framework

phosek added a comment. In https://reviews.llvm.org/D41102#955200, @JDevlieghere wrote: > I don't know what basis is used to differentiate between the two, but should > this be part of clang tools or clang-tools-extra? AFAIK there's a general agreement that clang-tools-extra should be

  1   2   >