Skip to content

Commit b68ff81

Browse files
committed
[BoundsSafety][NFC] Simplify the interface of
`BoundsSafetyCheckAssignmentToCountAttrPtrWithIncompletePointeeTy` Previously the interface took a std::function<std::string>. The rationale behind this was to prevent callers from always allocating and computing a `std::string`. While trying to upstream this code (rdar://133600117) (llvm#106321) it was pointed out that there might be a simpler way to implement this. This patch instead has callers pass * a `ValueDecl*` pointer. In the cases where this isn't known (currently return values and unnamed parameters) this can be set to nullptr * a boolean flag stating whether or not the `ValueDecl*` should be fully qualified when printed. This avoids needing to pass a `std::function` and also avoids `std::string` unnecessary allocation. rdar://142544708
1 parent fd340c7 commit b68ff81

File tree

3 files changed

+46
-40
lines changed

3 files changed

+46
-40
lines changed

clang/include/clang/Sema/Sema.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2076,16 +2076,18 @@ class Sema final : public SemaBase {
20762076
/// \param RHSExpr The expression being assigned from.
20772077
/// \param Action The type assignment being performed
20782078
/// \param Loc The SourceLocation to use for error diagnostics
2079-
/// \param ComputeAssignee If provided this function will be called before
2080-
/// emitting a diagnostic. The function should return the name of
2081-
/// entity being assigned to or an empty string if this cannot be
2082-
/// determined.
2079+
/// \param Assignee The ValueDecl being assigned. This is used to compute
2080+
/// the name of the assignee. If the assignee isn't known this can
2081+
/// be set to nullptr.
2082+
/// \param ShowFullyQualifiedAssigneeName If set to true when using \p
2083+
/// Assignee to compute the name of the assignee use the fully
2084+
/// qualified name, otherwise use the unqualified name.
20832085
///
20842086
/// \returns True iff no diagnostic where emitted, false otherwise.
20852087
bool BoundsSafetyCheckAssignmentToCountAttrPtr(
20862088
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
2087-
SourceLocation Loc,
2088-
llvm::function_ref<std::string()> ComputeAssignee = nullptr);
2089+
SourceLocation Loc, const ValueDecl *Assignee,
2090+
bool ShowFullyQualifiedAssigneeName);
20892091

20902092
/// Perform Bounds Safety Semantic checks for initializing a Bounds Safety
20912093
/// pointer.

clang/lib/Sema/SemaBoundsSafety.cpp

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -288,30 +288,38 @@ HasCountedByAttrOnIncompletePointee(QualType Ty, NamedDecl **ND) {
288288
/// \param RHSExpr The expression being assigned from.
289289
/// \param Action The type assignment being performed
290290
/// \param Loc The SourceLocation to use for error diagnostics
291-
/// \param ComputeAssignee If provided this function will be called before
292-
/// emitting a diagnostic. The function should return the name of
293-
/// entity being assigned to or an empty string if this cannot be
294-
/// determined.
291+
/// \param Assignee The ValueDecl being assigned. This is used to compute
292+
/// the name of the assignee. If the assignee isn't known this can
293+
/// be set to nullptr.
294+
/// \param ShowFullyQualifiedAssigneeName If set to true when using \p
295+
/// Assignee to compute the name of the assignee use the fully
296+
/// qualified name, otherwise use the unqualified name.
295297
///
296298
/// \returns True iff no diagnostic where emitted, false otherwise.
297299
static bool CheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
298300
Sema &S, QualType LHSTy, Expr *RHSExpr, AssignmentAction Action,
299-
SourceLocation Loc, llvm::function_ref<std::string()> ComputeAssignee) {
301+
SourceLocation Loc, const ValueDecl *Assignee,
302+
bool ShowFullyQualifiedAssigneeName) {
300303
NamedDecl *IncompleteTyDecl = nullptr;
301304
auto [CATy, PointeeTy] =
302305
HasCountedByAttrOnIncompletePointee(LHSTy, &IncompleteTyDecl);
303306
if (!CATy)
304307
return true;
305308

306-
// By having users provide a function we only pay the cost of allocation the
307-
// string and computing when a diagnostic is emitted.
308-
std::string Assignee = ComputeAssignee ? ComputeAssignee() : "";
309+
std::string AssigneeStr;
310+
if (Assignee) {
311+
if (ShowFullyQualifiedAssigneeName) {
312+
AssigneeStr = Assignee->getQualifiedNameAsString();
313+
} else {
314+
AssigneeStr = Assignee->getNameAsString();
315+
}
316+
}
309317
{
310318
auto D = S.Diag(Loc, diag::err_counted_by_on_incomplete_type_on_assign)
311-
<< static_cast<int>(Action) << Assignee << (Assignee.size() > 0)
312-
<< isa<ImplicitValueInitExpr>(RHSExpr) << LHSTy
313-
<< CATy->getAttributeName(/*WithMacroPrefix=*/true) << PointeeTy
314-
<< CATy->isOrNull();
319+
<< static_cast<int>(Action) << AssigneeStr
320+
<< (AssigneeStr.size() > 0) << isa<ImplicitValueInitExpr>(RHSExpr)
321+
<< LHSTy << CATy->getAttributeName(/*WithMacroPrefix=*/true)
322+
<< PointeeTy << CATy->isOrNull();
315323

316324
if (RHSExpr->getSourceRange().isValid())
317325
D << RHSExpr->getSourceRange();
@@ -323,9 +331,10 @@ static bool CheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
323331

324332
bool Sema::BoundsSafetyCheckAssignmentToCountAttrPtr(
325333
QualType LHSTy, Expr *RHSExpr, AssignmentAction Action, SourceLocation Loc,
326-
llvm::function_ref<std::string()> ComputeAssignee) {
334+
const ValueDecl *Assignee, bool ShowFullyQualifiedAssigneeName) {
327335
return CheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
328-
*this, LHSTy, RHSExpr, Action, Loc, ComputeAssignee);
336+
*this, LHSTy, RHSExpr, Action, Loc, Assignee,
337+
ShowFullyQualifiedAssigneeName);
329338
}
330339

331340
bool Sema::BoundsSafetyCheckInitialization(const InitializedEntity &Entity,
@@ -346,13 +355,9 @@ bool Sema::BoundsSafetyCheckInitialization(const InitializedEntity &Entity,
346355
Entity.getKind() != InitializedEntity::EK_Variable) {
347356

348357
if (!CheckAssignmentToCountAttrPtrWithIncompletePointeeTy(
349-
*this, LHSType, RHSExpr, Action, SL, [&Entity]() -> std::string {
350-
if (const auto *VD =
351-
dyn_cast_or_null<ValueDecl>(Entity.getDecl())) {
352-
return VD->getQualifiedNameAsString();
353-
}
354-
return "";
355-
})) {
358+
*this, LHSType, RHSExpr, Action, SL,
359+
dyn_cast_or_null<ValueDecl>(Entity.getDecl()),
360+
/*ShowFullQualifiedAssigneeName=*/true)) {
356361
return false;
357362
}
358363
}

clang/lib/Sema/SemaExpr.cpp

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13736,20 +13736,19 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
1373613736
// Even if this check fails don't return early to allow the best
1373713737
// possible error recovery and to allow any subsequent diagnostics to
1373813738
// work.
13739+
const ValueDecl *Assignee = nullptr;
13740+
bool ShowFullyQualifiedAssigneeName = false;
13741+
// In simple cases describe what is being assigned to
13742+
if (auto *DR = dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenCasts())) {
13743+
Assignee = DR->getDecl();
13744+
} else if (auto *ME = dyn_cast<MemberExpr>(LHSExpr->IgnoreParenCasts())) {
13745+
Assignee = ME->getMemberDecl();
13746+
ShowFullyQualifiedAssigneeName = true;
13747+
}
13748+
1373913749
(void)BoundsSafetyCheckAssignmentToCountAttrPtr(
13740-
LHSType, RHS.get(), AssignmentAction::Assigning, Loc,
13741-
[&LHSExpr]() -> std::string {
13742-
// In simple cases describe what is being assigned to
13743-
if (auto *DR = dyn_cast<DeclRefExpr>(LHSExpr->IgnoreParenCasts())) {
13744-
auto *II = DR->getDecl()->getDeclName().getAsIdentifierInfo();
13745-
if (II)
13746-
return II->getName().str();
13747-
} else if (auto *ME =
13748-
dyn_cast<MemberExpr>(LHSExpr->IgnoreParenCasts())) {
13749-
return ME->getMemberDecl()->getQualifiedNameAsString();
13750-
}
13751-
return "";
13752-
});
13750+
LHSType, RHS.get(), AssignmentAction::Assigning, Loc, Assignee,
13751+
ShowFullyQualifiedAssigneeName);
1375313752
}
1375413753

1375513754
// OpenCL v1.2 s6.1.1.1 p2:

0 commit comments

Comments
 (0)