www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Formal review of dtoh

reply Jacob Carlborg <doob me.com> writes:
This is the formal review of Adam D. Ruppe's tool "dtoh" for inclusion 
in the tools repository [1].

Dtoh is a tool used to convert D modules to C/C++ headers. This allows 
to use D libraries in C/C++ code.

This review might be a bit special since this is the first time a tool 
is reviewed. Since this is a review of a tool the standard guidelines 
for reviewing might not apply. For example, we might require that the 
tool should have documentation like DMD does [3].

There's already a pull request with some discussion [2]. Note, the pull 
request has already been merged but we would like to do a review anyway.

Code: https://github.com/adamdruppe/tools/blob/dtoh/dtoh.d

The review will last for two weeks, ending on April 8.

[1] https://github.com/D-Programming-Language/tools
[2] https://github.com/D-Programming-Language/tools/pull/39
[3] http://dlang.org/dmd-osx.html

-- 
/Jacob Carlborg
Mar 25 2014
next sibling parent reply "Andrea Fontana" <nospam example.com> writes:
On Tuesday, 25 March 2014 at 20:38:40 UTC, Jacob Carlborg wrote:

 This is the formal review of Adam D. Ruppe's tool "dtoh" for 
 inclusion in the tools repository [1].

 Dtoh is a tool used to convert D modules to C/C++ headers. This 
 allows to use D libraries in C/C++ code.

 This review might be a bit special since this is the first time 
 a tool is reviewed. Since this is a review of a tool the 
 standard guidelines for reviewing might not apply. For example, 
 we might require that the tool should have documentation like 
 DMD does [3].

 There's already a pull request with some discussion [2]. Note, 
 the pull request has already been merged but we would like to 
 do a review anyway.

 Code: https://github.com/adamdruppe/tools/blob/dtoh/dtoh.d

 The review will last for two weeks, ending on April 8.

 [1] https://github.com/D-Programming-Language/tools
 [2] https://github.com/D-Programming-Language/tools/pull/39
 [3] http://dlang.org/dmd-osx.html


At least it shouldn't give error if called without any params but 
give some info :)

Is there any usage example?
Mar 26 2014
next sibling parent Jacob Carlborg <doob me.com> writes:
On 2014-03-26 16:33, Andrea Fontana wrote:


 At least it shouldn't give error if called without any params but give
 some info :)

 Is there any usage example?


Not that I have seen. Perhaps we should require some documentation.

-- 
/Jacob Carlborg
Mar 26 2014
prev sibling parent reply "Adam D. Ruppe" <destructionator gmail.com> writes:
On Wednesday, 26 March 2014 at 15:33:47 UTC, Andrea Fontana wrote:

 Is there any usage example?


You make the json with dmd -X yourfile.d yourotherfiles.d

Then run the json through the thingy with ./dtoh yourfile.json

It'll make .h files for the extern(C) and extern(C++) pieces of 
the D files which should work to bring it into your other project.



What I really want it someone to try it in a vaguely real-world 
situation and see if it is helpful but could be better, too 
limited to be useful, or buggy or what. There's some things I 
think could be done better if dmd output more information, but I 
think this is somewhat useful as it is now and just needs some 
kind of serious test beyond my proof of concept file.

That's what I was hoping it could be pulled, with the note that 
it is super experimental, so maybe people will try to use it and 
file some bugs as to what really sucks. The whole approach might 
need to be abandoned in favor of a new dmd switch or something 
too. That's also why the style doesn't match and stuff like that; 
if it needs a rewrite anyway, no point getting worked up over 
brace placement. (It did need to be mostly rewritten a few months 
ago because dmd changed the json output!)
Mar 27 2014
parent reply "Dicebot" <public dicebot.lv> writes:
On Thursday, 27 March 2014 at 13:25:14 UTC, Adam D. Ruppe wrote:

 That's what I was hoping it could be pulled, with the note that 
 it is super experimental, so maybe people will try to use it 
 and file some bugs as to what really sucks. The whole approach 
 might need to be abandoned in favor of a new dmd switch or 
 something too. That's also why the style doesn't match and 
 stuff like that; if it needs a rewrite anyway, no point getting 
 worked up over brace placement. (It did need to be mostly 
 rewritten a few months ago because dmd changed the json output!)


You still should provide better CLI, it does not rely on internal 
implementation and will make easier for random reviewers to test 
/ experiment.
Mar 27 2014
parent "Dicebot" <public dicebot.lv> writes:
On Thursday, 27 March 2014 at 18:28:28 UTC, Dicebot wrote:

 On Thursday, 27 March 2014 at 13:25:14 UTC, Adam D. Ruppe wrote:

 That's what I was hoping it could be pulled, with the note 
 that it is super experimental, so maybe people will try to use 
 it and file some bugs as to what really sucks. The whole 
 approach might need to be abandoned in favor of a new dmd 
 switch or something too. That's also why the style doesn't 
 match and stuff like that; if it needs a rewrite anyway, no 
 point getting worked up over brace placement. (It did need to 
 be mostly rewritten a few months ago because dmd changed the 
 json output!)


 You still should provide better CLI, it does not rely on 
 internal implementation and will make easier for random 
 reviewers to test / experiment.


