Skip to content

[clang] Remove source range from CXXOperatorCallExpr #147028

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 7, 2025
Merged

[clang] Remove source range from CXXOperatorCallExpr #147028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clang/include/clang/AST/ExprCXX.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class CXXOperatorCallExpr final : public CallExpr {
friend class ASTStmtReader;
friend class ASTStmtWriter;

SourceRange Range;
SourceLocation BeginLoc;

// CXXOperatorCallExpr has some trailing objects belonging
// to CallExpr. See CallExpr for the details.
Expand Down Expand Up @@ -158,9 +158,9 @@ class CXXOperatorCallExpr final : public CallExpr {
: getOperatorLoc();
}

SourceLocation getBeginLoc() const { return Range.getBegin(); }
SourceLocation getEndLoc() const { return Range.getEnd(); }
SourceRange getSourceRange() const { return Range; }
SourceLocation getBeginLoc() const { return BeginLoc; }
SourceLocation getEndLoc() const { return getSourceRangeImpl().getEnd(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could simplify getEndLoc() to be a trimmed-down version of getSourceRangeImpl

  • for OO_Call/ OO_Subscript => getRParenLoc()
  • for 1 argument = > getArg(0)->getEndLoc()
  • for OO_PlusPlus/OO_MinusMinus => getOperatorLoc()
  • for 2 argument: getArg(1)->getEndLoc()

I am not sure about the current handling of OO_Arrow in getSourceRangeImpl

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do this, and apply the same approach to getBeginLocImpl as well. However, I’m not sure it’s a good idea to split getSourceRangeImpl across two places -- it makes the logic harder to follow and reason about.

I actually tried this approach in an earlier version of the patch, but didn’t observe any performance benefit. Personally, I prefer the current version, which keeps all the logic centralized in getSourceRangeImpl for better maintainability and clarity.

SourceRange getSourceRange() const { return getSourceRangeImpl(); }

static bool classof(const Stmt *T) {
return T->getStmtClass() == CXXOperatorCallExprClass;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/ExprCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ CXXOperatorCallExpr::CXXOperatorCallExpr(OverloadedOperatorKind OpKind,
assert(
(CXXOperatorCallExprBits.OperatorKind == static_cast<unsigned>(OpKind)) &&
"OperatorKind overflow!");
Range = getSourceRangeImpl();
BeginLoc = getSourceRangeImpl().getBegin();
}

CXXOperatorCallExpr::CXXOperatorCallExpr(unsigned NumArgs, bool HasFPFeatures,
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Serialization/ASTReaderStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1733,7 +1733,7 @@ void ASTStmtReader::VisitMSDependentExistsStmt(MSDependentExistsStmt *S) {
void ASTStmtReader::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
VisitCallExpr(E);
E->CXXOperatorCallExprBits.OperatorKind = Record.readInt();
E->Range = Record.readSourceRange();
E->BeginLoc = Record.readSourceLocation();
}

void ASTStmtReader::VisitCXXRewrittenBinaryOperator(
Expand Down
1 change: 0 additions & 1 deletion clang/lib/Serialization/ASTWriterDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2934,7 +2934,6 @@ void ASTWriter::WriteDeclAbbrevs() {
// CXXOperatorCallExpr
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Operator Kind
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Source Location
CXXOperatorCallExprAbbrev = Stream.EmitAbbrev(std::move(Abv));

// Abbreviation for EXPR_CXX_MEMBER_CALL
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Serialization/ASTWriterStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1702,7 +1702,7 @@ void ASTStmtWriter::VisitMSDependentExistsStmt(MSDependentExistsStmt *S) {
void ASTStmtWriter::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
VisitCallExpr(E);
Record.push_back(E->getOperator());
Record.AddSourceRange(E->Range);
Record.AddSourceLocation(E->BeginLoc);

if (!E->hasStoredFPFeatures() && !static_cast<bool>(E->getADLCallKind()))
AbbrevToUse = Writer.getCXXOperatorCallExprAbbrev();
Expand Down
Loading