-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Clang][counted-by] Bail out of visitor for LValueToRValue cast #125571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
An LValueToRValue cast shouldn't be ignored, so bail out of the visitor if we encounter one.
@llvm/pr-subscribers-clang Author: Bill Wendling (bwendling) ChangesAn LValueToRValue cast shouldn't be ignored, so bail out of the visitor if we encounter one. Full diff: https://github.com/llvm/llvm-project/pull/125571.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 11fa295dad9524..339bcd14c5bc8c 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1055,20 +1055,20 @@ namespace {
/// StructFieldAccess is a simple visitor class to grab the first MemberExpr
/// from an Expr. It records any ArraySubscriptExpr we meet along the way.
class StructFieldAccess
- : public ConstStmtVisitor<StructFieldAccess, const MemberExpr *> {
+ : public ConstStmtVisitor<StructFieldAccess, const Expr *> {
bool AddrOfSeen = false;
public:
const ArraySubscriptExpr *ASE = nullptr;
- const MemberExpr *VisitMemberExpr(const MemberExpr *E) {
+ const Expr *VisitMemberExpr(const MemberExpr *E) {
if (AddrOfSeen && E->getType()->isArrayType())
// Avoid forms like '&ptr->array'.
return nullptr;
return E;
}
- const MemberExpr *VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
+ const Expr *VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
if (ASE)
// We don't support multiple subscripts.
return nullptr;
@@ -1077,17 +1077,19 @@ class StructFieldAccess
ASE = E;
return Visit(E->getBase());
}
- const MemberExpr *VisitCastExpr(const CastExpr *E) {
+ const Expr *VisitCastExpr(const CastExpr *E) {
+ if (E->getCastKind() == CK_LValueToRValue)
+ return E;
return Visit(E->getSubExpr());
}
- const MemberExpr *VisitParenExpr(const ParenExpr *E) {
+ const Expr *VisitParenExpr(const ParenExpr *E) {
return Visit(E->getSubExpr());
}
- const MemberExpr *VisitUnaryAddrOf(const clang::UnaryOperator *E) {
+ const Expr *VisitUnaryAddrOf(const clang::UnaryOperator *E) {
AddrOfSeen = true;
return Visit(E->getSubExpr());
}
- const MemberExpr *VisitUnaryDeref(const clang::UnaryOperator *E) {
+ const Expr *VisitUnaryDeref(const clang::UnaryOperator *E) {
AddrOfSeen = false;
return Visit(E->getSubExpr());
}
@@ -1188,7 +1190,7 @@ CodeGenFunction::emitCountedByMemberSize(const Expr *E, llvm::Value *EmittedE,
// GCC does for consistency's sake.
StructFieldAccess Visitor;
- const MemberExpr *ME = Visitor.Visit(E);
+ const MemberExpr *ME = dyn_cast_if_present<MemberExpr>(Visitor.Visit(E));
if (!ME)
return nullptr;
|
@llvm/pr-subscribers-clang-codegen Author: Bill Wendling (bwendling) ChangesAn LValueToRValue cast shouldn't be ignored, so bail out of the visitor if we encounter one. Full diff: https://github.com/llvm/llvm-project/pull/125571.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 11fa295dad9524..339bcd14c5bc8c 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -1055,20 +1055,20 @@ namespace {
/// StructFieldAccess is a simple visitor class to grab the first MemberExpr
/// from an Expr. It records any ArraySubscriptExpr we meet along the way.
class StructFieldAccess
- : public ConstStmtVisitor<StructFieldAccess, const MemberExpr *> {
+ : public ConstStmtVisitor<StructFieldAccess, const Expr *> {
bool AddrOfSeen = false;
public:
const ArraySubscriptExpr *ASE = nullptr;
- const MemberExpr *VisitMemberExpr(const MemberExpr *E) {
+ const Expr *VisitMemberExpr(const MemberExpr *E) {
if (AddrOfSeen && E->getType()->isArrayType())
// Avoid forms like '&ptr->array'.
return nullptr;
return E;
}
- const MemberExpr *VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
+ const Expr *VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
if (ASE)
// We don't support multiple subscripts.
return nullptr;
@@ -1077,17 +1077,19 @@ class StructFieldAccess
ASE = E;
return Visit(E->getBase());
}
- const MemberExpr *VisitCastExpr(const CastExpr *E) {
+ const Expr *VisitCastExpr(const CastExpr *E) {
+ if (E->getCastKind() == CK_LValueToRValue)
+ return E;
return Visit(E->getSubExpr());
}
- const MemberExpr *VisitParenExpr(const ParenExpr *E) {
+ const Expr *VisitParenExpr(const ParenExpr *E) {
return Visit(E->getSubExpr());
}
- const MemberExpr *VisitUnaryAddrOf(const clang::UnaryOperator *E) {
+ const Expr *VisitUnaryAddrOf(const clang::UnaryOperator *E) {
AddrOfSeen = true;
return Visit(E->getSubExpr());
}
- const MemberExpr *VisitUnaryDeref(const clang::UnaryOperator *E) {
+ const Expr *VisitUnaryDeref(const clang::UnaryOperator *E) {
AddrOfSeen = false;
return Visit(E->getSubExpr());
}
@@ -1188,7 +1190,7 @@ CodeGenFunction::emitCountedByMemberSize(const Expr *E, llvm::Value *EmittedE,
// GCC does for consistency's sake.
StructFieldAccess Visitor;
- const MemberExpr *ME = Visitor.Visit(E);
+ const MemberExpr *ME = dyn_cast_if_present<MemberExpr>(Visitor.Visit(E));
if (!ME)
return nullptr;
|
This change didn't affect the tests, which is either good or bad, depending on your point of view. I assume that it's okay because we're looking for a |
My expectation was that this would affect the tests from #125298, because the MemberExpr there is a child of an LValueToRValue conversion. I could be missing something, though? |
Those tests are meant to bail out if the MemberExpr is a pointer. That patch may be superseded by this patch? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…#125571) An LValueToRValue cast shouldn't be ignored, so bail out of the visitor if we encounter one.
An LValueToRValue cast shouldn't be ignored, so bail out of the visitor if we encounter one.