Want to extend this point a bit more : I believe that tools 
contributions should be evaluated based on a principle similar to 
Phobos modules - implementation may have flaws (or even changed 
completely) but user API (CLI + behavior for tools) should remain 
solid once merged. So it needs better attention even if you are 
going to re-write rest of the program later.

I'd consider it a blocker if this to undergo formal voting.
Apr 04 2014
prev sibling next sibling parent reply "Dicebot" <public dicebot.lv> writes:
Right now it does not really look in shape for a formal review. 
Documentation is missing. Tool itself does not have help output 
and throws exception on plain "./dtoh" call. I see quite a lot of 
"FIXME" and "idea" comments in its source code. Missing internal 
DDOC comments at least for basic structures. In general it looks 
more like proof of concept than actual proposal.

Concept itself looks reasonable though. Eventually I'd like to 
see it use dmd frontend as a library but parsing JSON output 
should do fine until such option becomes feasible.

I don't really want to dwell deep into implementation right now 
because I think that before any further exploration can happen, 
tool must be made more friendly to collaborative work.
Mar 27 2014
parent Jacob Carlborg <doob me.com> writes:
On 2014-03-27 12:27, Dicebot wrote:

 Right now it does not really look in shape for a formal review.
 Documentation is missing. Tool itself does not have help output and
 throws exception on plain "./dtoh" call. I see quite a lot of "FIXME"
 and "idea" comments in its source code. Missing internal DDOC comments
 at least for basic structures. In general it looks more like proof of
 concept than actual proposal.

 Concept itself looks reasonable though. Eventually I'd like to see it
 use dmd frontend as a library but parsing JSON output should do fine
 until such option becomes feasible.

 I don't really want to dwell deep into implementation right now because
 I think that before any further exploration can happen, tool must be
 made more friendly to collaborative work.


I fully agree with this.

-- 
/Jacob Carlborg
Apr 08 2014
prev sibling next sibling parent "Chris Williams" <yoreanon-chrisw yahoo.co.jp> writes:
On Tuesday, 25 March 2014 at 20:38:40 UTC, Jacob Carlborg wrote:

 Code: https://github.com/adamdruppe/tools/blob/dtoh/dtoh.d


The author might consider using an associative array of functions 
to handle the various keywords, rather than switches.

I would suggest adding a "jsonutils.d" or somesuch, to break out 
the re-usable bits from the task-specific ones.

"scope (exit)" should be used to add the #endif to the end of the 
.h files. Also, there should be a comment with the guard name 
after it. (May as well try and create a nice-looking .h file).
Mar 27 2014
prev sibling next sibling parent Jacob Carlborg <doob me.com> writes:
On 2014-03-25 21:38, Jacob Carlborg wrote:

 This is the formal review of Adam D. Ruppe's tool "dtoh" for inclusion
 in the tools repository [1].


The formal review of "dtoh" has now ended. I won't continue with voting 
since this the tool got very few reviews and in general doesn't feel ready.

-- 
/Jacob Carlborg
Apr 08 2014
prev sibling parent reply "John Colvin" <john.loughran.colvin gmail.com> writes:
On Tuesday, 25 March 2014 at 20:38:40 UTC, Jacob Carlborg wrote:

 This is the formal review of Adam D. Ruppe's tool "dtoh" for 
 inclusion in the tools repository [1].

 Dtoh is a tool used to convert D modules to C/C++ headers. This 
 allows to use D libraries in C/C++ code.

 This review might be a bit special since this is the first time 
 a tool is reviewed. Since this is a review of a tool the 
 standard guidelines for reviewing might not apply. For example, 
 we might require that the tool should have documentation like 
 DMD does [3].

 There's already a pull request with some discussion [2]. Note, 
 the pull request has already been merged but we would like to 
 do a review anyway.

 Code: https://github.com/adamdruppe/tools/blob/dtoh/dtoh.d

 The review will last for two weeks, ending on April 8.

 [1] https://github.com/D-Programming-Language/tools
 [2] https://github.com/D-Programming-Language/tools/pull/39
 [3] http://dlang.org/dmd-osx.html


Adam, have you considered sprucing this up for review?

There are so many things out there with c++ extension 
architectures that, with this tool in a solid state and more 
widely distributed, could trivially take D extensions too.
Mar 19 2015
parent reply "Adam D. Ruppe" <destructionator gmail.com> writes:
On Thursday, 19 March 2015 at 11:51:59 UTC, John Colvin wrote:

 Adam, have you considered sprucing this up for review?


Not possible right now, my schedule is already overloaded (I'm 
working two full-time jobs in addition to a variety of little 
things).

I'm also not really up with all the recent changes to 
extern(C++), using them would probably be really useful too but I 
don't know what all it does anymore.
Mar 24 2015
parent "John Colvin" <john.loughran.colvin gmail.com> writes:
On Tuesday, 24 March 2015 at 18:48:11 UTC, Adam D. Ruppe wrote:

 On Thursday, 19 March 2015 at 11:51:59 UTC, John Colvin wrote:

 Adam, have you considered sprucing this up for review?


 Not possible right now, my schedule is already overloaded (I'm 
 working two full-time jobs in addition to a variety of little 
 things).

 I'm also not really up with all the recent changes to 
 extern(C++), using them would probably be really useful too but 
 I don't know what all it does anymore.


Sure thing, just wanted to make sure this wasn't forgotten.
Mar 24 2015