martong requested changes to this revision. martong added a comment. This revision now requires changes to proceed.
Sorry for the late review, I just noticed something which is not a logical error, but we could make the ASTImporter code much cleaner. ================ Comment at: clang/lib/AST/ASTImporter.cpp:9047-9058 + llvm::Expected<QualType> ImpTypeInfo = Import(QualType( + FromValue.getLValueBase().get<TypeInfoLValue>().getType(), 0)); + if (!ImpTypeInfo) { + Error = ImpTypeInfo.takeError(); + break; + } + llvm::Expected<QualType> ImpType = ---------------- A series of `importChecked` would result in a much cleaner code by omitting the second check. Also, instead of breaking out from the switch we can immediately return with the error. This applies for the other cases where there are more import calls after each other (e.g. AddrLabelDiff, Union, ...). Giving it more thought, you could use `importChecked` instead of `Import` everywhere in this function (for consistency). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63640/new/ https://reviews.llvm.org/D63640 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits