[cfe-dev] [PATCH] preprocessor pragma push_macro support
Michael Spencer
bigcheesegs at gmail.com
Tue Jul 27 10:35:51 PDT 2010
On Tue, Jul 27, 2010 at 1:08 PM, Daniel Dunbar <daniel.dunbar at gmail.com> wrote:
> 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
Ah, you're right, I saw the getSpelling version that returns
llvm::StringRef and thought it could be used here.
- Michael Spencer
>>
>>> +
>>> + // 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