libfirm / cparser

C99 parser and frontend for libfirm
http://pp.ipd.kit.edu/firm
GNU General Public License v2.0
336 stars 38 forks source link

compatibility with _Pragma #16

Closed nilput closed 5 years ago

nilput commented 5 years ago

I'm not totally sure if this is the correct place to add it, i mean it could be better if it was handled at the preprocessor, this could serve as a temporary hack though, it works in the few test cases i've tried.

i'm not sure about the string allocation, by comparing it to the "asm" statements, it seems no memory is explicitly deallocated?

Best regards.

MatzeB commented 5 years ago

This should better be handled in the preprocessor (it's even described in the C99 standard). So I think you would better try handling it like a function macro. You can probably do something similar to

    pp_definition_t *def = add_define_("_Pragma", true);

    symbol_t *const parameter_symbol = symbol_table_insert("directive");
    pp_definition_t *parameter = OALLOCZ(&pp_obstack, pp_definition_t);
    parameter->pos          = builtin_position;
    parameter->symbol       = parameter_symbol;
    parameter->is_parameter = true;
    parameter->is_variadic  = true;

    def->has_parameters = true;
    def->n_parameters   = 1;
    def->parameters     = parameter;

and then catch it when expansion is started. I also recommend writing a testcase or two for the firm-testsuite so you can try it behaves as expected when the pragma is put at various places in the program (between functions, as statement, between an expresison, expanded through a #define...)

nilput commented 5 years ago

@MatzeB i see, doing what you suggested and adding a new predefined macro works.

about the tests, wouldn't it be a good idea to have a directory for cparser's tests? they would ideally run after building cparser itself and have input files and expected results in the outputs. many tests would need only simple shell scripts like:

./build/debug/cparser -o "$test_result" -E "$test_input" 2>"$test_stderr"
grep 'Wunknown-pragmas' "$test_stderr" && exit 0
exit 1

even results expected out of a compiled input can be reduced to something a shell script can verify.

MatzeB commented 5 years ago

We already have cparser tests, they just happen to be in a different repository: https://github.com/libfirm/firm-testsuite/tree/master/C

MatzeB commented 5 years ago

in this case actually https://github.com/libfirm/firm-testsuite/tree/master/preproctest

nilput commented 5 years ago

oh, i should've noticed! i updated the commit and opened a pull request there. i wanted to support the thing instead of just making sure it compiles but i'm new to the codebase, it would be great if there was a way to have a virtual "file" as a source of tokens in the preprocessor, and instead of having some of the code call next_preprocessing_token() or next_input_token() (what parse_pragma_directive() uses) there would be just a single interface, although such a change would require a lot of other changes i believe.

nilput commented 5 years ago

used git diff this time, but it's too late all spaces have been spotted.

rofl0r commented 5 years ago

i think this hasn't been merged so far due to https://github.com/libfirm/cparser/pull/16#discussion_r227626747 . there's no good reason to have unrelated changes in the same commit.