Skip to content

Commit

Permalink
[NFC] Cleanup the overload of ASTImporter::import()
Browse files Browse the repository at this point in the history
This patch aims to address the comment of a previous review:
https://reviews.llvm.org/D109237#inline-1040678

The original problem was the following:
  `T` is substituted by `clang::Type`

  Expected<T *> import(T *From) {
    auto ToOrErr = Importer.Import(From);
    //             ^^^^^^^^^^^^^^^^^^^^^
    if (!ToOrErr)
      return ToOrErr.takeError();
    return cast_or_null<T>(*ToOrErr);
    //     ^^^^^^^^^^^^^^^^^^^^^^^^^
  }

`Importer.Import()` operates on `const Type *`, thus returns `const Type *`.
Later, at the return statement, we will try to construct an `Expected<Type*>`
from a `const Type *`, which failed with a miserable error message.

In all other cases `importer.Import()` results in a non-const version,
so everything works out just fine, but for `clang::type`s, we should
really return a const version.

So, in case of `T` is a subclass of `clang::Type`, it will return a
`Exprected<const T*>` instead.

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D109269
  • Loading branch information
Balazs Benics committed Sep 30, 2021
1 parent 230a6ed commit e5e0e00
Showing 1 changed file with 24 additions and 26 deletions.
50 changes: 24 additions & 26 deletions clang/lib/AST/ASTImporter.cpp
Expand Up @@ -76,6 +76,7 @@ namespace clang {
using llvm::make_error;
using llvm::Error;
using llvm::Expected;
using ExpectedTypePtr = llvm::Expected<const Type *>;
using ExpectedType = llvm::Expected<QualType>;
using ExpectedStmt = llvm::Expected<Stmt *>;
using ExpectedExpr = llvm::Expected<Expr *>;
Expand Down Expand Up @@ -160,15 +161,17 @@ namespace clang {
// Call the import function of ASTImporter for a baseclass of type `T` and
// cast the return value to `T`.
template <typename T>
Expected<T *> import(T *From) {
auto import(T *From)
-> std::conditional_t<std::is_base_of<Type, T>::value,
Expected<const T *>, Expected<T *>> {
auto ToOrErr = Importer.Import(From);
if (!ToOrErr)
return ToOrErr.takeError();
return cast_or_null<T>(*ToOrErr);
}

template <typename T>
Expected<T *> import(const T *From) {
auto import(const T *From) {
return import(const_cast<T *>(From));
}

Expand Down Expand Up @@ -1164,12 +1167,12 @@ ASTNodeImporter::VisitMemberPointerType(const MemberPointerType *T) {
if (!ToPointeeTypeOrErr)
return ToPointeeTypeOrErr.takeError();

ExpectedType ClassTypeOrErr = import(QualType(T->getClass(), 0));
ExpectedTypePtr ClassTypeOrErr = import(T->getClass());
if (!ClassTypeOrErr)
return ClassTypeOrErr.takeError();

return Importer.getToContext().getMemberPointerType(
*ToPointeeTypeOrErr, (*ClassTypeOrErr).getTypePtr());
return Importer.getToContext().getMemberPointerType(*ToPointeeTypeOrErr,
*ClassTypeOrErr);
}

ExpectedType
Expand Down Expand Up @@ -1475,34 +1478,32 @@ ExpectedType ASTNodeImporter::VisitTemplateTypeParmType(

ExpectedType ASTNodeImporter::VisitSubstTemplateTypeParmType(
const SubstTemplateTypeParmType *T) {
ExpectedType ReplacedOrErr = import(QualType(T->getReplacedParameter(), 0));
Expected<const TemplateTypeParmType *> ReplacedOrErr =
import(T->getReplacedParameter());
if (!ReplacedOrErr)
return ReplacedOrErr.takeError();
const TemplateTypeParmType *Replaced =
cast<TemplateTypeParmType>((*ReplacedOrErr).getTypePtr());

ExpectedType ToReplacementTypeOrErr = import(T->getReplacementType());
if (!ToReplacementTypeOrErr)
return ToReplacementTypeOrErr.takeError();

return Importer.getToContext().getSubstTemplateTypeParmType(
Replaced, (*ToReplacementTypeOrErr).getCanonicalType());
*ReplacedOrErr, ToReplacementTypeOrErr->getCanonicalType());
}

ExpectedType ASTNodeImporter::VisitSubstTemplateTypeParmPackType(
const SubstTemplateTypeParmPackType *T) {
ExpectedType ReplacedOrErr = import(QualType(T->getReplacedParameter(), 0));
Expected<const TemplateTypeParmType *> ReplacedOrErr =
import(T->getReplacedParameter());
if (!ReplacedOrErr)
return ReplacedOrErr.takeError();
const TemplateTypeParmType *Replaced =
cast<TemplateTypeParmType>(ReplacedOrErr->getTypePtr());

Expected<TemplateArgument> ToArgumentPack = import(T->getArgumentPack());
if (!ToArgumentPack)
return ToArgumentPack.takeError();

return Importer.getToContext().getSubstTemplateTypeParmPackType(
Replaced, *ToArgumentPack);
*ReplacedOrErr, *ToArgumentPack);
}

ExpectedType ASTNodeImporter::VisitTemplateSpecializationType(
Expand All @@ -1517,7 +1518,7 @@ ExpectedType ASTNodeImporter::VisitTemplateSpecializationType(
return std::move(Err);

QualType ToCanonType;
if (!QualType(T, 0).isCanonical()) {
if (!T->isCanonicalUnqualified()) {
QualType FromCanonType
= Importer.getFromContext().getCanonicalType(QualType(T, 0));
if (ExpectedType TyOrErr = import(FromCanonType))
Expand Down Expand Up @@ -8378,7 +8379,7 @@ ASTImporter::Import(ExprWithCleanups::CleanupObject From) {
return make_error<ImportError>(ImportError::UnsupportedConstruct);
}

Expected<const Type *> ASTImporter::Import(const Type *FromT) {
ExpectedTypePtr ASTImporter::Import(const Type *FromT) {
if (!FromT)
return FromT;

Expand All @@ -8388,7 +8389,7 @@ Expected<const Type *> ASTImporter::Import(const Type *FromT) {
if (Pos != ImportedTypes.end())
return Pos->second;

// Import the type
// Import the type.
ASTNodeImporter Importer(*this);
ExpectedType ToTOrErr = Importer.Visit(FromT);
if (!ToTOrErr)
Expand All @@ -8404,7 +8405,7 @@ Expected<QualType> ASTImporter::Import(QualType FromT) {
if (FromT.isNull())
return QualType{};

Expected<const Type *> ToTyOrErr = Import(FromT.getTypePtr());
ExpectedTypePtr ToTyOrErr = Import(FromT.getTypePtr());
if (!ToTyOrErr)
return ToTyOrErr.takeError();

Expand Down Expand Up @@ -8904,12 +8905,11 @@ ASTImporter::Import(NestedNameSpecifier *FromNNS) {

case NestedNameSpecifier::TypeSpec:
case NestedNameSpecifier::TypeSpecWithTemplate:
if (Expected<QualType> TyOrErr =
Import(QualType(FromNNS->getAsType(), 0u))) {
if (ExpectedTypePtr TyOrErr = Import(FromNNS->getAsType())) {
bool TSTemplate =
FromNNS->getKind() == NestedNameSpecifier::TypeSpecWithTemplate;
return NestedNameSpecifier::Create(ToContext, Prefix, TSTemplate,
TyOrErr->getTypePtr());
*TyOrErr);
} else {
return TyOrErr.takeError();
}
Expand Down Expand Up @@ -9544,16 +9544,14 @@ ASTNodeImporter::ImportAPValue(const APValue &FromValue) {
}
} else {
FromElemTy = FromValue.getLValueBase().getTypeInfoType();
QualType ImpTypeInfo = importChecked(
Err,
QualType(FromValue.getLValueBase().get<TypeInfoLValue>().getType(),
0));
const Type *ImpTypeInfo = importChecked(
Err, FromValue.getLValueBase().get<TypeInfoLValue>().getType());
QualType ImpType =
importChecked(Err, FromValue.getLValueBase().getTypeInfoType());
if (Err)
return std::move(Err);
Base = APValue::LValueBase::getTypeInfo(
TypeInfoLValue(ImpTypeInfo.getTypePtr()), ImpType);
Base = APValue::LValueBase::getTypeInfo(TypeInfoLValue(ImpTypeInfo),
ImpType);
}
}
CharUnits Offset = FromValue.getLValueOffset();
Expand Down

0 comments on commit e5e0e00

Please sign in to comment.