Support bindings for literal defines #50

kornilova203 · 2018-06-12T11:52:44Z

Part of #4

Currently the code only finds defines and does not generate any code for it.

I think it would be better to implement end-to-end testing before moving to this one #51

kornilova203 · 2018-06-17T10:00:27Z

I decided that literals will be places directly to Scala code, so their values will be known by a programmer without looking into header file (see Support Bindings for Defines Design Doc)

This commit will be only about literals.

Currently there are following issues:

We need to find a way to get Scala type for numeric_constant token.
For example, we need to know that this token 10000000000 is Long, so we can correctly use it in a Scala file:
```
object DefineDefines {
  val LONG: native.CLong = 10000000000L
}
```
Find a way to follow chain of defines.
Example:
```
#define INT 42
#define INT_2 INT
```
Check that defines are not undef below in the header
Check that DefineFinderAction does not sees a define if #if/ #ifdef directive above the define is false.
Handle negative numbers correctly. They consist of 2 tokens

kornilova203 · 2018-06-17T19:29:14Z

Last commit is incorrect, I'll fix it tomorrow

jonas · 2018-06-18T00:54:24Z

bindgen/visitor/ScalaFrontendActionFactory.cpp

-        llvm::errs() << "IR is not empty. Please use new instance of "
-                        "ScalaFrontendActionFactory.\n";
-        llvm::errs().flush();
-    }


Yes I guess this is pointless now.

jonas · 2018-06-18T00:55:30Z

bindgen/Main.cpp


+    DefineFinderActionFactory defineFinderActionFactory(ir);
+    Tool.run(&defineFinderActionFactory);


Let's check the returned result code

jonas · 2018-06-18T01:00:05Z

bindgen/defines/DefineFinder.cpp

