0

does anyone know why this is syntactically wrong? Im trying to covert this

    #define OUTS_FROM_FP(_fp, _argCount) ((u4*) ((u1*)SAVEAREA_FROM_FP(_fp) - sizeof(u4) * (_argCount)))

to this

    #define OUTS_FROM_FP(_fp, _argCount) {\
    ((u4*) ((u1*)SAVEAREA_FROM_FP(_fp) - sizeof(u4) * (_argCount))); \
    cout<<"Hello World"<<endl;  \
    }

    outs = OUTS_FROM_FP(fp, vsrc1); --this is how it is being called

I get a lot of errors when running this: they start from statements that say that variables that were passed to the macro before are unused

varenik_aa
  • 27
  • 2
  • Why are you even using a macro instead of a function? – cdhowie Apr 04 '17 at 14:41
  • this is a snippet from Dalvik VM. Im trying to modify it – varenik_aa Apr 04 '17 at 14:43
  • Could you give an example of where the macro is used? I *think* it’s either that your compiler doesn’t support expression blocks, or that the last line of the block is a print statement instead of the expression in the original, but either way it would help if you gave an example of where you were using it and what the error was. – Daniel H Apr 04 '17 at 14:44
  • outs = OUTS_FROM_FP(fp, vsrc1); --this is how it is being called – varenik_aa Apr 04 '17 at 14:45
  • Can you edit the question with that, as well as what type `outs` is and what error you’re seeing? – Daniel H Apr 04 '17 at 14:46
  • Do the text replacement on your own and see if it something meaningful. I think `outs = {((u4*) ((u1*)SAVEAREA_FROM_FP(fp) - sizeof(u4) * (vsrc1))); cout<<"Hello World"< – mch Apr 04 '17 at 14:48
  • In order to answer this question we need to know what error you are getting, and how OUTS_FROM_FP is used. – Jack Aidley Apr 04 '17 at 14:52
  • If the compiler is gcc you can use the -E flag to output the preprocessed output without compiling, then you can see what the macro expands to. Other compilers will probably have the equivalent. – Jimbo Apr 04 '17 at 14:55
  • What compile error do you get? What compiler are you using? – Jimbo Apr 04 '17 at 14:56
  • updated. and I think it uses regular gcc – varenik_aa Apr 04 '17 at 15:01
  • The reason the compiler says the variables are unused is that it’s confused about the macro. What error(s) are you getting which reference that line specifically? – Daniel H Apr 04 '17 at 15:01
  • somehow it doesnt give me an error specific to that particular line – varenik_aa Apr 04 '17 at 15:06
  • By “that particular line”, I mean the line where you call the macro, not the line where you define the macro. – Daniel H Apr 04 '17 at 15:07
  • Also, how was `outs` declared? Is it a `u4*`, an `auto` with some other declaration, or what? – Daniel H Apr 04 '17 at 15:08
  • for example, dalvik/vm/Init.cpp: In function 'bool dvmInitAfterZygote()': dalvik/vm/Init.cpp:1740:8: warning: variable 'startHeap' set but not used [-Wunused-but-set-variable] ... – varenik_aa Apr 04 '17 at 15:10
  • u4 is an unsigned int – varenik_aa Apr 04 '17 at 15:21

2 Answers2

1

Expanded, the original macro will look like this:

outs = ((u4*) ((u1*)SAVEAREA_FROM_FP(fp) - sizeof(u4) * (vsrc)));

That's (as far as I can tell as you didn't provide much context) valid code.

Your modified macro expands the same statement to this:

outs = { /* ... */ };

Your compiler gets all kinds of confused as you are attempting to assign a code block to a variable...

All the usual caveats regarding the use of macros in general aside, you could use the comma operator to get your modified macro "working":

#define OUTS_FROM_FP( _fp, _argCount ) \
    cout << "Hello world\n", \
    ((u4*) ((u1*)SAVEAREA_FROM_FP(_fp) - sizeof(u4) * (_argCount)))

(The output is put first, as statements separated by the comma operator evaluate to the result of the last statement -- putting the output first makes the macro still evaluate to the same value as the original macro.)

All in all, you're probably better off turning that macro into a function.

DevSolar
  • 67,862
  • 21
  • 134
  • 209
0

Assuming that _fp and _argCount are variables or simple expressions, the original version is an expression of type u4*.

The second is more complicated. The braces make it a block, but syntactically you’re using it as an expression. This is not allowed in the C++ standard, but is supported by g++ and some other compilers. Since you say you’re using GCC, the value of this expression is the value of the last line of the block, which in this case is cout<<"Hello World"<<endl. If you were using a compiler which did not support statement expressions, you’d get a more confused syntax error.

I expect that unless you can convert an ostream to a u4 pointer (which, given what context we have, seems very unlikely), this won’t work. In this simple case, you can fix it by simply switching the order of the lines in the block. In a more complicated case, which I expect is the end goal, you probably would need to do something like

#define OUTS_FROM_FP(_fp, _argCount) {\
    u4* result = ((u4*) ((u1*)SAVEAREA_FROM_FP(_fp) - sizeof(u4) * (_argCount))); \
    cout<<"Hello World"<<endl;  \
    result; \
}

This saves the output of the macro to a temporary variable, does whatever calculations you want (which can change result), and then on the last line “returns” result outside the macro. This is less portable than DevSolar’s solution, but it works better if you need to create temporary variables, and in my opinion is more readable.

However, as others point out in the comments, there is little reason (at least that we can see) to keep this as a macro instead of converting it to a function. Functions are much more robust in a variety of ways. Reasons you might still want to keep it as a macro include the definition of SAVEAREA_FROM_FP changing or the types u4 and u1 being different in different places. Neither of these would not be good programming practice, but you can’t control what others have done before and I don’t know enough about Dalvik to say it isn’t the case.

Community
  • 1
  • 1
Daniel H
  • 7,223
  • 2
  • 26
  • 41