mono / CppSharp

Tools and libraries to glue C/C++ APIs to high-level languages
MIT License
3.12k stars 512 forks source link

Map primitive C++ constants to C# constants #674

Open ddobrev opened 8 years ago

ddobrev commented 8 years ago

We use dynamic calls in C# to resolve symbols for C++ constants. This has the following three problems:

  1. It's much slower than reading a hard-coded value;
  2. It prevents constants from being used in default values of parameters, attributes and anywhere else a compile-time value is required;
  3. It's needlessly complex because constants rarely if ever change their values across versions; also, because of the tight integration C++# creates between the binding and the native library it's unsafe anyway to use a binding generated for one version with a newer one.

We can actually support (in most cases) complex constants (objects) too by using C# read-only fields. But let's just start with primitive constants: integers, Booleans, possibly strings.

rokups commented 6 years ago

I am looking at this issue. Do you have any advice how i could extract constant values from variable declarations? I can see DebugText has this info but it probably is a very wrong and fragile approach to use that.

tritao commented 6 years ago

You will need to extend the AST and parser to properly support this.

@ddobrev has already done most of the heavy work in translating Clang expressions nodes as part of support for default method parameter values, so what you need to do is:

  1. Extend Variable native AST node with Expression* field

https://github.com/mono/CppSharp/blob/master/src/CppParser/AST.h#L810

  1. Extend VarDecl node handling to convert the expression to our node

https://github.com/mono/CppSharp/blob/master/src/CppParser/Parser.cpp#L3298 https://github.com/mono/CppSharp/blob/master/src/CppParser/Parser.cpp#L3488

See AST::Expression* Parser::WalkExpression(const clang::Expr* Expr): https://clang.llvm.org/doxygen/classclang_1_1VarDecl.html#a9121be86e97f008ae3ea599cfb094b5

  1. Re-generate parser bindings

  2. Extend Variable.cs and ASTConverter.cs to deal with expression field.

https://github.com/mono/CppSharp/blob/master/src/AST/Variable.cs https://github.com/mono/CppSharp/blob/master/src/Parser/ASTConverter.cs#L1451

  1. Use the info on a new pass to convert C++ variables to C# constants.
rokups commented 6 years ago

My attempt: https://github.com/rokups/CppSharp/commit/2f0a417a6aba75a030cd34c6ef72cd613cc02c71 However VisitStatement(decl.Expression); call throws NullPointerException exception on statement.Class access. I basically copied exact same steps that were used by Parameter for initializing it's DefaultArgument. Any clue what is going on here?

ddobrev commented 6 years ago

@rokups you have added a new member in C++ without regenerating the parser bindings so what happens is normal. In addition, please initialise the new field to 0 in the constructor rather than when using it. Finally, please do not stack your commits on top - produce a single commit by amending.

rokups commented 6 years ago

Oh but i did regenerate parser bindings. If i would not have done it then CppSharp would not compile because VisitStatement(decl.Expression); would be accessing something that does not exist in parser bindings.

Edit: As for commits - i guess you mean previous PR. I know, many small fix commits are useless noise. I do squash them often, but when it comes to PRs constant squashing makes it hard for reviewer to see what changed. Ideally once PR is done reviewers should not hurry to merge but instead give a green light for commit history cleanup. Then reviewing process will be easy and commit history will be clean 👍

ddobrev commented 6 years ago

@rokups for some reason the large diffs are not even noted in that link. Anyway, then try the second step, initialisation to 0 in the constructor instead.

rokups commented 6 years ago

Alright i have some kind of success. Please see my branch.

I am pretty sure there is million things i did wrong though. For starters i added constants pass that forces constant variables to be included in generation by excluding them from symbols check. Is that a valid approach?

Secondly i generate constant values based on their string representation i got from CppWrapper. For some reason even most complex expressions have class Any and seem to not contain any extra info like default parameter expressions do. var Notice how string representation is just a blob of c++ code. Got any advice for me here how to properly handle this?

tritao commented 6 years ago

Nice to see you making progress.

Add a new debug statement here: https://github.com/mono/CppSharp/blob/master/src/CppParser/Parser.cpp#L3562

Something like:

        Debug("Parser::WalkExpression: Unhandled expression kind: %s\n", Expr->getDeclKindName());

And let's see what comes up for your code.

rokups commented 6 years ago
Parser::WalkExpression: Unhandled expression kind: CXXNullPtrLiteralExpr
Parser::WalkExpression: Unhandled expression kind: ExprWithCleanups
Parser::WalkExpression: Unhandled expression kind: FloatingLiteral
Parser::WalkExpression: Unhandled expression kind: IntegerLiteral
Parser::WalkExpression: Unhandled expression kind: ParenExpr
Parser::WalkExpression: Unhandled expression kind: StringLiteral
Parser::WalkExpression: Unhandled expression kind: UnaryExprOrTypeTraitExpr
Parser::WalkExpression: Unhandled expression kind: UnaryOperator
ddobrev commented 6 years ago

@rokups supporting all expressions is really difficult, it essentially amounts to writing a C++ to C# code converter. Neither @tritao nor I have that kind of time so I have resorted to supporting expressions one case at a time. Unless you'd be willing to write support for all Clang expressions, I'm afraid you'd have to do the same.

rokups commented 6 years ago

Ah thats cool. It probably is enough taking care of litterals, anything else users can fix themselves manually by altering string representation during preprocessing step.

tritao commented 6 years ago

We don't need to support all kinds of expressions, just the subset used in constant and default parameter initializers. And some we can probably skip altogether, like ParenExpr, we just need to forward visit its inner expression like we already do for some.

@rokups I'd start with adding support for the basic literal expressions and work one case at a time until your codebase is fully supported.

tritao commented 6 years ago

I just realized there's some APIs that might make our life easier here.

@rokups Check VarDecl::ensureEvaluatedStmt () const

https://clang.llvm.org/doxygen/classclang_1_1VarDecl.html#a9121be86e97f008ae3ea599cfb094b5 https://clang.llvm.org/doxygen/structclang_1_1EvaluatedStmt.html

Or maybe even: APValue * VarDecl::evaluateValue ()

If we use this first, it might reduce the kind of expressions we need to work with. Worth giving a try.