paulkirth added a comment. Can you add a test that checks the `IsUsing == false` case? Otherwise LGTM modulo one small nit.
================ Comment at: clang-tools-extra/clang-doc/BitcodeReader.cpp:396-397 +template <> llvm::Expected<CommentInfo *> getCommentInfo(TypedefInfo *I) { + I->Description.emplace_back(); + return &I->Description.back(); +} ---------------- nit: I know this is in the style of the rest of the file, but I believe that code was written before LLVM allowed C++17. I //think// we can just return the reference from `emplace_back()` directly, right? ================ Comment at: clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp:184 I.Underlying = TypeInfo("unsigned"); I.IsUsing = true; ---------------- This seems to be a test for the `IsUsing == true` case, are there tests where it is `false`? I realize that's the default now, but I'm not sure it's covered. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136638/new/ https://reviews.llvm.org/D136638 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits