Skip to content

Commit c96e373

Browse files
nunoplopesnunoplopes
committed
Code refactoring
1 parent ee98fe0 commit c96e373

14 files changed

Lines changed: 348 additions & 392 deletions

File tree

‎cpp2rust/converter/converter.cpp‎

Lines changed: 81 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,7 @@ std::string Converter::EmitMaterializedTempBinding(clang::QualType param_type,
286286
clang::Expr *expr) {
287287
assert(materialized_temp_bindings_ && "materialized temp emitted outside a "
288288
"HoistMaterializedTempBindings scope");
289-
auto [binding, ref] = MaterializeTemp(
290-
std::format("__tmp_{}", materialized_temp_id_++), param_type, expr);
289+
auto [binding, ref] = MaterializeTemp(NewTempName(), param_type, expr);
291290
*materialized_temp_bindings_ += std::move(binding);
292291
return ref;
293292
}
@@ -1413,13 +1412,10 @@ void Converter::ConvertCondition(clang::Expr *cond) {
14131412
}
14141413

14151414
bool Converter::VisitIfStmt(clang::IfStmt *stmt) {
1415+
// The variables of the init statement are only visible in the if.
1416+
PushBrace scope(*this, stmt->getInit());
14161417
if (auto *init = stmt->getInit()) {
1417-
PushBrace scope(*this);
14181418
Convert(init);
1419-
stmt->setInit(nullptr);
1420-
Convert(stmt);
1421-
stmt->setInit(init);
1422-
return false;
14231419
}
14241420
if (bool taken = false;
14251421
!stmt->getConditionVariable() && !stmt->isConsteval() &&
@@ -1500,14 +1496,12 @@ bool Converter::VisitCXXForRangeStmt(clang::CXXForRangeStmt *stmt) {
15001496
auto *begin =
15011497
clang::cast<clang::VarDecl>(stmt->getBeginStmt()->getSingleDecl());
15021498
auto *end = clang::cast<clang::VarDecl>(stmt->getEndStmt()->getSingleDecl());
1503-
begin->setDeclName(
1504-
clang::isa<clang::DecompositionDecl>(stmt->getLoopVariable())
1505-
? clang::DeclarationName(
1506-
&ctx_.Idents.get(GetNamedDeclAsString(stmt->getLoopVariable())))
1507-
: stmt->getLoopVariable()->getDeclName());
1499+
// The iterator is named after the loop variable, which shadows it.
1500+
auto name = GetNamedDeclAsString(stmt->getLoopVariable());
1501+
range_iterator_names_[begin] = name;
15081502
StrCat("'loop_:");
1509-
StrCat(keyword::kFor, GetNamedDeclAsString(begin), keyword::kIn,
1510-
"IterRange::new(", ConvertFreshRValue(begin->getInit()), token::kComma,
1503+
StrCat(keyword::kFor, name, keyword::kIn, "IterRange::new(",
1504+
ConvertFreshRValue(begin->getInit()), token::kComma,
15111505
ConvertFreshRValue(end->getInit()), ')');
15121506
{
15131507
PushBrace brace(*this);
@@ -1741,21 +1735,6 @@ void Converter::ConvertCallToOstream(clang::CallExpr *expr) {
17411735
assert(*fmt_trait == '\0' && "Stream state was not restored after call");
17421736
}
17431737

1744-
void Converter::ConvertPrintf(clang::CallExpr *expr) {
1745-
bool is_fprintf = IsCDecl(expr->getDirectCallee(), "fprintf");
1746-
1747-
StrCat("printf(");
1748-
for (unsigned i = is_fprintf; i < expr->getNumArgs(); ++i) {
1749-
Convert(expr->getArg(i));
1750-
// The format string
1751-
if (i == is_fprintf) {
1752-
StrCat("as *const i8");
1753-
}
1754-
StrCat(token::kComma);
1755-
}
1756-
StrCat(')');
1757-
}
1758-
17591738
void Converter::ConvertVariadicArg(clang::Expr *arg) {
17601739
PushExprKind push(*this, ExprKind::RValue);
17611740
if (arg->getType()->isFunctionPointerType()) {
@@ -1783,25 +1762,40 @@ void Converter::ConvertVAArgCall(clang::CallExpr *expr) {
17831762
}
17841763

17851764
bool Converter::VisitCallExpr(clang::CallExpr *expr) {
1765+
if (ConvertSpecialCall(expr)) {
1766+
return false;
1767+
}
1768+
std::optional<TempMaterializationCtx> temps;
1769+
std::string str;
1770+
{
1771+
Buffer buf(*this);
1772+
temps = ConvertCallExpr(expr);
1773+
str = std::move(buf).str();
1774+
}
1775+
EmitCallResult(expr, std::move(str), temps ? &*temps : nullptr);
1776+
return false;
1777+
}
1778+
1779+
bool Converter::ConvertSpecialCall(clang::CallExpr *expr) {
17861780
if (IsBuiltinVaStart(expr) || IsBuiltinVaEnd(expr) || IsBuiltinVaCopy(expr)) {
17871781
ConvertVAArgCall(expr);
17881782
MarkFresh();
1789-
return false;
1783+
return true;
17901784
}
17911785

17921786
// p->~T() on a scalar is a no-op
17931787
if (clang::isa<clang::CXXPseudoDestructorExpr>(
17941788
expr->getCallee()->IgnoreParenImpCasts())) {
17951789
MarkFresh();
1796-
return false;
1790+
return true;
17971791
}
17981792

17991793
// p->~T() is a no-op when T has nothing to destruct
18001794
if (auto *dtor = clang::dyn_cast_or_null<clang::CXXDestructorDecl>(
18011795
expr->getCalleeDecl());
18021796
dtor && !RecordNeedsDestruction(dtor->getParent())) {
18031797
MarkFresh();
1804-
return false;
1798+
return true;
18051799
}
18061800

18071801
if (auto conversion = clang::dyn_cast_or_null<clang::CXXConversionDecl>(
@@ -1810,76 +1804,51 @@ bool Converter::VisitCallExpr(clang::CallExpr *expr) {
18101804
ConvertLambdaToFunctionPointer(clang::cast<clang::CXXMemberCallExpr>(expr)
18111805
->getImplicitObjectArgument(),
18121806
conversion);
1813-
return false;
1807+
return true;
18141808
}
18151809

1816-
if (IsImplicitAssignmentCall(expr) && !Mapper::Contains(ctx_, expr)) {
1817-
auto *call = clang::cast<clang::CXXMemberCallExpr>(expr);
1818-
ConvertAssignment(call->getImplicitObjectArgument(), call->getArg(0), "=");
1810+
// The remaining calls are converted as rules say, if there is one.
1811+
if (Mapper::Contains(ctx_, expr)) {
18191812
return false;
18201813
}
18211814

1822-
if (Mapper::Contains(ctx_, expr)) {
1823-
if (RuleRegistry::IsLibcPassthrough(ctx_, expr)) {
1824-
ConvertGenericCallExpr(expr);
1825-
return false;
1826-
}
1827-
1828-
bool deref_ref = (IsReferenceType(expr) ||
1829-
GetReturnTypeOfFunction(expr)->isReferenceType()) &&
1830-
!isAddrOf() && !isVoid();
1831-
1832-
auto **args = expr->getArgs();
1833-
auto num_args = expr->getNumArgs();
1834-
auto ctx = CollectRefBindingTempArgs(expr);
1835-
auto str = GetMappedAsString(expr, args, num_args, &ctx);
1836-
1837-
if (deref_ref && !Mapper::IsRefExpr(ctx_, expr)) {
1838-
Buffer buf(*this);
1839-
EmitDeref(std::move(str), expr->getType());
1840-
str = std::move(buf).str();
1841-
}
1842-
1843-
if (!ctx.temporary_bindings.empty()) {
1844-
str = std::format("{{ {} {} }}", ctx.temporary_bindings, str);
1845-
}
1846-
1847-
StrCat(str);
1848-
if (deref_ref) {
1849-
SetValueFreshness(expr->getType());
1850-
} else if (!IsPassThroughRule(expr)) {
1851-
MarkFresh();
1852-
}
1853-
return false;
1815+
if (IsImplicitAssignmentCall(expr)) {
1816+
auto *call = clang::cast<clang::CXXMemberCallExpr>(expr);
1817+
ConvertAssignment(call->getImplicitObjectArgument(), call->getArg(0), "=");
1818+
return true;
18541819
}
18551820

18561821
if (IsTransparentStdCall(expr)) {
18571822
Convert(expr->getArg(0));
1858-
return false;
1823+
return true;
18591824
}
18601825

18611826
if (auto *opcall = clang::dyn_cast<clang::CXXOperatorCallExpr>(expr);
1862-
opcall && !IsUserOperatorCall(opcall) && !Mapper::Contains(ctx_, expr)) {
1863-
return ConvertCXXOperatorCallExpr(opcall);
1827+
opcall && !IsUserOperatorCall(opcall)) {
1828+
ConvertCXXOperatorCallExpr(opcall);
1829+
return true;
18641830
}
1831+
return false;
1832+
}
18651833

1866-
std::string str;
1867-
{
1834+
void Converter::EmitCallResult(clang::CallExpr *expr, std::string str,
1835+
const TempMaterializationCtx *temps) {
1836+
bool deref = (GetReturnTypeOfFunction(expr)->isReferenceType() ||
1837+
(temps && IsReferenceType(expr))) &&
1838+
!isAddrOf() && !isVoid();
1839+
// The dereference is in the block of the temporaries, so they are still
1840+
// alive.
1841+
if (deref && !(temps && Mapper::IsRefExpr(ctx_, expr))) {
18681842
Buffer buf(*this);
1869-
Converter::ConvertCallExpr(expr);
1843+
EmitDeref(std::move(str), expr->getType());
18701844
str = std::move(buf).str();
18711845
}
1872-
1873-
auto ty = GetReturnTypeOfFunction(expr);
1874-
auto ref = ty->getAs<clang::ReferenceType>();
1875-
1876-
if (ref && !isAddrOf() && !isVoid()) {
1877-
EmitDeref(std::move(str), ref->getPointeeType());
1878-
return false;
1846+
StrCat(temps ? temps->Wrap(std::move(str)) : str);
1847+
if (deref) {
1848+
SetValueFreshness(expr->getType());
1849+
} else if (!temps || !IsPassThroughRule(expr)) {
1850+
MarkFresh();
18791851
}
1880-
1881-
EmitFresh(str);
1882-
return false;
18831852
}
18841853

18851854
void Converter::ConvertLambdaToFunctionPointer(
@@ -2212,14 +2181,7 @@ std::optional<Converter::TempMaterializationCtx>
22122181
Converter::ConvertCallExpr(clang::CallExpr *expr) {
22132182
auto *callee = expr->getCallee();
22142183

2215-
bool is_printf = IsCDecl(expr->getDirectCallee(), "printf");
2216-
bool is_fprintf = IsCDecl(expr->getDirectCallee(), "fprintf");
2217-
unsigned format_arg = is_fprintf ? 1 : 0;
2218-
if ((is_printf || is_fprintf) &&
2219-
clang::isa<clang::StringLiteral>(
2220-
expr->getArg(format_arg)->IgnoreImplicit())) {
2221-
ConvertPrintf(expr);
2222-
} else if (IsBuiltinConstantP(callee)) {
2184+
if (IsBuiltinConstantP(callee)) {
22232185
clang::APValue V;
22242186
StrCat(expr->getArg(0)->isCXX11ConstantExpr(ctx_
22252187
#if CLANG_VERSION_MAJOR >= 24
@@ -2229,7 +2191,8 @@ Converter::ConvertCallExpr(clang::CallExpr *expr) {
22292191
)
22302192
? token::kOne
22312193
: token::kZero);
2232-
} else if (Mapper::Contains(ctx_, expr)) {
2194+
} else if (Mapper::Contains(ctx_, expr) &&
2195+
!RuleRegistry::IsLibcPassthrough(ctx_, expr)) {
22332196
auto **args = expr->getArgs();
22342197
auto num_args = expr->getNumArgs();
22352198
auto ctx = CollectRefBindingTempArgs(expr);
@@ -3090,6 +3053,10 @@ std::string Converter::ConvertDeclRef(clang::Expr *expr,
30903053
GetNamedDeclAsString(decl));
30913054
}
30923055

3056+
if (auto it = range_iterator_names_.find(decl);
3057+
it != range_iterator_names_.end()) {
3058+
return it->second;
3059+
}
30933060
return GetNamedDeclAsString(decl);
30943061
}
30953062

@@ -3184,20 +3151,17 @@ bool Converter::ConvertCXXOperatorCallExpr(clang::CXXOperatorCallExpr *expr) {
31843151
// Rust borrows the container before computing the index, so an index that
31853152
// reads the container, or that has side effects through anything the
31863153
// container is reached through, is computed beforehand.
3187-
std::unordered_set<const clang::ValueDecl *> base_vars;
3188-
auto *inner = base->IgnoreParenImpCasts();
3189-
if (idx->HasSideEffects(ctx_)) {
3190-
base_vars = GetAllVars(base);
3191-
} else if (auto *ref = clang::dyn_cast<clang::DeclRefExpr>(inner)) {
3192-
base_vars.insert(ref->getDecl());
3193-
} else if (auto *member = clang::dyn_cast<clang::MemberExpr>(inner)) {
3194-
base_vars.insert(member->getMemberDecl());
3195-
} else {
3196-
base_vars = GetAllVars(base);
3154+
const clang::ValueDecl *container = nullptr;
3155+
if (!idx->HasSideEffects(ctx_)) {
3156+
auto *inner = base->IgnoreParenImpCasts();
3157+
if (auto *ref = clang::dyn_cast<clang::DeclRefExpr>(inner)) {
3158+
container = ref->getDecl();
3159+
} else if (auto *member = clang::dyn_cast<clang::MemberExpr>(inner)) {
3160+
container = member->getMemberDecl();
3161+
}
31973162
}
3198-
if (std::ranges::any_of(GetAllVars(idx), [&](const clang::ValueDecl *var) {
3199-
return base_vars.contains(var);
3200-
})) {
3163+
if (container ? GetAllVars(idx).contains(container)
3164+
: ReferToCommonVar(idx, base)) {
32013165
std::string idx_str;
32023166
{
32033167
ScopedValue no_autoref(autoref_mut_, std::nullopt);
@@ -4925,19 +4889,24 @@ Converter::CollectRefBindingTempArgs(clang::CallExpr *expr) {
49254889
return ctx;
49264890
}
49274891

4892+
std::string Converter::TempMaterializationCtx::Wrap(std::string str) const {
4893+
if (temporary_bindings.empty()) {
4894+
return str;
4895+
}
4896+
return std::format("{{ {} {} }}", temporary_bindings, str);
4897+
}
4898+
49284899
const std::string &Converter::TempMaterializationCtx::GetOrMaterialize(
49294900
unsigned argument_num,
4930-
std::function<std::pair<std::string, std::string>(const std::string &,
4931-
clang::QualType)>
4901+
std::function<std::pair<std::string, std::string>(clang::QualType)>
49324902
materialize_fn) {
49334903
auto &str = materialized_refs_.at(argument_num);
49344904
if (!str.empty()) {
49354905
return str;
49364906
}
49374907

49384908
if (auto m = materialized_args.at(argument_num)) {
4939-
auto [binding, ref] =
4940-
materialize_fn(std::format("__tmp_{}", argument_num), *m);
4909+
auto [binding, ref] = materialize_fn(*m);
49414910
temporary_bindings += std::move(binding);
49424911
str = std::move(ref);
49434912
return str;
@@ -4973,8 +4942,8 @@ std::string Converter::ConvertPlaceholder(clang::Expr *expr, clang::Expr *arg,
49734942
if (ph_ctx.needs_materialization()) {
49744943
auto materialized = ph_ctx.materialize_ctx->GetOrMaterialize(
49754944
static_cast<unsigned>(ph_ctx.materialize_idx),
4976-
[this, arg](const std::string &name, clang::QualType type) {
4977-
return MaterializeTemp(name, type, arg);
4945+
[this, arg](clang::QualType type) {
4946+
return MaterializeTemp(NewTempName(), type, arg);
49784947
});
49794948
if (!materialized.empty()) {
49804949
return materialized;

‎cpp2rust/converter/converter.h‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -284,10 +284,12 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
284284

285285
const std::string &GetOrMaterialize(
286286
unsigned argument_num,
287-
std::function<std::pair<std::string, std::string>(const std::string &,
288-
clang::QualType)>
287+
std::function<std::pair<std::string, std::string>(clang::QualType)>
289288
materialize_fn);
290289

290+
// str, preceded by the bindings of the temporaries that it refers to.
291+
std::string Wrap(std::string str) const;
292+
291293
private:
292294
std::vector<std::string> materialized_refs_;
293295
};
@@ -329,7 +331,18 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
329331
void dump() const;
330332
};
331333

332-
std::optional<TempMaterializationCtx> ConvertCallExpr(clang::CallExpr *expr);
334+
// Converts the calls that are converted to something other than a call,
335+
// and returns whether expr is one.
336+
bool ConvertSpecialCall(clang::CallExpr *expr);
337+
338+
// Converts the call expr, and returns the temporaries of the arguments of a
339+
// call translated by a rule.
340+
virtual std::optional<TempMaterializationCtx>
341+
ConvertCallExpr(clang::CallExpr *expr);
342+
343+
// Emits the result of the call expr, which was converted to str.
344+
virtual void EmitCallResult(clang::CallExpr *expr, std::string str,
345+
const TempMaterializationCtx *temps);
333346

334347
struct CallArg {
335348
enum class Kind : int8_t {
@@ -414,15 +427,13 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
414427
return false;
415428
}
416429

417-
virtual void ConvertPrintf(clang::CallExpr *expr);
418-
419-
void ConvertVAArgCall(clang::CallExpr *expr);
430+
virtual void ConvertVAArgCall(clang::CallExpr *expr);
420431

421432
virtual void ConvertVariadicArg(clang::Expr *arg);
422433

423434
void DefineImplicitMembers(clang::CXXRecordDecl *decl);
424435

425-
virtual bool VisitCallExpr(clang::CallExpr *expr);
436+
bool VisitCallExpr(clang::CallExpr *expr);
426437

427438
virtual bool VisitIntegerLiteral(clang::IntegerLiteral *expr);
428439

@@ -901,6 +912,9 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
901912
std::optional<bool> autoref_mut_;
902913

903914
std::vector<clang::Expr *> curr_for_inc_;
915+
// The names of the iterators of range-based for loops.
916+
std::unordered_map<const clang::ValueDecl *, std::string>
917+
range_iterator_names_;
904918
std::vector<clang::QualType> curr_init_type_;
905919

906920
enum class BreakTarget : int8_t { Loop, FallthroughSwitch, Switch };
@@ -916,6 +930,9 @@ class Converter : public clang::RecursiveASTVisitor<Converter> {
916930
std::unordered_set<const clang::VarDecl *> hoisted_decls_;
917931

918932
unsigned materialized_temp_id_ = 0;
933+
std::string NewTempName() {
934+
return std::format("__tmp_{}", materialized_temp_id_++);
935+
}
919936
std::string *materialized_temp_bindings_ = nullptr;
920937
class HoistMaterializedTempBindings {
921938
Converter &c;

0 commit comments

Comments
 (0)