Skip to content

Commit c74690f

Browse files
john-h-kastnerjohn-h-kastner
committed
Cleanup and reduce duplicate code
1 parent d923a66 commit c74690f

File tree

4 files changed

+142
-103
lines changed

4 files changed

+142
-103
lines changed

‎clang/include/clang/3C/DeclRewriter.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ class DeclRewriter {
3737
static void
3838
buildItypeDecl(PVConstraint *Defn, DeclaratorDecl *Decl, std::string &Type,
3939
std::string &IType, ProgramInfo &Info,
40-
ArrayBoundsRewriter &ABR, std::vector<std::string> &SDecls);
40+
ArrayBoundsRewriter &ABR, std::vector<std::string> *SDecls);
41+
42+
static void
43+
buildCheckedDecl(PVConstraint *Defn, DeclaratorDecl *Decl, std::string &Type,
44+
std::string &IType, std::string UseName, ProgramInfo &Info,
45+
ArrayBoundsRewriter &ABR, std::vector<std::string> *SDecls);
4146

4247
private:
4348
static RecordDecl *LastRecordDecl;
@@ -109,14 +114,14 @@ class FunctionDeclBuilder : public RecursiveASTVisitor<FunctionDeclBuilder> {
109114
std::string &IType, std::string UseName,
110115
bool &RewriteGen, bool &RewriteParm,
111116
bool &RewriteRet, bool StaticFunc,
112-
std::vector<std::string> &SDecls);
117+
std::vector<std::string> *SDecls);
113118
void buildCheckedDecl(PVConstraint *Defn, DeclaratorDecl *Decl,
114119
std::string &Type, std::string &IType,
115120
std::string UseName, bool &RewriteParm,
116-
bool &RewriteRet, std::vector<std::string> &SDecls);
121+
bool &RewriteRet, std::vector<std::string> *SDecls);
117122
void buildItypeDecl(PVConstraint *Defn, DeclaratorDecl *Decl,
118123
std::string &Type, std::string &IType, bool &RewriteParm,
119-
bool &RewriteRet, std::vector<std::string> &SDecls);
124+
bool &RewriteRet, std::vector<std::string> *SDecls);
120125

121126
bool hasDeclWithTypedef(const FunctionDecl *FD);
122127

‎clang/lib/3C/DeclRewriter.cpp

Lines changed: 76 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -35,18 +35,22 @@ using namespace clang;
3535
void DeclRewriter::buildItypeDecl(PVConstraint *Defn, DeclaratorDecl *Decl,
3636
std::string &Type, std::string &IType,
3737
ProgramInfo &Info, ArrayBoundsRewriter &ABR,
38-
std::vector<std::string> &SDecls) {
38+
std::vector<std::string> *SDecls) {
3939
bool NeedsRangeBound =
40+
SDecls != nullptr &&
4041
Defn->hasBoundsKey() &&
4142
Info.getABoundsInfo().needsRangeBound(Defn->getBoundsKey());
43+
assert("Adding range bounds on return, global variable, or field!" &&
44+
(!NeedsRangeBound || (isa_and_nonnull<VarDecl>(Decl) &&
45+
cast<VarDecl>(Decl)->hasLocalStorage())));
4246

4347
std::string DeclName = Decl ? Decl->getNameAsString() : "";
44-
if (NeedsRangeBound) {
45-
// TODO: Think about this assert. When is Decl null? When is the name empty?
46-
// One case that comes to mind if unnamed parameter declaration.
47-
assert(!DeclName.empty());
48+
// The idea here is that the name should only be empty if this is an unnamed
49+
// parameter in a function pre-declaration, or the pre-declaration is not a
50+
// prototype so Decl is null.
51+
assert(!DeclName.empty() || Decl == nullptr || isa<ParmVarDecl>(Decl));
52+
if (NeedsRangeBound)
4853
DeclName = "__3c_tmp_" + DeclName;
49-
}
5054

5155
const EnvironmentMap &Env = Info.getConstraints().getVariables();
5256
// True when the type of this variable is defined by a typedef, and the
@@ -109,11 +113,49 @@ void DeclRewriter::buildItypeDecl(PVConstraint *Defn, DeclaratorDecl *Decl,
109113
UnmaskTypedef = IsUncheckedTypedef));
110114
IType += ")" + ABR.getBoundsString(Defn, Decl, true, false);
111115

112-
// We don't insert a bounds string because this is an unchecked pointer.
113-
if (NeedsRangeBound)
114-
SDecls.push_back(Defn->mkString(Info.getConstraints(),
115-
MKSTRING_OPTS(ForItypeBase = true)) +
116-
" = " + DeclName + ";");
116+
if (NeedsRangeBound) {
117+
// For itypes, the the copy of the array cannot use a checked type because
118+
// we know it will be used unsafely somewhere. Giving it a checked type
119+
// would result in Checked C type errors at tee unsafe uses.
120+
SDecls->push_back(Defn->mkString(Info.getConstraints(),
121+
MKSTRING_OPTS(ForItypeBase = true)) +
122+
" = " + DeclName + ";");
123+
}
124+
}
125+
126+
void DeclRewriter::buildCheckedDecl(PVConstraint *Defn, DeclaratorDecl *Decl,
127+
std::string &Type, std::string &IType,
128+
std::string UseName, ProgramInfo &Info,
129+
ArrayBoundsRewriter &ABR,
130+
std::vector<std::string> *SDecls) {
131+
// I don't like using SDecls != nullptr as a nullptr as a sentinel like this.
132+
bool NeedsRangeBound =
133+
SDecls != nullptr &&
134+
Defn->hasBoundsKey() &&
135+
Info.getABoundsInfo().needsRangeBound(Defn->getBoundsKey());
136+
assert("Adding range bounds on return, global variable, or field!" &&
137+
(!NeedsRangeBound || (isa_and_nonnull<VarDecl>(Decl) &&
138+
cast<VarDecl>(Decl)->hasLocalStorage())));
139+
140+
std::string DeclName = UseName;
141+
// The idea here is that the name should only be empty if this is an unnamed
142+
// parameter in a function pre-declaration, or the pre-declaration is not a
143+
// prototype so Decl is null. In all of these cases, we don't want to insert
144+
// a duplicate declaration, so we don't need a tmp name.
145+
if (NeedsRangeBound) {
146+
assert(!DeclName.empty());
147+
DeclName = "__3c_tmp_" + DeclName;
148+
}
149+
150+
Type =
151+
Defn->mkString(Info.getConstraints(), MKSTRING_OPTS(UseName = DeclName));
152+
IType = ABR.getBoundsString(Defn, Decl);
153+
if (NeedsRangeBound) {
154+
SDecls->push_back(
155+
Defn->mkString(Info.getConstraints(), MKSTRING_OPTS(UseName = UseName)) +
156+
ABR.getBoundsString(Defn, Decl, false, true, DeclName) + " = " +
157+
DeclName + ";");
158+
}
117159
}
118160

