487918 - porky: tracking

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Coding a pork rewrite tool is a bit time consuming, because one needs to learn (at the very least) the pork "stack" and the elsa AST.  Pork aims to make this easier; one should only have to learn the (hopefully) much simpler porky syntax.  Porky does this by only supporting a vastly reduced set of possible rewrites that can be done with pork.

Porky is implemented as a simple porky-source to pork-source (C++) compiler.  Basically, the state of the art of porky at the moment is implementing the rewrite program

rewrite SwapArgs {
  type PRLock* => mozilla::Mutex*

  call PR_Lock(g, lock) => lock->Lock(g)
  call mozilla::Mutex::Lock(this, g) => PR_Lock(g, this)
}

which means
  * create the rewriter |SwapArgs|
  * globally change the type |PRLock*| into |mozilla::Mutex*|
  * at each callsite matching |PR_Lock(*1, *2)|, rewrite it into |*2->Lock(*1)|
  * at each callsite matching |mozilla::Mutex::Lock(*1, *2)|, rewrite it into |PR_Lock(*2, *1)|

Note that the current state of porky allows |Foo(f, b)| to be rewritten to |f->Foo(b)| and vice versa.  This is as almost as much as is necessary to solve the problem that motivated the creation of porky.

There are many ways to extend porky, and they should be tracked here.  The most immediately important change is making it sanely integrate into the piglet build system.  This is blocked on bug 440068.

Ideas on extending porky will probably follow in this bug.  Right now, it's sleep time in the Pacific time zone.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: porky prototype

Attached for backup purposes.  Not likely to be terribly usable for the general mozilla public.

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Porky 0.1

After a few hours' hacking, I got porky to handle the rewrite that inspired its creation (bug 486606).  Ready to spit out a patch.

Still waiting on the pork build system to stabilize before asking for review.  There are a few other niceties porky should support, like automatically invoking pork-barrel.py, understanding file filters (--exclude 'nspr/*'), and handling simple rewrite specs passed on the command line (|porky -e 'type Foo => Bar'|).  These are easy, though.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: porky 0.0.1, RC1

Seem me on IRC (cjones) if you want a walkthrough.

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I should add that porky expects by default to run in this directory structure
  /
    piglet/
    porky/
but all it really needs to know is where |piglet/| is.  This can be communicated through the |PIGLET_DIR| environment variable.

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: v 0.0.1, python 2.4 compatible

Comment 6

[(dormant account)]

First I'd like to say this is extremely impressive. I couldn't imagine building something this nice on top of Pork until I saw the code.

Things that I'd like to see cleaned up
* Break up parsing/astgen/cmdline+running into separate modules. set python path to script location to avoid getting annoyed

* No temporary directory business, output directory should be based on rewrite filename and spit out to the cwd. Need minimal error handling for that(maybe a big try statement look for filesystem exceptions)

* Support for multi-stage operation. Either spit out a makefile that can do the individual steps or like setup.py

* I prefer human-readable commandline options. porky isn't something as frequently used as ls, one can take a few seconds to write out the arguments, but it makes porky usage a little more self-documenting

* Past experience has shown that it's convenient to have -j stuff default to the number of cpus

Comment 7

[(dormant account)]

forgot to add
* Need a configure that takes care of stuff like 
    if not (os.path.isdir(pigletdir)
            and os.path.isfile(config_mk)
            and os.path.isfile(build_mk)):

Comment 8

[(dormant account)]

Some notes on generated code:
varName = (const D_name*) idecl;
* Don't do C/C++ casts, use Elsa's -asFoo(), they do more error checking

* I think it's better to structure the generated code as an #include and just copy over(or even symlink) the handwritten portion

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: porky v 0.0.1, comments addressed

The core of porky now expects to live under piglet/, although this is still configurable.  Run piglet/porky/configure.  In the future, once pork/ has been properly reorganized, this configure script can be invoked automatically by pork/configure.

Using porky:

Step 1: invoke 'piglet/porkyc' on the rewrite spec.  It now doesn't matter where this command is run from; every successive step has its PYTHONPATH automagically set, if necessary.  This spits out code in './[Rewriter].code'.

