llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Rahul Joshi (jurahul) <details> <summary>Changes</summary> Adopt non-templated and array-ref returning forms of `getTrailingObjects` in Expr.cpp/.h. Use ArrayRef forms to eliminate manual asserting for OOB index. Use llvm::copy() instead of std::copy() in some instances. --- Full diff: https://github.com/llvm/llvm-project/pull/140102.diff 3 Files Affected: - (modified) clang/include/clang/AST/Expr.h (+36-63) - (modified) clang/lib/AST/Expr.cpp (+29-35) - (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+2-2) ``````````diff diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 1e6749dda71fe..e9c3c16c87598 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -2023,7 +2023,7 @@ class PredefinedExpr final void setFunctionName(StringLiteral *SL) { assert(hasFunctionName() && "This PredefinedExpr has no storage for a function name!"); - *getTrailingObjects<Stmt *>() = SL; + *getTrailingObjects() = SL; } public: @@ -2050,13 +2050,13 @@ class PredefinedExpr final StringLiteral *getFunctionName() { return hasFunctionName() - ? static_cast<StringLiteral *>(*getTrailingObjects<Stmt *>()) + ? static_cast<StringLiteral *>(*getTrailingObjects()) : nullptr; } const StringLiteral *getFunctionName() const { return hasFunctionName() - ? static_cast<StringLiteral *>(*getTrailingObjects<Stmt *>()) + ? static_cast<StringLiteral *>(*getTrailingObjects()) : nullptr; } @@ -2078,13 +2078,11 @@ class PredefinedExpr final // Iterators child_range children() { - return child_range(getTrailingObjects<Stmt *>(), - getTrailingObjects<Stmt *>() + hasFunctionName()); + return child_range(getTrailingObjects(hasFunctionName())); } const_child_range children() const { - return const_child_range(getTrailingObjects<Stmt *>(), - getTrailingObjects<Stmt *>() + hasFunctionName()); + return const_child_range(getTrailingObjects(hasFunctionName())); } }; @@ -2248,18 +2246,14 @@ class UnaryOperator final private llvm::TrailingObjects<UnaryOperator, FPOptionsOverride> { Stmt *Val; - size_t numTrailingObjects(OverloadToken<FPOptionsOverride>) const { - return UnaryOperatorBits.HasFPFeatures ? 1 : 0; - } - FPOptionsOverride &getTrailingFPFeatures() { assert(UnaryOperatorBits.HasFPFeatures); - return *getTrailingObjects<FPOptionsOverride>(); + return *getTrailingObjects(); } const FPOptionsOverride &getTrailingFPFeatures() const { assert(UnaryOperatorBits.HasFPFeatures); - return *getTrailingObjects<FPOptionsOverride>(); + return *getTrailingObjects(); } public: @@ -2580,13 +2574,11 @@ class OffsetOfExpr final } const OffsetOfNode &getComponent(unsigned Idx) const { - assert(Idx < NumComps && "Subscript out of range"); - return getTrailingObjects<OffsetOfNode>()[Idx]; + return getTrailingObjects<OffsetOfNode>(NumComps)[Idx]; } void setComponent(unsigned Idx, OffsetOfNode ON) { - assert(Idx < NumComps && "Subscript out of range"); - getTrailingObjects<OffsetOfNode>()[Idx] = ON; + getTrailingObjects<OffsetOfNode>(NumComps)[Idx] = ON; } unsigned getNumComponents() const { @@ -2594,18 +2586,15 @@ class OffsetOfExpr final } Expr* getIndexExpr(unsigned Idx) { - assert(Idx < NumExprs && "Subscript out of range"); - return getTrailingObjects<Expr *>()[Idx]; + return getTrailingObjects<Expr *>(NumExprs)[Idx]; } const Expr *getIndexExpr(unsigned Idx) const { - assert(Idx < NumExprs && "Subscript out of range"); - return getTrailingObjects<Expr *>()[Idx]; + return getTrailingObjects<Expr *>(NumExprs)[Idx]; } void setIndexExpr(unsigned Idx, Expr* E) { - assert(Idx < NumComps && "Subscript out of range"); - getTrailingObjects<Expr *>()[Idx] = E; + getTrailingObjects<Expr *>(NumComps)[Idx] = E; } unsigned getNumExpressions() const { @@ -4619,12 +4608,12 @@ class ConvertVectorExpr final FPOptionsOverride &getTrailingFPFeatures() { assert(ConvertVectorExprBits.HasFPFeatures); - return *getTrailingObjects<FPOptionsOverride>(); + return *getTrailingObjects(); } const FPOptionsOverride &getTrailingFPFeatures() const { assert(ConvertVectorExprBits.HasFPFeatures); - return *getTrailingObjects<FPOptionsOverride>(); + return *getTrailingObjects(); } public: @@ -5705,13 +5694,11 @@ class DesignatedInitExpr final unsigned getNumSubExprs() const { return NumSubExprs; } Expr *getSubExpr(unsigned Idx) const { - assert(Idx < NumSubExprs && "Subscript out of range"); - return cast<Expr>(getTrailingObjects<Stmt *>()[Idx]); + return cast<Expr>(getTrailingObjects(NumSubExprs)[Idx]); } void setSubExpr(unsigned Idx, Expr *E) { - assert(Idx < NumSubExprs && "Subscript out of range"); - getTrailingObjects<Stmt *>()[Idx] = E; + getTrailingObjects(NumSubExprs)[Idx] = E; } /// Replaces the designator at index @p Idx with the series @@ -5730,11 +5717,11 @@ class DesignatedInitExpr final // Iterators child_range children() { - Stmt **begin = getTrailingObjects<Stmt *>(); + Stmt **begin = getTrailingObjects(); return child_range(begin, begin + NumSubExprs); } const_child_range children() const { - Stmt * const *begin = getTrailingObjects<Stmt *>(); + Stmt *const *begin = getTrailingObjects(); return const_child_range(begin, begin + NumSubExprs); } @@ -5994,9 +5981,7 @@ class ParenListExpr final return const_cast<ParenListExpr *>(this)->getExpr(Init); } - Expr **getExprs() { - return reinterpret_cast<Expr **>(getTrailingObjects<Stmt *>()); - } + Expr **getExprs() { return reinterpret_cast<Expr **>(getTrailingObjects()); } ArrayRef<Expr *> exprs() { return llvm::ArrayRef(getExprs(), getNumExprs()); } @@ -6011,12 +5996,10 @@ class ParenListExpr final // Iterators child_range children() { - return child_range(getTrailingObjects<Stmt *>(), - getTrailingObjects<Stmt *>() + getNumExprs()); + return child_range(getTrailingObjects(getNumExprs())); } const_child_range children() const { - return const_child_range(getTrailingObjects<Stmt *>(), - getTrailingObjects<Stmt *>() + getNumExprs()); + return const_child_range(getTrailingObjects(getNumExprs())); } }; @@ -6421,14 +6404,12 @@ class GenericSelectionExpr final } child_range children() { - return child_range(getTrailingObjects<Stmt *>(), - getTrailingObjects<Stmt *>() + - numTrailingObjects(OverloadToken<Stmt *>())); + return child_range(getTrailingObjects<Stmt *>( + numTrailingObjects(OverloadToken<Stmt *>()))); } const_child_range children() const { - return const_child_range(getTrailingObjects<Stmt *>(), - getTrailingObjects<Stmt *>() + - numTrailingObjects(OverloadToken<Stmt *>())); + return const_child_range(getTrailingObjects<Stmt *>( + numTrailingObjects(OverloadToken<Stmt *>()))); } }; @@ -6647,11 +6628,6 @@ class PseudoObjectExpr final // in to Create, which is an index within the semantic forms. // Note also that ASTStmtWriter assumes this encoding. - Expr **getSubExprsBuffer() { return getTrailingObjects<Expr *>(); } - const Expr * const *getSubExprsBuffer() const { - return getTrailingObjects<Expr *>(); - } - PseudoObjectExpr(QualType type, ExprValueKind VK, Expr *syntactic, ArrayRef<Expr*> semantic, unsigned resultIndex); @@ -6677,8 +6653,8 @@ class PseudoObjectExpr final /// Return the syntactic form of this expression, i.e. the /// expression it actually looks like. Likely to be expressed in /// terms of OpaqueValueExprs bound in the semantic form. - Expr *getSyntacticForm() { return getSubExprsBuffer()[0]; } - const Expr *getSyntacticForm() const { return getSubExprsBuffer()[0]; } + Expr *getSyntacticForm() { return getTrailingObjects()[0]; } + const Expr *getSyntacticForm() const { return getTrailingObjects()[0]; } /// Return the index of the result-bearing expression into the semantics /// expressions, or PseudoObjectExpr::NoResult if there is none. @@ -6691,7 +6667,7 @@ class PseudoObjectExpr final Expr *getResultExpr() { if (PseudoObjectExprBits.ResultIndex == 0) return nullptr; - return getSubExprsBuffer()[PseudoObjectExprBits.ResultIndex]; + return getTrailingObjects()[PseudoObjectExprBits.ResultIndex]; } const Expr *getResultExpr() const { return const_cast<PseudoObjectExpr*>(this)->getResultExpr(); @@ -6701,29 +6677,26 @@ class PseudoObjectExpr final typedef Expr * const *semantics_iterator; typedef const Expr * const *const_semantics_iterator; - semantics_iterator semantics_begin() { - return getSubExprsBuffer() + 1; - } + semantics_iterator semantics_begin() { return getTrailingObjects() + 1; } const_semantics_iterator semantics_begin() const { - return getSubExprsBuffer() + 1; + return getTrailingObjects() + 1; } semantics_iterator semantics_end() { - return getSubExprsBuffer() + getNumSubExprs(); + return getTrailingObjects() + getNumSubExprs(); } const_semantics_iterator semantics_end() const { - return getSubExprsBuffer() + getNumSubExprs(); + return getTrailingObjects() + getNumSubExprs(); } ArrayRef<Expr*> semantics() { - return ArrayRef(semantics_begin(), semantics_end()); + return getTrailingObjects(getNumSubExprs()).drop_front(); } ArrayRef<const Expr*> semantics() const { - return ArrayRef(semantics_begin(), semantics_end()); + return getTrailingObjects(getNumSubExprs()).drop_front(); } Expr *getSemanticExpr(unsigned index) { - assert(index + 1 < getNumSubExprs()); - return getSubExprsBuffer()[index + 1]; + return getTrailingObjects(getNumSubExprs())[index + 1]; } const Expr *getSemanticExpr(unsigned index) const { return const_cast<PseudoObjectExpr*>(this)->getSemanticExpr(index); @@ -6748,7 +6721,7 @@ class PseudoObjectExpr final } const_child_range children() const { Stmt *const *cs = const_cast<Stmt *const *>( - reinterpret_cast<const Stmt *const *>(getSubExprsBuffer())); + reinterpret_cast<const Stmt *const *>(getTrailingObjects())); return const_child_range(cs, cs + getNumSubExprs()); } diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 8557c3b82ca39..1d0207e8dc172 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -4449,11 +4449,10 @@ GenericSelectionExpr::GenericSelectionExpr( GenericSelectionExprBits.GenericLoc = GenericLoc; getTrailingObjects<Stmt *>()[getIndexOfControllingExpression()] = ControllingExpr; - std::copy(AssocExprs.begin(), AssocExprs.end(), - getTrailingObjects<Stmt *>() + getIndexOfStartOfAssociatedExprs()); - std::copy(AssocTypes.begin(), AssocTypes.end(), - getTrailingObjects<TypeSourceInfo *>() + - getIndexOfStartOfAssociatedTypes()); + llvm::copy(AssocExprs, + getTrailingObjects<Stmt *>() + getIndexOfStartOfAssociatedExprs()); + llvm::copy(AssocTypes, getTrailingObjects<TypeSourceInfo *>() + + getIndexOfStartOfAssociatedTypes()); setDependence(computeDependence(this, ContainsUnexpandedParameterPack)); } @@ -4477,11 +4476,10 @@ GenericSelectionExpr::GenericSelectionExpr( GenericSelectionExprBits.GenericLoc = GenericLoc; getTrailingObjects<TypeSourceInfo *>()[getIndexOfControllingType()] = ControllingType; - std::copy(AssocExprs.begin(), AssocExprs.end(), - getTrailingObjects<Stmt *>() + getIndexOfStartOfAssociatedExprs()); - std::copy(AssocTypes.begin(), AssocTypes.end(), - getTrailingObjects<TypeSourceInfo *>() + - getIndexOfStartOfAssociatedTypes()); + llvm::copy(AssocExprs, + getTrailingObjects<Stmt *>() + getIndexOfStartOfAssociatedExprs()); + llvm::copy(AssocTypes, getTrailingObjects<TypeSourceInfo *>() + + getIndexOfStartOfAssociatedTypes()); setDependence(computeDependence(this, ContainsUnexpandedParameterPack)); } @@ -4502,11 +4500,10 @@ GenericSelectionExpr::GenericSelectionExpr( GenericSelectionExprBits.GenericLoc = GenericLoc; getTrailingObjects<Stmt *>()[getIndexOfControllingExpression()] = ControllingExpr; - std::copy(AssocExprs.begin(), AssocExprs.end(), - getTrailingObjects<Stmt *>() + getIndexOfStartOfAssociatedExprs()); - std::copy(AssocTypes.begin(), AssocTypes.end(), - getTrailingObjects<TypeSourceInfo *>() + - getIndexOfStartOfAssociatedTypes()); + llvm::copy(AssocExprs, + getTrailingObjects<Stmt *>() + getIndexOfStartOfAssociatedExprs()); + llvm::copy(AssocTypes, getTrailingObjects<TypeSourceInfo *>() + + getIndexOfStartOfAssociatedTypes()); setDependence(computeDependence(this, ContainsUnexpandedParameterPack)); } @@ -4527,11 +4524,10 @@ GenericSelectionExpr::GenericSelectionExpr( GenericSelectionExprBits.GenericLoc = GenericLoc; getTrailingObjects<TypeSourceInfo *>()[getIndexOfControllingType()] = ControllingType; - std::copy(AssocExprs.begin(), AssocExprs.end(), - getTrailingObjects<Stmt *>() + getIndexOfStartOfAssociatedExprs()); - std::copy(AssocTypes.begin(), AssocTypes.end(), - getTrailingObjects<TypeSourceInfo *>() + - getIndexOfStartOfAssociatedTypes()); + llvm::copy(AssocExprs, + getTrailingObjects<Stmt *>() + getIndexOfStartOfAssociatedExprs()); + llvm::copy(AssocTypes, getTrailingObjects<TypeSourceInfo *>() + + getIndexOfStartOfAssociatedTypes()); setDependence(computeDependence(this, ContainsUnexpandedParameterPack)); } @@ -4780,9 +4776,7 @@ ParenListExpr::ParenListExpr(SourceLocation LParenLoc, ArrayRef<Expr *> Exprs, : Expr(ParenListExprClass, QualType(), VK_PRValue, OK_Ordinary), LParenLoc(LParenLoc), RParenLoc(RParenLoc) { ParenListExprBits.NumExprs = Exprs.size(); - - for (unsigned I = 0, N = Exprs.size(); I != N; ++I) - getTrailingObjects<Stmt *>()[I] = Exprs[I]; + llvm::copy(Exprs, getTrailingObjects()); setDependence(computeDependence(this)); } @@ -5043,16 +5037,18 @@ PseudoObjectExpr::PseudoObjectExpr(QualType type, ExprValueKind VK, : Expr(PseudoObjectExprClass, type, VK, OK_Ordinary) { PseudoObjectExprBits.NumSubExprs = semantics.size() + 1; PseudoObjectExprBits.ResultIndex = resultIndex + 1; - - for (unsigned i = 0, e = semantics.size() + 1; i != e; ++i) { - Expr *E = (i == 0 ? syntax : semantics[i-1]); - getSubExprsBuffer()[i] = E; - - if (isa<OpaqueValueExpr>(E)) - assert(cast<OpaqueValueExpr>(E)->getSourceExpr() != nullptr && + MutableArrayRef<Expr *> Trail = getTrailingObjects(semantics.size() + 1); + Trail[0] = syntax; + llvm::copy(semantics, Trail.drop_front().begin()); + +#ifndef NDEBUG + llvm::for_each(semantics, [](const Expr *E) { + if (auto *OE = dyn_cast<OpaqueValueExpr>(E)) + assert(OE->getSourceExpr() != nullptr && "opaque-value semantic expressions for pseudo-object " "operations must have sources"); - } + }); +#endif setDependence(computeDependence(this)); } @@ -5240,7 +5236,7 @@ RecoveryExpr::RecoveryExpr(ASTContext &Ctx, QualType T, SourceLocation BeginLoc, assert(!T.isNull()); assert(!llvm::is_contained(SubExprs, nullptr)); - llvm::copy(SubExprs, getTrailingObjects<Expr *>()); + llvm::copy(SubExprs, getTrailingObjects()); setDependence(computeDependence(this)); } @@ -5307,9 +5303,7 @@ OMPArrayShapingExpr *OMPArrayShapingExpr::CreateEmpty(const ASTContext &Context, } void OMPIteratorExpr::setIteratorDeclaration(unsigned I, Decl *D) { - assert(I < NumIterators && - "Idx is greater or equal the number of iterators definitions."); - getTrailingObjects<Decl *>()[I] = D; + getTrailingObjects<Decl *>(NumIterators)[I] = D; } void OMPIteratorExpr::setAssignmentLoc(unsigned I, SourceLocation Loc) { diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index f41cfcc53a35d..8305db8fad087 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -1433,12 +1433,12 @@ void ASTStmtReader::VisitPseudoObjectExpr(PseudoObjectExpr *E) { E->PseudoObjectExprBits.ResultIndex = Record.readInt(); // Read the syntactic expression. - E->getSubExprsBuffer()[0] = Record.readSubExpr(); + E->getTrailingObjects()[0] = Record.readSubExpr(); // Read all the semantic expressions. for (unsigned i = 0; i != numSemanticExprs; ++i) { Expr *subExpr = Record.readSubExpr(); - E->getSubExprsBuffer()[i+1] = subExpr; + E->getTrailingObjects()[i + 1] = subExpr; } } `````````` </details> https://github.com/llvm/llvm-project/pull/140102 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
