Page MenuHomePhabricator

[analyzer][PlistMacroExpansion] Part 5.: Support for # and ##
ClosedPublic

Authored by Szelethus on Oct 8 2018, 9:17 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

Szelethus created this revision.Oct 8 2018, 9:17 AM

This patch didn't really work, sometimes it printed extra spaces, sometimes printed the hash tokens, and so on, so I refactored the whole project to deal with this issue almost out of the box.

Looks good.

What happens if the macro is to stringify a partially string argument?

#define BOOYAH(x) #x ";

... 

std::string foo = BOOYAH(blabla)
std::string foo2 = BOOYAH("blabla)
int x = 2;

Not sure if these cases are even valid C(XX), but if they are, we should test.

An idea for a follow-up patch if it's not that hard work: you mentioned your original approach with that madness in the HTML printer. Perhaps it could be refactored to use this implementation too and thus we'll only have 9 places where macro expansion logic is to be maintained, not 10. 😈

I verified this project on tmux, which uses the preprocessor very heavily. It works perfectly, and doesn't crash anywhere despite the very liberal use of asserts.

Looks good.

What happens if the macro is to stringify a partially string argument?

#define BOOYAH(x) #x ";

... 

std::string foo = BOOYAH(blabla)
std::string foo2 = BOOYAH("blabla)
int x = 2;

Not sure if these cases are even valid C(XX), but if they are, we should test.

Lucky, this spawn of a nightmare doesn't compile.

An idea for a follow-up patch if it's not that hard work: you mentioned your original approach with that madness in the HTML printer. Perhaps it could be refactored to use this implementation too and thus we'll only have 9 places where macro expansion logic is to be maintained, not 10. 😈

The HTML output contains the macro expansions for all macros in the file, so it's justifiable that entire file is lexed and preprocessed. Granted, using const_cast and the like (there are some next level hacks in that implementation) is risky, but as long as it doesn't break, it does it's job better then this solution would.

As long as it doesn't break. If you generate a HTML output, the report generation speed may not be the greatest concern, so I'll definitely think about this a little bit.

Szelethus updated this revision to Diff 170073.Oct 18 2018, 6:05 AM

Fixed an issue where if ## had spaces around it, those spaces weren't removed.

NoQ accepted this revision.Nov 1 2018, 7:32 PM

Yay.

This revision is now accepted and ready to land.Nov 1 2018, 7:32 PM
xazax.hun accepted this revision.Nov 2 2018, 4:58 AM
This revision was automatically updated to reflect the committed changes.