Step 2: compile the rewriter.  |make -C ./[Rewriter].code| will do the trick.

Step 3: run the rewriter.  porky generates a convenience script './[Rewriter].code/dorewrite'; this is easiest to invoke.  Just |./[Rewriter].code/dorewrite FILE_PATS -x EXCLUDE_PATS| should work most of the time.  This spits out a patch to stdout, but you can override this with '-o ...'.

Comment 10

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: porky v 0.0.1, compiles code by default

Whoops, forgot to make porkyc compile by default.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I should add that Step 2 in comment 9 is not necessary by default.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 373197 [details] [diff] [review]
porky v 0.0.1, compiles code by default

I've fixed a few bugs since this was submitted, and managing all its dependencies is getting complicated.  Delaying review until the pork reorg.

Comment 13

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: minor configure changes to eventually support porky

Comment 14

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: porky v0.0.1

Comment 15

[(dormant account)]

Comment on attachment 383832 [details] [diff] [review]
minor configure changes to eventually support porky


>@@ -1,5 +1,3 @@
>-PIGLET_DIR = $(PWD)
>-
> ELSA_INCDIRS := -I$(ELSA_DIR)/smbase -I$(ELSA_DIR)/elkhound -I$(ELSA_DIR)/ast -I$(ELSA_DIR)
> ELSA_INCFLAGS := -D__UNIX__ -DUSE_SERIAL_NUMBERS=1 -DNDEBUG -D__LINUX__ $(ELSA_INCDIRS)
> 
>@@ -7,3 +5,8 @@ PIGLET_INCDIRS := -I$(PIGLET_DIR)
> PIGLET_INCFLAGS := -I$(PIGLET_DIR)
> 
> CXXFLAGS += $(ELSA_INCFLAGS) $(PIGLET_INCFLAGS)
>+
>+PIGLET_EXPORTS = piglet expr_visitor patcher
>+PIGLET_OBJS =								\
>+	$(addprefix $(PIGLET_DIR)/, $(addsuffix .o, $(PIGLET_EXPORTS)))	\
>+	$(ELSA_DIR)/libelsa.a

I don't like this piglet_exports stuff. just archive the piglet objects into libpiglet.a. Can stick contents of libelsa.a into there too.

Comment 16

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: address comments, build libpork.a

Comment 17

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: porky v0.0.1, updated to new build system

Comment 18

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: porky 0.0.1, additional tweaks

Comment 19

[(dormant account)]

Comment on attachment 389832 [details] [diff] [review]
address comments, build libpork.a

