jansvoboda11 accepted this revision. jansvoboda11 added a comment. This revision is now accepted and ready to land.
Thanks for explaining. This LGTM, but you might want to get a second opinion. ================ Comment at: clang/lib/AST/ODRDiagsEmitter.cpp:1660 + case TypeDef: + case Var: + // C++ only, invalid in this context. ---------------- vsapsai wrote: > jansvoboda11 wrote: > > This will fall-through and call `llvm_unreachable`, will it not? I assume > > that's not intended. > Good question. Protocols cannot have fields or vars (properties are covered > by D130326). Typedefs are top-level and not nested inside protocols, so that > case cannot happen too. `Other` is covered by `FirstDiffType == Other || > SecondDiffType == Other` and `EndOfClass` by `FirstDiffType != > SecondDiffType`. So it is correct that in all of these cases we fall-through > to `llvm_unreachable`. That makes sense, thanks for the explanation. Can you a short summary in a comment? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130324/new/ https://reviews.llvm.org/D130324 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