+                llvm::errs().flush();
+            }
+        } else if (finalToken->isAnyIdentifier()) {
+            // TODO: save identifier and get its type in ScalaFrontendAction


Would it make sense to use while (token->isAnyIdentifier()) in getFinalIdentifier() so this branch never happens?

No, because finalToken might still be anyIdentifier, since not all defines have literal values.
So the loop may never end. But I can reorganize the code so it will be a bit more clear

We need this branch because we want to find defines that are aliases for variables.

jonas · 2018-06-18T01:03:37Z

bindgen/defines/DefineFinder.cpp

+}
+
+const clang::Token *
+DefineFinder::getFinalIdentifier(const clang::Token &token) const {


I'm not familiar with the AST but I suggest renaming this method to resolveIdentifier() and recursively call it on a token until we resolve it to a literal or something we don't handle. Maybe I misunderstood something here though.

Name is good
It should be called recursively until the identifier is not an another define, but the problem is that defines may contain multiple tokens:

extern int a; #define MY_A a #define MY_A_PTR MY_A * // two tokens, one of them is another define

So the function might get too complicated. I'll try to find clang function that will resolve value of define for us

jonas · 2018-06-18T01:05:14Z

bindgen/defines/DefineFinder.cpp

+    : ir(ir), compiler(compiler), pp(pp) {}
+
+void DefineFinder::MacroDefined(const clang::Token &MacroNameTok,
+                                const clang::MacroDirective *MD) {


From a style point of view I find it confusing to have variable names starting with upper. We don't do it for ir.

The names are from the overriden methods, but they can be renamed

jonas · 2018-06-18T01:09:18Z

tests/samples/Define.h

+
+extern int a;
+#define MY_A a // unsupported
+


#define wait_for_it(cond) do { sleep (1000); } while (!cond)

jonas · 2018-06-18T01:12:11Z

bindgen/ir/IR.h

@@ -28,6 +29,11 @@ class IR {
    void addUnion(std::string name, std::vector<Field> fields,
                  uint64_t maxSize);

+    void addLiteralDefine(std::string name, std::string literal);
+
+    void addLiteralDefine(std::string name, std::string literal,


Can type have a default value to reduce this to one method?

LiteralDefine always have a type now, so I can simply delete the method without a type

kornilova203 · 2018-06-18T07:52:46Z

I couldn't find a function that would expand a define.
My expandDefine implementation is able to follow only a simple chain of defines:

#define INT 42
#define INT_2 INT

But I think it shouldn't be a big problem to write the implementation that handles more difficult cases.

Once we expand a define we need to process it. If it is a literal then there is no problems to embed it in the Scala code, but if it's not then we need to find a way to check if it's a variable of data type or of pointer type (or something else). Currently I don't know how to do that.

kornilova203 · 2018-06-18T07:59:48Z

Tests in a container fail because clang API changed since version 4.0
I need to update to 5.0v
We also may want to note somewhere that only versions >= 5.0 are supported

kornilova203 · 2018-06-18T09:52:23Z

To check is a define is an alias for a variable maybe it is a good idea to generate C code that includes the header and uses the value, feed it into FrontendAction and see if there will be errors

kornilova203 · 2018-06-19T08:21:15Z

I think saving all floating point numbers as native.CDouble is good enough for now.

kornilova203 · 2018-06-19T11:34:55Z

Maybe some of C literals are not valid literals in Scala, so it also has to be checked

jonas · 2018-06-20T03:34:41Z

We can test against LLVM 4 if you want. It would however be nice to support more recent versions.

jonas

LGTM

jonas · 2018-06-20T03:43:41Z

I suggest to wait with merging this until 0.1 has been tagged.

kornilova203 force-pushed the support-bindings-for-defines branch 2 times, most recently from cc24b1b to 458aa66 Compare June 12, 2018 12:24

kornilova203 force-pushed the master branch from 1570b61 to 5c1b35e Compare June 15, 2018 19:07

kornilova203 added 2 commits June 17, 2018 13:22

Find defines

2efe884

Generate Scala code for literal definitions

1ce2e45

kornilova203 force-pushed the support-bindings-for-defines branch from 458aa66 to 1ce2e45 Compare June 17, 2018 09:46

kornilova203 changed the title ~~[WIP] Support bindings for defines~~ [WIP] Support bindings for literal defines Jun 17, 2018

kornilova203 added 4 commits June 17, 2018 14:17

Check macro conditionals

0104a5b

Remove undefined defines

4a2c905

Get type of numeric_constant

abe03c4

Get type of integers without ending l or ll

42de8c0

jonas reviewed Jun 18, 2018

View reviewed changes

Follow simple chain of defines. Small fixes

c8c7543

kornilova203 force-pushed the support-bindings-for-defines branch from c0bbf73 to c8c7543 Compare June 18, 2018 07:43

kornilova203 force-pushed the support-bindings-for-defines branch 2 times, most recently from 7b81274 to 5a67df5 Compare June 19, 2018 07:58

kornilova203 requested a review from jonas June 19, 2018 07:58

kornilova203 force-pushed the support-bindings-for-defines branch from 5a67df5 to fad3366 Compare June 19, 2018 08:16

Support negative values

e288ac9

kornilova203 force-pushed the support-bindings-for-defines branch from fad3366 to e288ac9 Compare June 19, 2018 08:20

kornilova203 changed the title ~~[WIP] Support bindings for literal defines~~ Support bindings for literal defines Jun 19, 2018

jonas approved these changes Jun 20, 2018

View reviewed changes

kornilova203 force-pushed the support-bindings-for-defines branch from 0537917 to 41585a3 Compare June 21, 2018 14:46

Convert C literals to Scala literals

2db8071

kornilova203 force-pushed the support-bindings-for-defines branch from 41585a3 to 2db8071 Compare June 21, 2018 18:26

Filter private defines

471b06a

kornilova203 changed the base branch from master to 0.2 June 22, 2018 17:47

kornilova203 merged commit 7b0bccb into 0.2 Jun 22, 2018

kornilova203 deleted the support-bindings-for-defines branch June 23, 2018 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support bindings for literal defines #50

Support bindings for literal defines #50

kornilova203 commented Jun 12, 2018 •

edited

Loading

kornilova203 commented Jun 17, 2018 •

edited

Loading

kornilova203 commented Jun 17, 2018

jonas Jun 18, 2018

jonas Jun 18, 2018

jonas Jun 18, 2018

kornilova203 Jun 18, 2018

kornilova203 Jun 18, 2018

jonas Jun 18, 2018

kornilova203 Jun 18, 2018

jonas Jun 18, 2018

kornilova203 Jun 18, 2018

jonas Jun 18, 2018

jonas Jun 18, 2018

kornilova203 Jun 18, 2018

kornilova203 commented Jun 18, 2018

kornilova203 commented Jun 18, 2018

kornilova203 commented Jun 18, 2018 •

edited

Loading

kornilova203 commented Jun 19, 2018

kornilova203 commented Jun 19, 2018

jonas commented Jun 20, 2018

jonas left a comment

jonas commented Jun 20, 2018


		DefineFinderActionFactory defineFinderActionFactory(ir);
		Tool.run(&defineFinderActionFactory);

Support bindings for literal defines #50

Support bindings for literal defines #50

Conversation

kornilova203 commented Jun 12, 2018 • edited Loading

kornilova203 commented Jun 17, 2018 • edited Loading

kornilova203 commented Jun 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kornilova203 commented Jun 18, 2018

kornilova203 commented Jun 18, 2018

kornilova203 commented Jun 18, 2018 • edited Loading

kornilova203 commented Jun 19, 2018

kornilova203 commented Jun 19, 2018

jonas commented Jun 20, 2018

jonas left a comment

Choose a reason for hiding this comment

jonas commented Jun 20, 2018

kornilova203 commented Jun 12, 2018 •

edited

Loading

kornilova203 commented Jun 17, 2018 •

edited

Loading

kornilova203 commented Jun 18, 2018 •

edited

Loading