Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Checks: >
-cppcoreguidelines-init-variables,
-cppcoreguidelines-pro-bounds-pointer-arithmetic,
-cppcoreguidelines-pro-type-member-init,
-cppcoreguidelines-macro-usage,
-llvm-namespace-comment

CheckOptions:
Expand Down
93 changes: 56 additions & 37 deletions include/clang/Interpreter/CppInterOp.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,42 +37,57 @@ namespace Cpp {
using TInterp_t = void*;
using TCppObject_t = void*;

enum BinaryOperator {
PtrMemD = 0,
PtrMemI,
Mul,
Div,
Rem,
Add,
Sub,
Shl,
Shr,
Cmp,
LT,
GT,
LE,
GE,
EQ,
NE,
And,
Xor,
Or,
LAnd,
LOr,
Assign,
MulAssign,
DivAssign,
RemAssign,
AddAssign,
SubAssign,
ShlAssign,
ShrAssign,
AndAssign,
XorAssign,
OrAssign,
Comma,
enum Operator {
OP_None,
OP_New,
OP_Delete,
OP_Array_New,
OP_Array_Delete,
OP_Plus,
OP_Minus,
OP_Star,
OP_Slash,
OP_Percent,
OP_Caret,
OP_Amp,
OP_Pipe,
OP_Tilde,
OP_Exclaim,
OP_Equal,
OP_Less,
OP_Greater,
OP_PlusEqual,
OP_MinusEqual,
OP_StarEqual,
OP_SlashEqual,
OP_PercentEqual,
OP_CaretEqual,
OP_AmpEqual,
OP_PipeEqual,
OP_LessLess,
OP_GreaterGreater,
OP_LessLessEqual,
OP_GreaterGreaterEqual,
OP_EqualEqual,
OP_ExclaimEqual,
OP_LessEqual,
OP_GreaterEqual,
OP_Spaceship,
OP_AmpAmp,
OP_PipePipe,
OP_PlusPlus,
OP_MinusMinus,
OP_Comma,
OP_ArrowStar,
OP_Arrow,
OP_Call,
OP_Subscript,
OP_Conditional,
OP_Coawait,
};

enum OperatorArity { kUnary = 1, kBinary, kBoth };
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: enum 'OperatorArity' uses a larger base type ('unsigned int', size: 4 bytes) than necessary for its value set, consider using 'std::uint8_t' (1 byte) as the base type to reduce its size [performance-enum-size]

  enum OperatorArity { kUnary = 1, kBinary, kBoth };
       ^

Copy link
Contributor

@vgvassilev vgvassilev Dec 18, 2024

Choose a reason for hiding this comment

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

Suggested change
enum OperatorArity { kUnary = 1, kBinary, kBoth };
enum OperatorArity { kBoth = 0, kUnary, kBinary };

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did not understand this.
All Unary operators are not Binary operators. So kBoth = 0 and kUnary = 0 is confusing. Example ~ operator.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, my bad, fixed the suggestion.


/// A class modeling function calls for functions produced by the interpreter
/// in compiled code. It provides an information if we are calling a standard
/// function, constructor or destructor.
Expand Down Expand Up @@ -513,9 +528,13 @@ namespace Cpp {
CPPINTEROP_API std::string GetFunctionArgName(TCppFunction_t func,
TCppIndex_t param_index);

///\returns function that performs operation op on lc and rc
void GetBinaryOperator(TCppScope_t scope, enum BinaryOperator op,
std::vector<TCppFunction_t>& operators);
///\returns arity of the operator or kNone
OperatorArity GetOperatorArity(TCppFunction_t op);

///\returns list of operator overloads
void GetOperator(TCppScope_t scope, Operator op,
std::vector<TCppFunction_t>& operators,
OperatorArity kind = kBoth);

/// Creates an instance of the interpreter we need for the various interop
/// services.
Expand Down
49 changes: 35 additions & 14 deletions lib/Interpreter/CppInterOp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3315,22 +3315,43 @@
return PI->getNameAsString();
}

void GetBinaryOperator(TCppScope_t scope, enum BinaryOperator op,
std::vector<TCppFunction_t>& operators) {
Decl* D = static_cast<Decl*>(scope);
auto* DC = llvm::dyn_cast<DeclContext>(D);
Scope* S = getSema().getScopeForContext(DC);
if (!S)
return;

clang::UnresolvedSet<8> lookup;
OperatorArity GetOperatorArity(TCppFunction_t op) {
Decl* D = static_cast<Decl*>(op);
if (auto* FD = llvm::dyn_cast<FunctionDecl>(D)) {
if (FD->isOverloadedOperator()) {
switch (FD->getOverloadedOperator()) {
#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, \
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: function-like macro 'OVERLOADED_OPERATOR' used; consider a 'constexpr' template function [cppcoreguidelines-macro-usage]

#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary,            \
        ^

Copy link
Contributor

Choose a reason for hiding this comment

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

That type of check should be disabled in our clang-tidy config..

MemberOnly) \
case OO_##Name: \
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

    auto *D = (clang::Decl *)method;
              ^

if ((Unary) && (Binary)) \
return kBoth; \
if (Unary) \
return kUnary; \
if (Binary) \
return kBinary; \
break;
#include "clang/Basic/OperatorKinds.def"
default:
break;

Check warning on line 3335 in lib/Interpreter/CppInterOp.cpp

View check run for this annotation

Codecov / codecov/patch

lib/Interpreter/CppInterOp.cpp#L3334-L3335

Added lines #L3334 - L3335 were not covered by tests
}
}
}
return (OperatorArity)~0U;

Check warning on line 3339 in lib/Interpreter/CppInterOp.cpp

View check run for this annotation

Codecov / codecov/patch

lib/Interpreter/CppInterOp.cpp#L3339

Added line #L3339 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: The value '4294967295' provided to the cast expression is not in the valid range of values for 'OperatorArity' [clang-analyzer-optin.core.EnumCastOutOfRange]

    return (OperatorArity)~0U;
           ^
Additional context

include/clang/Interpreter/CppInterOp.h:88: enum declared here

  enum OperatorArity { kUnary = 1, kBinary, kBoth };
       ^

lib/Interpreter/CppInterOp.cpp:3319: Assuming 'D' is not a 'CastReturnType'

    if (auto* FD = llvm::dyn_cast<FunctionDecl>(D)) {
                   ^

lib/Interpreter/CppInterOp.cpp:3319: 'FD' is null

    if (auto* FD = llvm::dyn_cast<FunctionDecl>(D)) {
              ^

lib/Interpreter/CppInterOp.cpp:3319: Taking false branch

    if (auto* FD = llvm::dyn_cast<FunctionDecl>(D)) {
    ^

lib/Interpreter/CppInterOp.cpp:3338: The value '4294967295' provided to the cast expression is not in the valid range of values for 'OperatorArity'

    return (OperatorArity)~0U;
           ^

}
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]

    auto *D = (clang::Decl *)func;
              ^


getSema().LookupBinOp(S, SourceLocation(), (clang::BinaryOperatorKind)op,
lookup);
void GetOperator(TCppScope_t scope, Operator op,
std::vector<TCppFunction_t>& operators, OperatorArity kind) {
Decl* D = static_cast<Decl*>(scope);
if (auto* DC = llvm::dyn_cast_or_null<DeclContext>(D)) {
ASTContext& C = getSema().getASTContext();
DeclContextLookupResult Result =
DC->lookup(C.DeclarationNames.getCXXOperatorName(
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: Called C++ object pointer is null [clang-analyzer-core.CallAndMessage]

    return PI->getNameAsString();
           ^
Additional context

lib/Interpreter/CppInterOp.cpp:3299: 'PI' initialized to a null pointer value

    clang::ParmVarDecl* PI = nullptr;
    ^

lib/Interpreter/CppInterOp.cpp:3301: Assuming null pointer is passed into cast

    if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionDecl>(D))
                   ^

lib/Interpreter/CppInterOp.cpp:3301: 'FD' is null

    if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionDecl>(D))
              ^

lib/Interpreter/CppInterOp.cpp:3301: Taking false branch

    if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionDecl>(D))
    ^

lib/Interpreter/CppInterOp.cpp:3303: Assuming null pointer is passed into cast

    else if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionTemplateDecl>(D))
                        ^

lib/Interpreter/CppInterOp.cpp:3303: 'FD' is null

    else if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionTemplateDecl>(D))
                   ^

lib/Interpreter/CppInterOp.cpp:3303: Taking false branch

    else if (auto* FD = llvm::dyn_cast_or_null<clang::FunctionTemplateDecl>(D))
         ^

lib/Interpreter/CppInterOp.cpp:3306: Called C++ object pointer is null

    return PI->getNameAsString();
           ^

(clang::OverloadedOperatorKind)op));

for (NamedDecl* D : lookup) {
if (auto* FD = llvm::dyn_cast<Decl>(D))
operators.push_back(FD);
for (auto* i : Result) {
if (kind & GetOperatorArity(i))
operators.push_back(i);
}
}
}