>diff --git a/Makefile.in b/Makefile.in
>--- a/Makefile.in
>+++ b/Makefile.in
>@@ -1,22 +1,28 @@
> # Piglet makefile
> # Builds Elsa apps without Oink!
> 
> include config.mk
> include build.mk
> 
>+PORK_SRCS = expr_visitor piglet patcher
>+PORK_OBJS = $(addsuffix .o, $(PORK_SRCS))
>+
> all: elsa_all stopwatch
> 
> $(ELSA_DIR)/Makefile:
> 	cd $(ELSA_DIR) && ./configure
> 
> elsa_all: $(ELSA_DIR)/Makefile
> 	$(MAKE) -C $(ELSA_DIR)
> 
>-stopwatch: stopwatch.o piglet.o expr_visitor.o patcher.o $(ELSA_DIR)/libelsa.a
>+stopwatch: stopwatch.o $(LIBPORK)
> 	g++ -o $@ $+
> 
> piglet.o: piglet.h
>-main.o: piglet.h expr_visitor.h
>+
>+$(LIBPORK): elsa_all $(PORK_OBJS)
>+	cp $(LIBELSA) $@
>+	ar rs $@ $(PORK_OBJS)
> 
> clean:
>-	rm -f stopwatch *.o *~
>+	rm -f $(LIBPORK) stopwatch *.o *~
>diff --git a/build.mk b/build.mk
>--- a/build.mk
>+++ b/build.mk
>@@ -1,9 +1,10 @@
>-PIGLET_DIR = $(PWD)
>-
> ELSA_INCDIRS := -I$(ELSA_DIR)/smbase -I$(ELSA_DIR)/elkhound -I$(ELSA_DIR)/ast -I$(ELSA_DIR)
> ELSA_INCFLAGS := -D__UNIX__ -DUSE_SERIAL_NUMBERS=1 -DNDEBUG -D__LINUX__ $(ELSA_INCDIRS)
>+LIBELSA := $(ELSA_DIR)/libelsa.a
> 
> PIGLET_INCDIRS := -I$(PIGLET_DIR)
> PIGLET_INCFLAGS := -I$(PIGLET_DIR)
> 
> CXXFLAGS += $(ELSA_INCFLAGS) $(PIGLET_INCFLAGS)
>+
>+LIBPORK := $(PIGLET_DIR)/libpork.a
>diff --git a/configure b/configure
>--- a/configure
>+++ b/configure
>@@ -52,23 +52,29 @@ if __name__ == "__main__":
>             usage()
>             exit(0)
>         elif o in ('--CXX',):
>             cxx = val
>         elif o in ('--CXXFLAGS',):
>             cxxflags = val
>         else:
>             error("unhandled option " +  o)
>+            usage()
>+            sys.exit(2)
> 
>     elsa_dir = checkdir(elsa_dir, "elsa")
>     if not elsa_dir:
>         error("""check out elsa with: hg clone http://hg.mozilla.org/elsa""")
>         exit(1)
>+
>+    piglet_dir = os.path.dirname(os.path.abspath(sys.argv[0]))
>+
>     config = dict(REWRITING_CXX=rewriting_cxx,
>                   ELSA_DIR=elsa_dir,
>+                  PIGLET_DIR=piglet_dir,
>                   CXX=cxx,
>                   CXXFLAGS=cxxflags)
>     f = open("config.mk", "w")
>     f.write("""# %s
> %s
> """ % (" ".join(["'" + a + "'" for a in sys.argv]),
>        "\n".join([k + "=" + v for k, v in config.iteritems()])))
>     f.close()
>diff --git a/test/Makefile b/test/Makefile
>--- a/test/Makefile
>+++ b/test/Makefile
>@@ -1,29 +1,21 @@
> # Piglet makefile
> #
> # Builds an Elsa app without Oink!
> 
>-PIGLET_DIR := ../
>+topdir = ..
> 
>-include $(PIGLET_DIR)/config.mk
>-
>-ELSA_INCDIRS := -I$(PORK_DIR)/smbase -I$(PORK_DIR)/elkhound -I$(PORK_DIR)/ast -I$(PORK_DIR)/elsa -I$(PIGLET_DIR)
>-ELSA_INCFLAGS := -D__UNIX__ -DUSE_SERIAL_NUMBERS=1 -DNDEBUG -D__LINUX__ $(ELSA_INCDIRS)
>-
>-CXXFLAGS := -g -Wall $(ELSA_INCFLAGS)
>-PIGLET_OBJS := $(addprefix $(PIGLET_DIR), $(addsuffix .o,piglet expr_visitor patcher))
>-LIBS = $(PORK_DIR)/elsa/libelsa.a
>-
>-OBJDEPS = $(PIGLET_OBJS) $(LIBS)
>+include $(topdir)/config.mk
>+include $(topdir)/build.mk
> 
> .PHONY: check clean
> 
> check: checkpatcher
> 
> checkpatcher: patchertests
> 	./patchertests
> 
>-patchertests: patchertests.o $(OBJDEPS)
>+patchertests: patchertests.o $(PIGLET_OBJS)
> 	g++ -o $@ $+;
> 
> clean:
> 	-rm *.o

Comment 20

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Pushed
6c4e86b1502e Chris Jones - bug 487918: build libpork.a, couple of other tweaks in preparation for porky.py
3c39899f0c9b Chris Jones - bug 487918: porky 0.0.1

Leaving open for tracking.