119161
// This function is the public entry point for declaration rewriting.
@@ -212,7 +254,7 @@ void DeclRewriter::rewriteDecls(ASTContext &Context, ProgramInfo &Info,
212254
if (VDLToStmtMap.find(D) != VDLToStmtMap.end())
213255
DS = VDLToStmtMap[D];
214256

215-
std::string NewTy = getStorageQualifierString(D);
257+
std::string Type, IType;
216258
std::vector<std::string> SupplementaryDecls;
217259
bool IsExternGlobalVar =
218260
isa<VarDecl>(D) &&
@@ -226,40 +268,16 @@ void DeclRewriter::rewriteDecls(ASTContext &Context, ProgramInfo &Info,
226268
// used. This does provide most of the rewriting infrastructure that
227269
// would be required to support these itypes if constraint generation
228270
// is updated to handle structure/global itypes.
229-
std::string Type, IType;
230271
// VarDecl and FieldDecl subclass DeclaratorDecl, so the cast will
231272
// always succeed.
232273
DeclRewriter::buildItypeDecl(PV, cast<DeclaratorDecl>(D), Type, IType,
233-
Info, ABRewriter, SupplementaryDecls);
234-
NewTy += Type + IType;
274+
Info, ABRewriter, &SupplementaryDecls);
235275
} else {
236-
237-
bool NeedsRangeBounds =
238-
PV->hasBoundsKey() &&
239-
Info.getABoundsInfo().needsRangeBound(PV->getBoundsKey());
240-
if (NeedsRangeBounds) {
241-
std::string NewName = "__3c_tmp_" + PV->getName();
242-
NewTy += PV->mkString(Info.getConstraints(),
243-
MKSTRING_OPTS(UseName = NewName)) +
244-
ABRewriter.getBoundsString(PV, D);
245-
std::string SDecl = PV->mkString(Info.getConstraints()) +
246-
ABRewriter.getBoundsString(PV, D, false, true,
247-
NewName);
248-
if (auto *VD = dyn_cast<VarDecl>(D)) {
249-
if (VD->hasLocalStorage()) {
250-
SDecl += " = " + NewName;
251-
} else if (VD->hasInit()) {
252-
SDecl += " = " + getSourceText(VD->getInit()->getSourceRange(),
253-
Context);
254-
}
255-
}
256-
SDecl += ";";
257-
SupplementaryDecls.push_back(SDecl);
258-
} else {
259-
NewTy += PV->mkString(Info.getConstraints()) +
260-
ABRewriter.getBoundsString(PV, D);
261-
}
276+
DeclRewriter::buildCheckedDecl(PV, cast<DeclaratorDecl>(D), Type,
277+
IType, PV->getName(), Info, ABRewriter,
278+
&SupplementaryDecls);
262279
}
280+
std::string NewTy = getStorageQualifierString(D) + Type + IType;
263281

