-
Notifications
You must be signed in to change notification settings - Fork 26
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
fix MakeFunctionCallable
for dealing with public typedef to private type
#407
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1600,7 +1600,12 @@ namespace Cpp { | |
PrintingPolicy Policy) { | ||
//TODO: Implement cling desugaring from utils::AST | ||
// cling::utils::Transform::GetPartiallyDesugaredType() | ||
QT = QT.getDesugaredType(C); | ||
if (!QT->isTypedefNameType() || QT->isBuiltinType()) | ||
QT = QT.getDesugaredType(C); | ||
#if CLANG_VERSION_MAJOR > 16 | ||
Policy.SuppressElaboration = true; | ||
#endif | ||
Policy.FullyQualifiedName = true; | ||
QT.getAsStringInternal(type_name, Policy); | ||
} | ||
|
||
|
@@ -1652,13 +1657,13 @@ namespace Cpp { | |
return; | ||
} else if (QT->isPointerType()) { | ||
isPointer = true; | ||
QT = cast<clang::PointerType>(QT)->getPointeeType(); | ||
QT = cast<clang::PointerType>(QT.getCanonicalType())->getPointeeType(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no header providing "clang::cast" is directly included [misc-include-cleaner] QT = cast<clang::PointerType>(QT.getCanonicalType())->getPointeeType();
^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add test covering that line? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. 👍🏽 |
||
} else if (QT->isReferenceType()) { | ||
if (QT->isRValueReferenceType()) | ||
refType = kRValueReference; | ||
else | ||
refType = kLValueReference; | ||
QT = cast<ReferenceType>(QT)->getPointeeType(); | ||
QT = cast<ReferenceType>(QT.getCanonicalType())->getPointeeType(); | ||
} | ||
// Fall through for the array type to deal with reference/pointer ro array | ||
// type. | ||
|
@@ -1911,7 +1916,7 @@ namespace Cpp { | |
make_narg_ctor_with_return(FD, N, class_name, buf, indent_level); | ||
return; | ||
} | ||
QualType QT = FD->getReturnType().getCanonicalType(); | ||
QualType QT = FD->getReturnType(); | ||
if (QT->isVoidType()) { | ||
std::ostringstream typedefbuf; | ||
std::ostringstream callbuf; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,5 +4,8 @@ Checks: > | |
-cppcoreguidelines-macro-usage, | ||
-cppcoreguidelines-pro-type-cstyle-cast, | ||
-cppcoreguidelines-avoid-non-const-global-variables, | ||
-cppcoreguidelines-avoid-c-arrays, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we ignore running clang-tidy on the unittest folder instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have disabled these tests in the unittests folder. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably start disabling one by one. Maybe some checks still make sense... |
||
-cppcoreguidelines-pro-bounds-array-to-pointer-decay, | ||
-performance-unnecessary-value-param, | ||
-performance-no-int-to-ptr, | ||
-bugprone-multi-level-implicit-pointer-conversion, |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
#include "Utils.h" | ||
|
||
#include "clang/AST/ASTContext.h" | ||
#include "clang/Interpreter/CppInterOp.h" | ||
#include "clang/Basic/Version.h" | ||
#include "clang/Frontend/CompilerInstance.h" | ||
#include "clang/Interpreter/CppInterOp.h" | ||
#include "clang/Sema/Sema.h" | ||
|
||
#include "gtest/gtest.h" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: 'gtest/gtest.h' file not found [clang-diagnostic-error] #include "gtest/gtest.h"
^ |
||
|
@@ -974,6 +975,56 @@ TEST(FunctionReflectionTest, GetFunctionCallWrapper) { | |
|
||
FCI_Add.Invoke(&result, {args, /*args_size=*/2}); | ||
EXPECT_EQ(result, a + b); | ||
|
||
// call with pointers | ||
Interp->process(R"( | ||
void set_5(int *out) { | ||
*out = 5; | ||
} | ||
)"); | ||
|
||
Cpp::TCppScope_t set_5 = Cpp::GetNamed("set_5"); | ||
EXPECT_TRUE(set_5); | ||
|
||
Cpp::JitCall set_5_f = Cpp::MakeFunctionCallable(set_5); | ||
EXPECT_EQ(set_5_f.getKind(), Cpp::JitCall::kGenericCall); | ||
|
||
int* bp = &b; | ||
void* set_5_args[1] = {(void*)&bp}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays] void* set_5_args[1] = {(void*)&bp};
^ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: multilevel pointer conversion from 'int **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] void* set_5_args[1] = {(void*)&bp};
^ |
||
set_5_f.Invoke(nullptr, {set_5_args, 1}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay] set_5_f.Invoke(nullptr, {set_5_args, 1});
^ |
||
EXPECT_EQ(b, 5); | ||
|
||
#if CLANG_VERSION_MAJOR > 16 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no header providing "CLANG_VERSION_MAJOR" is directly included [misc-include-cleaner] unittests/CppInterOp/FunctionReflectionTest.cpp:9: - #include <string>
+ #include <clang/Basic/Version.h>
+ #include <string> |
||
// typedef resolution testing | ||
// supported for clang version >16 only | ||
Interp->process(R"( | ||
class TypedefToPrivateClass { | ||
private: | ||
class PC { | ||
public: | ||
int m_val = 4; | ||
}; | ||
|
||
public: | ||
typedef PC PP; | ||
static PP f() { return PC(); } | ||
}; | ||
)"); | ||
|
||
Cpp::TCppScope_t TypedefToPrivateClass = | ||
Cpp::GetNamed("TypedefToPrivateClass"); | ||
EXPECT_TRUE(TypedefToPrivateClass); | ||
|
||
Cpp::TCppScope_t f = Cpp::GetNamed("f", TypedefToPrivateClass); | ||
EXPECT_TRUE(f); | ||
|
||
Cpp::JitCall FCI_f = Cpp::MakeFunctionCallable(f); | ||
EXPECT_EQ(FCI_f.getKind(), Cpp::JitCall::kGenericCall); | ||
|
||
void* res = nullptr; | ||
FCI_f.Invoke(&res, {nullptr, 0}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: multilevel pointer conversion from 'void **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion] FCI_f.Invoke(&res, {nullptr, 0});
^ |
||
EXPECT_TRUE(res); | ||
#endif | ||
} | ||
|
||
TEST(FunctionReflectionTest, IsConstMethod) { | ||
|
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.
The
SuppressElaboration
Printing Policy is not part ofclang <= 16
, therefor CI fails.