Expand Down
38 changes: 33 additions & 5 deletions unittests/CppInterOp/ScopeReflectionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ TEST(ScopeReflectionTest, IncludeVector) {
Interp->process(code);
}

TEST(ScopeReflectionTest, GetBinaryOperator) {
TEST(ScopeReflectionTest, GetOperator) {
if (llvm::sys::RunningOnValgrind())
GTEST_SKIP() << "XFAIL due to Valgrind report";

Expand All @@ -988,27 +988,55 @@ TEST(ScopeReflectionTest, GetBinaryOperator) {
return MyClass(lhs.x + rhs.x);
}

namespace extra_ops {

MyClass operator+(MyClass lhs, int rhs) {
return MyClass(lhs.x + rhs);
}

MyClass operator+(int lhs, MyClass rhs) {
return MyClass(lhs + rhs.x);
}

MyClass operator~(MyClass self) {
return MyClass(-self.x);
}

}
)";

Cpp::Declare(code.c_str());

std::vector<Cpp::TCppFunction_t> ops;

Cpp::GetBinaryOperator(Cpp::GetGlobalScope(), Cpp::BinaryOperator::Add, ops);
EXPECT_EQ(ops.size(), 3);
Cpp::GetOperator(Cpp::GetGlobalScope(), Cpp::Operator::OP_Plus, ops);
EXPECT_EQ(ops.size(), 1);
ops.clear();

Cpp::GetOperator(Cpp::GetGlobalScope(), Cpp::Operator::OP_Minus, ops);
EXPECT_EQ(ops.size(), 1);
ops.clear();

Cpp::GetOperator(Cpp::GetGlobalScope(), Cpp::Operator::OP_Star, ops);
EXPECT_EQ(ops.size(), 0);
ops.clear();

// operators defined within a namespace
Cpp::GetOperator(Cpp::GetScope("extra_ops"), Cpp::Operator::OP_Plus, ops);
EXPECT_EQ(ops.size(), 2);
ops.clear();

// unary operator
Cpp::GetOperator(Cpp::GetScope("extra_ops"), Cpp::Operator::OP_Tilde, ops);
EXPECT_EQ(ops.size(), 1);
ops.clear();

Cpp::GetBinaryOperator(Cpp::GetGlobalScope(), Cpp::BinaryOperator::Sub, ops);
Cpp::GetOperator(Cpp::GetScope("extra_ops"), Cpp::Operator::OP_Tilde, ops,
Cpp::OperatorArity::kUnary);
EXPECT_EQ(ops.size(), 1);
ops.clear();

Cpp::GetBinaryOperator(Cpp::GetGlobalScope(), Cpp::BinaryOperator::Mul, ops);
Cpp::GetOperator(Cpp::GetScope("extra_ops"), Cpp::Operator::OP_Tilde, ops,
Cpp::OperatorArity::kBinary);
EXPECT_EQ(ops.size(), 0);
}