llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Roberto Turrado Camblor (rturrado)
<details>
<summary>Changes</summary>
Issue #<!-- -->182790 points that `clang/lib/Sema/SemaExprCXX.cpp` code
contains a bunch of `ArraySize && *ArraySize` constructions, which
should be cleaned up.
This PR introduces an `ArraySizeOptExpr` helper class (in
`SemaExprCXXUtils.{h,cpp}`) to encapsulate the `std::optional<Expr*>
ArraySize` and deal with those `std::optional::has_value()` and `Expr* ==
nullptr` checks internally. It also works as a kind of proxy providing an API
to access `Expr*` methods, such as `getType` or `getIntegerConstantExpr`.
---
Full diff: https://github.com/llvm/llvm-project/pull/186617.diff
5 Files Affected:
- (modified) clang/include/clang/Sema/Sema.h (+2-1)
- (added) clang/include/clang/Sema/SemaExprCXXUtils.h (+58)
- (modified) clang/lib/Sema/CMakeLists.txt (+1)
- (modified) clang/lib/Sema/SemaExprCXX.cpp (+30-29)
- (added) clang/lib/Sema/SemaExprCXXUtils.cpp (+77)
``````````diff
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 832e46286194a..6b536643b62f5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -67,6 +67,7 @@
#include "clang/Sema/Scope.h"
#include "clang/Sema/SemaBase.h"
#include "clang/Sema/SemaConcept.h"
+#include "clang/Sema/SemaExprCXXUtils.h"
#include "clang/Sema/SemaRISCV.h"
#include "clang/Sema/TypoCorrection.h"
#include "clang/Sema/Weak.h"
@@ -8598,7 +8599,7 @@ class Sema final : public SemaBase {
BuildCXXNew(SourceRange Range, bool UseGlobal, SourceLocation
PlacementLParen,
MultiExprArg PlacementArgs, SourceLocation PlacementRParen,
SourceRange TypeIdParens, QualType AllocType,
- TypeSourceInfo *AllocTypeInfo, std::optional<Expr *> ArraySize,
+ TypeSourceInfo *AllocTypeInfo, ArraySizeOptExpr ArraySize,
SourceRange DirectInitRange, Expr *Initializer);
/// Determine whether \p FD is an aligned allocation or deallocation
diff --git a/clang/include/clang/Sema/SemaExprCXXUtils.h
b/clang/include/clang/Sema/SemaExprCXXUtils.h
new file mode 100644
index 0000000000000..a0e79a4381a10
--- /dev/null
+++ b/clang/include/clang/Sema/SemaExprCXXUtils.h
@@ -0,0 +1,58 @@
+//===--- SemaExprCXXtUtils.h - Utils for SemaExprCXX ------------*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines helper classes for Sema Expr CXX.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_SEMA_SEMAEXPRCXXUTILS_H
+#define LLVM_CLANG_SEMA_SEMAEXPRCXXUTILS_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/TypeBase.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/APSInt.h"
+#include <optional>
+
+namespace clang {
+
+/// Helper class to encapsulate an optional pointer to an array size Expr
+///
+/// Allows rewriting constructions like:
+/// @code if (ArraySize && *ArraySize && (*ArraySize)->getBeginLoc())
+/// into cleaner code:
+/// @code if (ArraySize.getExprBeginLoc())
+class ArraySizeOptExpr {
+ std::optional<Expr *> ArraySize;
+
+public:
+ ArraySizeOptExpr();
+ ArraySizeOptExpr(std::nullopt_t);
+ ArraySizeOptExpr(std::optional<Expr *> ArraySize);
+ ArraySizeOptExpr &operator=(Expr *E);
+
+ explicit operator bool() const;
+ operator std::optional<Expr *>() const;
+ bool hasValue() const;
+ Expr *value() const;
+ Expr *valueOr(Expr *default_value) const;
+
+ bool isExprTypeDependent() const;
+ std::optional<llvm::APSInt>
+ getIntegerConstantExpr(const ASTContext &Ctx) const;
+ QualType getExprType() const;
+ SourceLocation getExprBeginLoc() const;
+ SourceLocation getExprLocOr(SourceLocation defaultValue) const;
+ SourceRange getExprSourceRange() const;
+ SourceRange getExprSourceRangeOr(SourceRange defaultValue) const;
+};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_SEMA_SEMAEXPRCXXUTILS_H
diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index 0ebf56ecffe69..0a61589bcf933 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -55,6 +55,7 @@ add_clang_library(clangSema
SemaExceptionSpec.cpp
SemaExpr.cpp
SemaExprCXX.cpp
+ SemaExprCXXUtils.cpp
SemaExprMember.cpp
SemaExprObjC.cpp
SemaFixItUtils.cpp
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 5a5bbf4d900dc..547ad1dbce663 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -39,6 +39,7 @@
#include "clang/Sema/Scope.h"
#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/SemaCUDA.h"
+#include "clang/Sema/SemaExprCXXUtils.h"
#include "clang/Sema/SemaHLSL.h"
#include "clang/Sema/SemaLambda.h"
#include "clang/Sema/SemaObjC.h"
@@ -2142,7 +2143,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
SourceLocation PlacementRParen,
SourceRange TypeIdParens, QualType AllocType,
TypeSourceInfo *AllocTypeInfo,
- std::optional<Expr *> ArraySize,
+ ArraySizeOptExpr ArraySize,
SourceRange DirectInitRange, Expr *Initializer) {
SourceRange TypeRange = AllocTypeInfo->getTypeLoc().getSourceRange();
SourceLocation StartLoc = Range.getBegin();
@@ -2199,12 +2200,12 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
auto *Deduced = AllocType->getContainedDeducedType();
if (Deduced && !Deduced->isDeduced() &&
isa<DeducedTemplateSpecializationType>(Deduced)) {
- if (ArraySize)
- return ExprError(
- Diag(*ArraySize ? (*ArraySize)->getExprLoc() : TypeRange.getBegin(),
- diag::err_deduced_class_template_compound_type)
- << /*array*/ 2
- << (*ArraySize ? (*ArraySize)->getSourceRange() : TypeRange));
+ if (ArraySize) {
+ return ExprError(Diag(ArraySize.getExprLocOr(TypeRange.getBegin()),
+ diag::err_deduced_class_template_compound_type)
+ << /*array*/ 2
+ << ArraySize.getExprSourceRangeOr(TypeRange));
+ }
InitializedEntity Entity =
InitializedEntity::InitializeNew(StartLoc, AllocType,
@@ -2283,9 +2284,8 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
QualType ResultType = Context.getPointerType(AllocType);
- if (ArraySize && *ArraySize &&
- (*ArraySize)->getType()->isNonOverloadPlaceholderType()) {
- ExprResult result = CheckPlaceholderExpr(*ArraySize);
+ if (ArraySize.getExprType()->isNonOverloadPlaceholderType()) {
+ ExprResult result = CheckPlaceholderExpr(ArraySize.value());
if (result.isInvalid()) return ExprError();
ArraySize = result.get();
}
@@ -2297,18 +2297,19 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
// C++1y [expr.new]p6: The expression [...] is implicitly converted to
// std::size_t.
std::optional<uint64_t> KnownArraySize;
- if (ArraySize && *ArraySize && !(*ArraySize)->isTypeDependent()) {
+ if (!ArraySize.isExprTypeDependent()) {
ExprResult ConvertedSize;
if (getLangOpts().CPlusPlus14) {
assert(Context.getTargetInfo().getIntWidth() && "Builtin type of size
0?");
- ConvertedSize = PerformImplicitConversion(
- *ArraySize, Context.getSizeType(), AssignmentAction::Converting);
+ ConvertedSize =
+ PerformImplicitConversion(ArraySize.value(), Context.getSizeType(),
+ AssignmentAction::Converting);
- if (!ConvertedSize.isInvalid() &&
(*ArraySize)->getType()->isRecordType())
+ if (!ConvertedSize.isInvalid() &&
ArraySize.getExprType()->isRecordType())
// Diagnose the compatibility of this conversion.
Diag(StartLoc, diag::warn_cxx98_compat_array_size_conversion)
- << (*ArraySize)->getType() << 0 << "'size_t'";
+ << ArraySize.getExprType() << 0 << "'size_t'";
} else {
class SizeConvertDiagnoser : public ICEConvertDiagnoser {
protected:
@@ -2362,16 +2363,16 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
: diag::ext_array_size_conversion)
<< T << ConvTy->isEnumeralType() << ConvTy;
}
- } SizeDiagnoser(*ArraySize);
+ } SizeDiagnoser(ArraySize.value());
- ConvertedSize = PerformContextualImplicitConversion(StartLoc, *ArraySize,
- SizeDiagnoser);
+ ConvertedSize = PerformContextualImplicitConversion(
+ StartLoc, ArraySize.value(), SizeDiagnoser);
}
if (ConvertedSize.isInvalid())
return ExprError();
ArraySize = ConvertedSize.get();
- QualType SizeType = (*ArraySize)->getType();
+ QualType SizeType = ArraySize.getExprType();
if (!SizeType->isIntegralOrUnscopedEnumerationType())
return ExprError();
@@ -2390,11 +2391,11 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
// converting to size_t. This will never find a negative array size in
// C++14 onwards, because Value is always unsigned here!
if (std::optional<llvm::APSInt> Value =
- (*ArraySize)->getIntegerConstantExpr(Context)) {
+ ArraySize.getIntegerConstantExpr(Context)) {
if (Value->isSigned() && Value->isNegative()) {
- return ExprError(Diag((*ArraySize)->getBeginLoc(),
+ return ExprError(Diag(ArraySize.getExprBeginLoc(),
diag::err_typecheck_negative_array_size)
- << (*ArraySize)->getSourceRange());
+ << ArraySize.getExprSourceRange());
}
if (!AllocType->isDependentType()) {
@@ -2402,18 +2403,18 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
ConstantArrayType::getNumAddressingBits(Context, AllocType,
*Value);
if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context))
return ExprError(
- Diag((*ArraySize)->getBeginLoc(), diag::err_array_too_large)
+ Diag(ArraySize.getExprBeginLoc(), diag::err_array_too_large)
<< toString(*Value, 10, Value->isSigned(),
/*formatAsCLiteral=*/false, /*UpperCase=*/false,
/*InsertSeparators=*/true)
- << (*ArraySize)->getSourceRange());
+ << ArraySize.getExprSourceRange());
}
KnownArraySize = Value->getZExtValue();
} else if (TypeIdParens.isValid()) {
// Can't have dynamic array size when the type-id is in parentheses.
- Diag((*ArraySize)->getBeginLoc(), diag::ext_new_paren_array_nonconst)
- << (*ArraySize)->getSourceRange()
+ Diag(ArraySize.getExprBeginLoc(), diag::ext_new_paren_array_nonconst)
+ << ArraySize.getExprSourceRange()
<< FixItHint::CreateRemoval(TypeIdParens.getBegin())
<< FixItHint::CreateRemoval(TypeIdParens.getEnd());
@@ -2445,7 +2446,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
if (!AllocType->isDependentType() &&
!Expr::hasAnyTypeDependentArguments(PlacementArgs) &&
FindAllocationFunctions(StartLoc, AllocationParameterRange, Scope, Scope,
- AllocType, ArraySize.has_value(), IAP,
+ AllocType, ArraySize.hasValue(), IAP,
PlacementArgs, OperatorNew, OperatorDelete))
return ExprError();
@@ -2583,7 +2584,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
AllocType,
llvm::APInt(Context.getTypeSize(Context.getSizeType()),
*KnownArraySize),
- *ArraySize, ArraySizeModifier::Normal, 0);
+ ArraySize.value(), ArraySizeModifier::Normal, 0);
else if (ArraySize)
InitType = Context.getIncompleteArrayType(AllocType,
ArraySizeModifier::Normal, 0);
@@ -2610,7 +2611,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool
UseGlobal,
// FIXME: If we have a KnownArraySize, check that the array bound of the
// initializer is no greater than that constant value.
- if (ArraySize && !*ArraySize) {
+ if (ArraySize && !ArraySize.value()) {
auto *CAT = Context.getAsConstantArrayType(Initializer->getType());
if (CAT) {
// FIXME: Track that the array size was inferred rather than explicitly
diff --git a/clang/lib/Sema/SemaExprCXXUtils.cpp
b/clang/lib/Sema/SemaExprCXXUtils.cpp
new file mode 100644
index 0000000000000..725234bae3318
--- /dev/null
+++ b/clang/lib/Sema/SemaExprCXXUtils.cpp
@@ -0,0 +1,77 @@
+//===--- SemaExprCXXUtils.cpp - Utils for SemaExprCXX
---------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// Implements helper classes for Sema Expr CXX.
+///
+//===----------------------------------------------------------------------===//
+
+#include "clang/Sema/SemaExprCXXUtils.h"
+
+namespace clang {
+
+ArraySizeOptExpr::ArraySizeOptExpr() : ArraySize{} {}
+
+ArraySizeOptExpr::ArraySizeOptExpr(std::nullopt_t) : ArraySize{} {}
+
+ArraySizeOptExpr::ArraySizeOptExpr(std::optional<Expr *> ArraySize)
+ : ArraySize(ArraySize) {}
+
+ArraySizeOptExpr &ArraySizeOptExpr::operator=(Expr *E) {
+ ArraySize = E;
+ return *this;
+}
+
+ArraySizeOptExpr::operator bool() const { return ArraySize.operator bool(); }
+
+ArraySizeOptExpr::operator std::optional<Expr *>() const { return ArraySize; }
+
+bool ArraySizeOptExpr::hasValue() const { return ArraySize.has_value(); }
+
+Expr *ArraySizeOptExpr::value() const { return ArraySize.value(); }
+
+Expr *ArraySizeOptExpr::valueOr(Expr *default_value) const {
+ return ArraySize.value_or(default_value);
+}
+
+bool ArraySizeOptExpr::isExprTypeDependent() const {
+ return ArraySize && *ArraySize && (*ArraySize)->isTypeDependent();
+}
+
+std::optional<llvm::APSInt>
+ArraySizeOptExpr::getIntegerConstantExpr(const ASTContext &Ctx) const {
+ return ArraySize && *ArraySize ? (*ArraySize)->getIntegerConstantExpr(Ctx)
+ : std::nullopt;
+}
+
+QualType ArraySizeOptExpr::getExprType() const {
+ return ArraySize && *ArraySize ? (*ArraySize)->getType() : QualType{};
+}
+
+SourceLocation ArraySizeOptExpr::getExprBeginLoc() const {
+ return ArraySize && *ArraySize ? (*ArraySize)->getBeginLoc()
+ : SourceLocation{};
+}
+
+SourceLocation
+ArraySizeOptExpr::getExprLocOr(SourceLocation defaultValue) const {
+ return ArraySize && *ArraySize ? (*ArraySize)->getExprLoc() : defaultValue;
+}
+
+SourceRange ArraySizeOptExpr::getExprSourceRange() const {
+ return ArraySize && *ArraySize ? (*ArraySize)->getSourceRange()
+ : SourceRange{};
+}
+
+SourceRange
+ArraySizeOptExpr::getExprSourceRangeOr(SourceRange defaultValue) const {
+ return ArraySize && *ArraySize ? (*ArraySize)->getSourceRange()
+ : defaultValue;
+}
+
+} // namespace clang
``````````
</details>
https://github.com/llvm/llvm-project/pull/186617
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
