-
Notifications
You must be signed in to change notification settings - Fork 0
Exploring API Refactoring with clang
clang exposes an API to the abstract syntax tree (AST) as used by the compiler, as well as an AST Visitor and Rewriter APIs. This is a pretty good start on the subject: Eli Bendersky’s intro into refactoring using clang .
This summarizes my experiences attempting to apply this tool refactoring parts of the HAL API. The experimental code is here .
The idea is laid out in issue 284 : change the thread function API from
typedef void(*hal_funct_t)(void * arg, long period);
to
struct threadargs {
long _period;
// other stuff potentially interesting to a thread function
};
typedef void(*hal_funct_t)(void * arg, const struct threadargs *ta);
A 'grep -r hal_export_funct hal|wc -l' yields 102 hits, so a manual reedit is a signifcant job.
Many thread functions rely on the 'period' parameter, so refactoring the signature needs to take this into account and inject the parameter at the beginning of the function body, eg like so:
long period = ta->_period;
Also, for any source file containing a thread function, both declarations and definition signatures need to be adapted.
-
parse the source file into an AST
-
use a AST Matcher to collect calls to 'hal_export_funct' and extract the name of the thread function (second argument)
-
collect those names in a map
-
do a second recursive pass over the AST with an recursive AST visitor visiting "interesting" nodes, in this case function declarations and definitions
-
use the Rewriter to modify, delete or change code as needed
-
by default the resulting output matches the input (proper comments, includes etc)
Sample program using the legacy API:
#include "hal.h"
static void efunct(void * arg, long period);
void efunct(void * arg, long period) {
int foo = period;
};
void efunct2(void * arg, long period) {};
void foo()
{
int rc;
rc = hal_export_funct("efunct", efunct, 0, 0,0,42);
rc = hal_export_funct("efunct2", efunct2, 0, 0,0,4711);
}running the refactor step, using a current machinekit/src directory in $MK:
mpass he.c -- -I$MK -I$MK/hal/lib -I$MK/rtapi -DRTAPI -D_GCC_LIMITS_H_ -I$MK/machinetalk/include -DTHREAD_FLAVOR_ID=1
/home/mah/tests/clang/he.c:5: detect hal_export_funct() of: efunct
/home/mah/tests/clang/he.c:9: detect hal_export_funct() of: efunct2
/home/mah/tests/clang/he.c:3: refactored 'efunct'
/home/mah/tests/clang/he.c:5: refactored 'efunct'
/home/mah/tests/clang/he.c:9: refactored 'efunct2'
#include "hal.h"
static void efunct(void * arg, const struct threadargs * ta);
void efunct(void * arg, const struct threadargs * ta) {
long period = ta->_period;
int foo = period;
};
void efunct2(void * arg, const struct threadargs * ta) {};
void foo()
{
int rc;
rc = hal_export_funct("efunct", efunct, 0, 0,0,42);
rc = hal_export_funct("efunct2", efunct2, 0, 0,0,4711);
}diff:
mah@nwheezy:~/tests/clang$ diff he.c he.o
2c2
< static void efunct(void * arg, long period);
---
> static void efunct(void * arg, const struct threadargs * ta);
4c4,5
< void efunct(void * arg, long period) {
---
> void efunct(void * arg, const struct threadargs * ta) {
> long period = ta->_period;
8c9
< void efunct2(void * arg, long period) {};
---
> void efunct2(void * arg, const struct threadargs * ta) {};
This is unfinished, and was just to explore the basic feasibility. The idea is:
-
investigate assignment statements in a recursive AST visit
-
select those which have an "interesting" result type, e.g. "hal_float_t" (doubles are not atomic on ARM < arm6)
-
wrap assignments into a setter function of the appropriate type (I skipped getters because the technique is the same, but we better specify exactly what to wrap how before going about it)
-
see the transformation code
Example:
hal_float_t f, *fp;
*fp = 3.14;
f = 47.11;set_hal_float(fp, 3.14);
set_hal_float(&f, 47.11);#include "stdio.h"
typedef double hal_float_t; // a comment
void foo( )
{
hal_float_t f, *fp;
double pi, *pip;
f = 123.0;
*fp = f;
*fp = f = 47.0;
pi = 3.1415;
*pip = pi;
*pip = pi = 3.1415;
}mah@nwheezy:~/tests/clang$ ~/src/llvm-clang-samples/build/mpass hf4.c -- -I$MK -I$MK/hal/lib -I$MK/rtapi -DRTAPI -D_GCC_LIMITS_H_ -I$MK/machinetalk/include -DTHREAD_FLAVOR_ID=1
/home/mah/tests/clang/hf4.c:9: refactored: 'f = 123.' to 'set_hal_float_t(&f, 123.)'
/home/mah/tests/clang/hf4.c:10: refactored: '*fp = f' to 'set_hal_float_t(fp, f)'
/home/mah/tests/clang/hf4.c:12: refactored: '*fp = f = 47.' to 'set_hal_float_t(fp, f = 47.)'
/home/mah/tests/clang/hf4.c:12: refactored: 'f = 47.' to 'set_hal_float_t(&f, 47.)'
#include "stdio.h"
typedef double hal_float_t; // a comment
void foo( )
{
hal_float_t f, *fp;
double pi, *pip;
set_hal_float_t(&f, 123.);
set_hal_float_t(fp, f);
set_hal_float_t(fp, set_hal_float_t(&f, 47.); // note missing rparen
pi = 3.1415;
*pip = pi;
*pip = pi = 3.1415;
}-
type detection works - note double ops untouched, hal_float_t ops rewritten
-
simple assignment statements are fine
-
recursive expansion (like needed for 'var1 = var2 = value') is flawed but can possibly fixed (Rewriter is challenged on recursive rewrites, or maybe just me)
-
overall the approach looks feasible, but needs an accurate rulebook what to transform how.
A recent "Refactoring" API is available to make simple refacoring tasks even easier. I rewrote the hal_export_funct fixer in terms of this simpler API - down to 170 lines C++.