Skip to content

Commit

Permalink
Fix default arguments to only map to null if pointers
Browse files Browse the repository at this point in the history
This bug is revealed by properly fixing the reading of ABI parameters in the parser.

Signed-off-by: Dimitar Dobrev <[email protected]>
  • Loading branch information
ddobrev committed Jun 17, 2019
1 parent 458293e commit 1fc1b4e
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 33 deletions.
14 changes: 2 additions & 12 deletions src/AST/FunctionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,8 @@ public static IList<Parameter> GatherInternalParams(this Function function,

var pointer = new QualifiedType(new PointerType(new QualifiedType(new BuiltinType(PrimitiveType.Void))));

if (isInstanceMethod && !isItaniumLikeAbi)
{
@params.Add(new Parameter
{
QualifiedType = pointer,
Name = "__instance",
Namespace = function
});
}

if (!function.HasIndirectReturnTypeParameter &&
isInstanceMethod && isItaniumLikeAbi)
if (isInstanceMethod &&
(!isItaniumLikeAbi || !function.HasIndirectReturnTypeParameter))
{
@params.Add(new Parameter
{
Expand Down
16 changes: 5 additions & 11 deletions src/CppParser/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2923,14 +2923,9 @@ Enumeration::Item* Parser::WalkEnumItem(clang::EnumConstantDecl* ECD)
static const clang::CodeGen::CGFunctionInfo& GetCodeGenFunctionInfo(
clang::CodeGen::CodeGenTypes* CodeGenTypes, const clang::FunctionDecl* FD)
{
using namespace clang;
if (auto CD = dyn_cast<clang::CXXConstructorDecl>(FD)) {
return CodeGenTypes->arrangeCXXStructorDeclaration(CD);
} else if (auto DD = dyn_cast<clang::CXXDestructorDecl>(FD)) {
return CodeGenTypes->arrangeCXXStructorDeclaration(DD);
}

return CodeGenTypes->arrangeFunctionDeclaration(FD);
auto FTy = FD->getType()->getCanonicalTypeUnqualified();
return CodeGenTypes->arrangeFreeFunctionType(
FTy.castAs<clang::FunctionProtoType>());
}

bool Parser::CanCheckCodeGenInfo(clang::Sema& S, const clang::Type* Ty)
Expand Down Expand Up @@ -3294,9 +3289,8 @@ void Parser::WalkFunction(const clang::FunctionDecl* FD, Function* F,
unsigned Index = 0;
for (const auto& Arg : CGInfo.arguments())
{
if (Index >= F->Parameters.size())
continue;
F->Parameters[Index++]->isIndirect = Arg.info.isIndirect();
F->Parameters[Index++]->isIndirect =
Arg.info.isIndirect() && !Arg.info.getIndirectByVal();
}

MarkValidity(F);
Expand Down
5 changes: 0 additions & 5 deletions src/Generator/Driver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,6 @@ public void SetupPasses(ILibrary library)

if (Options.IsCSharpGenerator)
{
if (Options.GenerateDefaultValuesForArguments)
{
TranslationUnitPasses.AddPass(new FixDefaultParamValuesOfOverridesPass());
TranslationUnitPasses.AddPass(new HandleDefaultParamValuesPass());
}
TranslationUnitPasses.AddPass(new GenerateAbstractImplementationsPass());
TranslationUnitPasses.AddPass(new MultipleInheritancePass());
}
Expand Down
6 changes: 6 additions & 0 deletions src/Generator/Generators/CSharp/CSharpGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ public override List<CodeGenerator> Generate(IEnumerable<TranslationUnit> units)

public override bool SetupPasses()
{
if (Context.Options.GenerateDefaultValuesForArguments)
{
Context.TranslationUnitPasses.AddPass(new FixDefaultParamValuesOfOverridesPass());
Context.TranslationUnitPasses.AddPass(new HandleDefaultParamValuesPass());
}

// Both the CheckOperatorsOverloadsPass and CheckAbiParameters can
// create and and new parameters to functions and methods. Make sure
// CheckAbiParameters runs last because hidden structure parameters
Expand Down
2 changes: 1 addition & 1 deletion src/Generator/Passes/HandleDefaultParamValuesPass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public override bool VisitFunctionDecl(Function function)
{
var desugared = type.Desugar();

if (!desugared.IsPrimitiveTypeConvertibleToRef() &&
if (desugared.IsAddress() && !desugared.IsPrimitiveTypeConvertibleToRef() &&
(expression.String == "0" || expression.String == "nullptr"))
{
result = desugared.GetPointee()?.Desugar() is FunctionType ?
Expand Down
2 changes: 1 addition & 1 deletion tests/CSharp/CSharp.Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ public void TestDefaultArguments()
methodsWithDefaultValues.DefaultMappedToZeroEnum();
methodsWithDefaultValues.DefaultMappedToEnumAssignedWithCtor();
methodsWithDefaultValues.DefaultZeroMappedToEnumAssignedWithCtor();
methodsWithDefaultValues.DefaultImplicitCtorInt();
Assert.That(methodsWithDefaultValues.DefaultImplicitCtorInt().Priv, Is.EqualTo(0));
methodsWithDefaultValues.DefaultImplicitCtorChar();
methodsWithDefaultValues.DefaultImplicitCtorFoo();
methodsWithDefaultValues.DefaultImplicitCtorEnum();
Expand Down
10 changes: 8 additions & 2 deletions tests/CSharp/CSharp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ Quux::Quux() : _setterWithDefaultOverload(0)

Quux::Quux(int i) : Quux()
{

priv = i;
}

Quux::Quux(char c) : Quux()
Expand All @@ -139,6 +139,11 @@ Quux::~Quux()
}
}

int Quux::getPriv() const
{
return priv;
}

Foo* Quux::setterWithDefaultOverload()
{
return _setterWithDefaultOverload;
Expand Down Expand Up @@ -645,8 +650,9 @@ void MethodsWithDefaultValues::defaultZeroMappedToEnumAssignedWithCtor(DefaultZe
{
}

void MethodsWithDefaultValues::defaultImplicitCtorInt(Quux arg)
Quux MethodsWithDefaultValues::defaultImplicitCtorInt(Quux arg)
{
return arg;
}

void MethodsWithDefaultValues::defaultImplicitCtorChar(Quux arg)
Expand Down
3 changes: 2 additions & 1 deletion tests/CSharp/CSharp.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class DLL_API Quux
Quux(Foo f);
~Quux();

int getPriv() const;
Foo* setterWithDefaultOverload();
void setSetterWithDefaultOverload(Foo* value = new Foo());

Expand Down Expand Up @@ -419,7 +420,7 @@ class DLL_API MethodsWithDefaultValues : public Quux
void defaultMappedToZeroEnum(QFlags<Flags> qFlags = 0);
void defaultMappedToEnumAssignedWithCtor(QFlags<Flags> qFlags = QFlags<Flags>());
void defaultZeroMappedToEnumAssignedWithCtor(DefaultZeroMappedToEnum defaultZeroMappedToEnum = DefaultZeroMappedToEnum());
void defaultImplicitCtorInt(Quux arg = 0);
Quux defaultImplicitCtorInt(Quux arg = 0);
void defaultImplicitCtorChar(Quux arg = 'a');
void defaultImplicitCtorFoo(Quux arg = Foo());
// this looks the same test as 'defaultRefTypeEnumImplicitCtor' two lines below
Expand Down

0 comments on commit 1fc1b4e

Please sign in to comment.