264282
if (auto *VD = dyn_cast<VarDecl>(D)) {
265283
VarDeclReplacement *VDR = new VarDeclReplacement(VD, DS, NewTy);
@@ -269,9 +287,8 @@ void DeclRewriter::rewriteDecls(ASTContext &Context, ProgramInfo &Info,
269287
RewriteThese.insert(std::make_pair(VD, VDR));
270288
} else if (auto *FD = dyn_cast<FieldDecl>(D)) {
271289
FieldDeclReplacement *FDR = new FieldDeclReplacement(FD, DS, NewTy);
272-
FDR->SupplementaryDecls.insert(FDR->SupplementaryDecls.begin(),
273-
SupplementaryDecls.begin(),
274-
SupplementaryDecls.end());
290+
assert("Supplementary declaration created for FieldDecl." &&
291+
SupplementaryDecls.empty());
275292
RewriteThese.insert(std::make_pair(FD, FDR));
276293
} else
277294
llvm_unreachable("Unrecognized declaration type.");
@@ -711,7 +728,11 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
711728

712729
// TODO: These are only used if(FD->isThisDeclarationADefinition()), so we
713730
// don't need to populate the vector otherwise.
714-
std::vector<std::string> SupplementaryDecls;
731+
std::vector<std::string> *SupplementaryDecls = nullptr;
732+
std::vector<std::string> SDecls;
733+
if (FD->isThisDeclarationADefinition())
734+
SupplementaryDecls = &SDecls;
735+
715736

716737
// Get rewritten parameter variable declarations. Try to use
717738
// the source for as much as possible.
@@ -871,9 +892,11 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
871892
if (RewriteReturn || RewriteParams) {
872893
auto *FDR = new FunctionDeclReplacement(FD, NewSig, RewriteReturn,
873894
RewriteParams, RewriteGeneric);
874-
FDR->SupplementaryDecls.insert(FDR->SupplementaryDecls.begin(),
875-
SupplementaryDecls.begin(),
876-
SupplementaryDecls.end());
895+
if (SupplementaryDecls != nullptr) {
896+
FDR->SupplementaryDecls.insert(FDR->SupplementaryDecls.begin(),
897+
SupplementaryDecls->begin(),
898+
SupplementaryDecls->end());
899+
}
877900
RewriteThese.insert(std::make_pair(FD, FDR));
878901
}
879902

@@ -883,37 +906,20 @@ bool FunctionDeclBuilder::VisitFunctionDecl(FunctionDecl *FD) {
883906
void FunctionDeclBuilder::buildCheckedDecl(
884907
PVConstraint *Defn, DeclaratorDecl *Decl, std::string &Type,
885908
std::string &IType, std::string UseName, bool &RewriteParm,
886-
bool &RewriteRet, std::vector<std::string> &SDecls) {
887-
888-
bool NeedsRangeBound =
889-
Defn->hasBoundsKey() &&
890-
Info.getABoundsInfo().needsRangeBound(Defn->getBoundsKey());
891-
892-
std::string DeclName = UseName;
893-
if (NeedsRangeBound)
894-
DeclName = "__3c_tmp_" + DeclName;
895-
896-
Type =
897-
Defn->mkString(Info.getConstraints(), MKSTRING_OPTS(UseName = DeclName));
898-
IType = ABRewriter.getBoundsString(Defn, Decl, !IType.empty());
909+
bool &RewriteRet, std::vector<std::string> *SDecls) {
910+
DeclRewriter::buildCheckedDecl(Defn, Decl, Type, IType, UseName, Info,
911+
ABRewriter, SDecls);
899912
RewriteParm |= getExistingIType(Defn).empty() != IType.empty() ||
900913
isa_and_nonnull<ParmVarDecl>(Decl);
901914
RewriteRet |= isa_and_nonnull<FunctionDecl>(Decl);
902-
903-
if (NeedsRangeBound) {
904-
SDecls.push_back(Defn->mkString(Info.getConstraints()) +
905-
ABRewriter.getBoundsString(Defn, Decl, false, true,
906-
DeclName) + " = " + DeclName +
907-
";");
908-
}
909915
}
910916

911917

912918
void FunctionDeclBuilder::buildItypeDecl(PVConstraint *Defn,
913919
DeclaratorDecl *Decl,
914920
std::string &Type, std::string &IType,
915921
bool &RewriteParm, bool &RewriteRet,
916-
std::vector<std::string> &SDecls) {
922+
std::vector<std::string> *SDecls) {
917923
Info.getPerfStats().incrementNumITypes();
918924
DeclRewriter::buildItypeDecl(Defn, Decl, Type, IType, Info, ABRewriter,
919925
SDecls);
@@ -930,7 +936,7 @@ void FunctionDeclBuilder::buildDeclVar(const FVComponentVariable *CV,
930936
std::string &IType, std::string UseName,
931937
bool &RewriteGen, bool &RewriteParm,
932938
bool &RewriteRet, bool StaticFunc,
933-
std::vector<std::string> &SDecls) {
939+
std::vector<std::string> *SDecls) {
934940

935941
bool CheckedSolution = CV->hasCheckedSolution(Info.getConstraints());
936942
bool ItypeSolution = CV->hasItypeSolution(Info.getConstraints());

‎clang/lib/3C/RewriteUtils.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -448,8 +448,15 @@ class AssignmentUpdater : public RecursiveASTVisitor<AssignmentUpdater> {
448448
Info.getABoundsInfo().needsRangeBound(CV->getBoundsKey())) {
449449
std::string TmpVarName = "__3c_tmp_" + CV->getName();
450450
rewriteSourceRange(R, O->getLHS()->getSourceRange(), TmpVarName);
451-
R.InsertTextAfterToken(O->getEndLoc(),
452-
", " + CV->getName() + " = " + TmpVarName);
451+
bool InsertFail = R.InsertTextAfterToken(O->getEndLoc(),
452+
", " + CV->getName() +
453+
" = " + TmpVarName);
454+
if (InsertFail) {
455+
// FIXME: Use rewriteSourceRange so that it handle emitting error messages.
456+
llvm::errs()
457+
<< "Rewriting failed while updating assignment!\n";
458+
O->getEndLoc().print(llvm::errs(), R.getSourceMgr());
459+
}
453460
}
454461
}
455462
}

0 commit comments

Comments
 (0)