[cfe-dev] [PATCH] preprocessor pragma push_macro support
Daniel Dunbar
daniel.dunbar at gmail.com
Tue Jul 27 10:08:25 PDT 2010
On Jul 26, 2010, at 22:38, Michael Spencer <bigcheesegs at gmail.com> wrote:
> Overall the patch looks good. HandlePragmaPushMacro and
> HandlePragmaPopMacro seem to share a lot of code and you may consider
> refactoring the parsing and lookup into another function. There are
> some things that I'm not sure about and will have to be checked by
> others.
>
> It passes the test I wrote while going through the MS extensions. I've
> attached that test which you should add to your patch.
>
> One other thing is that clang currently warns about macro
> redefinitions after a push. It would be nice if it didn't on the first
> redefinition after a push, as you pushed it so you could redefine it
> :P.
>
> Now for some inline comments.
>
>> Index: include/clang/Basic/DiagnosticLexKinds.td
>> ===================================================================
>> --- include/clang/Basic/DiagnosticLexKinds.td (revision 109329)
>> +++ include/clang/Basic/DiagnosticLexKinds.td (working copy)
>> @@ -230,6 +230,12 @@
>> "pragma comment requires parenthesized identifier and optional string">;
>> def err_pragma_message_malformed : Error<
>> "pragma message requires parenthesized string">;
>> +def err_pragma_push_macro_malformed : Error<
>> + "pragma push_macro requires a parenthesized string">;
>> +def err_pragma_pop_macro_malformed : Error<
>> + "pragma pop_macro requires a parenthesized string">;
>> +def warn_pragma_pop_macro_no_push : Warning<
>> + "#pragma pop_macro : '%0' no previous #pragma push_macro for this identifier">;
>
> This may be better worded as "pragma pop_macro could not pop '%0', no
> matching push_macro", but that's really up to personal preference and
> whatnot. I just matched what warn_pragma_diagnostic_clang_cannot_ppp
> says for consistency.
>
>> def warn_pragma_message : Warning<"%0">;
>> def warn_pragma_ignored : Warning<"unknown pragma ignored">,
>> InGroup<UnknownPragmas>, DefaultIgnore;
>> Index: lib/Lex/Pragma.cpp
>> ===================================================================
>> --- lib/Lex/Pragma.cpp (revision 109329)
>> +++ lib/Lex/Pragma.cpp (working copy)
>> @@ -16,6 +16,7 @@
>> #include "clang/Lex/HeaderSearch.h"
>> #include "clang/Lex/LiteralSupport.h"
>> #include "clang/Lex/Preprocessor.h"
>> +#include "clang/Lex/MacroInfo.h"
>> #include "clang/Lex/LexDiagnostic.h"
>> #include "clang/Basic/FileManager.h"
>> #include "clang/Basic/SourceManager.h"
>> @@ -483,7 +484,131 @@
>> Callbacks->PragmaMessage(MessageLoc, MessageString);
>> }
>>
>> +/// HandlePragmaPushMacro - Handle #pragma push_macro.
>> +/// The syntax is:
>> +/// #pragma push_macro("macro")
>> +void Preprocessor::HandlePragmaPushMacro(Token &PushMacroTok) {
>> + // Remember the pragma token location.
>> + SourceLocation PragmaLoc = PushMacroTok.getLocation();
>>
>> + // Read the '('.
>> + Token Tok;
>> + Lex(Tok);
>> + if (Tok.isNot(tok::l_paren)) {
>> + Diag(PragmaLoc, diag::err_pragma_push_macro_malformed);
>> + return;
>> + }
>> +
>> + // Read the macro name string.
>> + Lex(Tok);
>> + if (Tok.isNot(tok::string_literal)) {
>> + Diag(PragmaLoc, diag::err_pragma_push_macro_malformed);
>> + return;
>> + }
>> +
>> + // Remember the macro string.
>> + std::string StrVal = getSpelling(Tok);
>
> This should be a llvm::StringRef instead of an std::string.
>
>> +
>> + // Read the ')'.
>> + Lex(Tok);
>> + if (Tok.isNot(tok::r_paren)) {
>> + Diag(PragmaLoc, diag::err_pragma_push_macro_malformed);
>> + return;
>> + }
>> +
>> + assert(StrVal[0] == '"' && StrVal[StrVal.size()-1] == '"' &&
>> + "Invalid string token!");
>
> This should be an actual diagnostic instead of an assert. Input
> provided by the user should _never_ cause an assert.
>
>> +
>> + // Create a Token from the string.
>> + Token MacroToPushTok;
>> + MacroToPushTok.startToken();
>> + MacroToPushTok.setKind(tok::identifier);
>> + CreateString(&StrVal[1], StrVal.size() - 2, MacroToPushTok);
>> +
>> + // Get the IdentifierInfo of MacroToPushTok.
>> + IdentifierInfo *IdentInfo = LookUpIdentifierInfo(MacroToPushTok);
>> +
>> + // Get the MacroInfo and flag IsPragmaPush.
>> + MacroInfo *MI = getMacroInfo(IdentInfo);
>> + if (MI) {
>> + MI->setIsPragmaPush(true);
>> + }
>> +
>> + // Push the MacroInfo pointer so we can retrieve in later.
>> + PragmaPushMacroInfo[IdentInfo].push(MI);
>> +}
>> +
>> +/// HandlePragmaPopMacro - Handle #pragma push_macro.
>> +/// The syntax is:
>> +/// #pragma pop_macro("macro")
>> +void Preprocessor::HandlePragmaPopMacro(Token &PopMacroTok) {
>> + SourceLocation PragmaLoc = PopMacroTok.getLocation();
>> + Token Tok;
>> +
>> + // Read the '('.
>> + Lex(Tok);
>> + if (Tok.isNot(tok::l_paren)) {
>> + Diag(PragmaLoc, diag::err_pragma_pop_macro_malformed);
>> + return;
>> + }
>> +
>> + // Read the '"..."'.
>> + Lex(Tok);
>> + if (Tok.isNot(tok::string_literal)) {
>> + Diag(PragmaLoc, diag::err_pragma_pop_macro_malformed);
>> + return;
>> + }
>> +
>> + // Remember the string.
>> + std::string StrVal = getSpelling(Tok);
>
> This should be a llvm::StringRef instead of an std::string.
Are you sure? getSpelling returns a new string, I think, you can't
keep a reference to it.
- Daniel
>
>> +
>> + // Read the ')'.
>> + Lex(Tok);
>> + if (Tok.isNot(tok::r_paren)) {
>> + Diag(PragmaLoc, diag::err_pragma_pop_macro_malformed);
>> + return;
>> + }
>> +
>> + assert(StrVal[0] == '"' && StrVal[StrVal.size()-1] == '"' &&
>> + "Invalid string token!");
>
> This should be an actual diagnostic instead of an assert. Input
> provided by the user should _never_ cause an assert.
>
>> +
>> + // Create a Token from the string.
>> + Token MacroToPopTok;
>> + MacroToPopTok.startToken();
>> + CreateString(&StrVal[1], StrVal.size() - 2, MacroToPopTok);
>> + MacroToPopTok.setKind(tok::identifier);
>> +
>> + // Get the IdentifierInfo of MacroToPopTok.
>> + IdentifierInfo *IdentInfo = LookUpIdentifierInfo(MacroToPopTok);
>> +
>> + // Retrived the stack<MacroInfo*> associated with the macro.
>> + std::map<IdentifierInfo*, std::stack<MacroInfo*> >::iterator iter =
>> + PragmaPushMacroInfo.find(IdentInfo);
>> + if (iter != PragmaPushMacroInfo.end()) {
>> + // PushedMI is the MacroInfo will want to reinstall.
>> + MacroInfo *PushedMI = iter->second.top();
>> +
>> + // CurrentMI is the current MacroInfo to release.
>> + MacroInfo *CurrentMI = getMacroInfo(IdentInfo);
>> + if (CurrentMI && CurrentMI != PushedMI)
>> + ReleaseMacroInfo(CurrentMI);
>> +
>> + // Reinstall the pushed macro.
>> + setMacroInfo(IdentInfo, PushedMI);
>> +
>> + // Reset the IsPragmaPush flag to false.
>> + if (PushedMI) PushedMI->setIsPragmaPush(false);
>> +
>> + // Pop PragmaPushMacroInfo stack.
>> + iter->second.pop();
>> + if (iter->second.size() == 0)
>> + PragmaPushMacroInfo.erase(iter);
>> + }
>> + else {
>
> No newline before the else.
>
>> + Diag(PragmaLoc, diag::warn_pragma_pop_macro_no_push) << getSpelling(MacroToPopTok);
>
> Keep lines below 80 columns.
>
>> + }
>> +}
>> +
>> /// AddPragmaHandler - Add the specified pragma handler to the preprocessor.
>> /// If 'Namespace' is non-null, then it is a token required to exist on the
>> /// pragma line before the pragma string starts, e.g. "STDC" or "GCC".
>> @@ -699,6 +824,23 @@
>> }
>> };
>>
>> +/// PragmaPushMacroHandler - "#pragma push_macro" marks the file as atomically included.
>
> Keep lines below 80 columns. And the comment is wrong.
>
>> +struct PragmaPushMacroHandler : public PragmaHandler {
>> + PragmaPushMacroHandler() : PragmaHandler("push_macro") {}
>> + virtual void HandlePragma(Preprocessor &PP, Token &PushMacroTok) {
>> + PP.HandlePragmaPushMacro(PushMacroTok);
>> + }
>> +};
>> +
>> +
>> +/// PragmaPopMacroHandler - "#pragma pop_macro" marks the file as atomically included.
>
> Keep lines below 80 columns. And the comment is wrong.
>
>> +struct PragmaPopMacroHandler : public PragmaHandler {
>> + PragmaPopMacroHandler() : PragmaHandler("pop_macro") {}
>> + virtual void HandlePragma(Preprocessor &PP, Token &PopMacroTok) {
>> + PP.HandlePragmaPopMacro(PopMacroTok);
>> + }
>> +};
>> +
>> // Pragma STDC implementations.
>>
>> enum STDCSetting {
>> @@ -780,6 +922,8 @@
>> void Preprocessor::RegisterBuiltinPragmas() {
>> AddPragmaHandler(new PragmaOnceHandler());
>> AddPragmaHandler(new PragmaMarkHandler());
>> + AddPragmaHandler(new PragmaPushMacroHandler());
>> + AddPragmaHandler(new PragmaPopMacroHandler());
>>
>> // #pragma GCC ...
>> AddPragmaHandler("GCC", new PragmaPoisonHandler());
>
> - Michael Spencer
> <pragma-pushpop-macro.c>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
More information about the cfe-dev
mailing list