[cfe-commits] [PATCH] Rewrite "#pragma clang arc_cf_code_audited" impl to play well with parser lookahead

Eli Friedman eli.friedman at gmail.com
Mon Oct 8 19:13:00 PDT 2012


Per subject, rewrite "#pragma clang arc_cf_code_audited"
implementation to be entirely in Parser/Sema; depending on Lexer state
for this pragma messes up parser lookahead.

There isn't really anything specific I want to call out for
discussion, but I think John will want to take a look before I commit
this.

-Eli
-------------- next part --------------
Index: test/Sema/Inputs/pragma-arc-cf-code-audited.h
===================================================================
--- test/Sema/Inputs/pragma-arc-cf-code-audited.h	(revision 165458)
+++ test/Sema/Inputs/pragma-arc-cf-code-audited.h	(working copy)
@@ -13,4 +13,7 @@
 
 
 
-#pragma clang arc_cf_code_audited begin
+
+
+
+void g();
Index: test/Sema/pragma-arc-cf-code-audited.c
===================================================================
--- test/Sema/pragma-arc-cf-code-audited.c	(revision 165458)
+++ test/Sema/pragma-arc-cf-code-audited.c	(working copy)
@@ -2,17 +2,21 @@
 
 #pragma clang arc_cf_code_audited foo // expected-error {{expected 'begin' or 'end'}}
 
-#pragma clang arc_cf_code_audited begin foo // expected-warning {{extra tokens at end of #pragma directive}}
+#pragma clang arc_cf_code_audited begin foo // expected-warning {{extra tokens}}
 
+#pragma clang arc_cf_code_audited begin
 #pragma clang arc_cf_code_audited end
-#pragma clang arc_cf_code_audited end // expected-error {{not currently inside '#pragma clang arc_cf_code_audited'}}
+#pragma clang arc_cf_code_audited end // expected-error {{'#pragma clang arc_cf_code_audited' is not open here}}
 
-#pragma clang arc_cf_code_audited begin // expected-note {{#pragma entered here}}
-#pragma clang arc_cf_code_audited begin // expected-error {{already inside '#pragma clang arc_cf_code_audited'}} expected-note {{#pragma entered here}}
+#pragma clang arc_cf_code_audited begin // expected-note {{'#pragma clang arc_cf_code_audited' entered here}}
+void f();
 
-#include "Inputs/pragma-arc-cf-code-audited.h" // expected-error {{cannot #include files inside '#pragma clang arc_cf_code_audited'}}
+#pragma clang arc_cf_code_audited begin // expected-error {{'#pragma clang arc_cf_code_audited' is already open here}} expected-note {{'#pragma clang arc_cf_code_audited' entered here}}
 
+#include "Inputs/pragma-arc-cf-code-audited.h"
+
 // This is actually on the #pragma line in the header.
-// expected-error {{'#pragma clang arc_cf_code_audited' was not ended within this file}}
+// expected-error {{'#pragma clang arc_cf_code_audited' cannot be applied to a declaration from a different file}}
 
-#pragma clang arc_cf_code_audited begin // expected-error {{'#pragma clang arc_cf_code_audited' was not ended within this file}}
+#pragma clang arc_cf_code_audited end
+#pragma clang arc_cf_code_audited begin // expected-error {{'#pragma clang arc_cf_code_audited' was opened but never closed}}
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td	(revision 165458)
+++ include/clang/Basic/DiagnosticLexKinds.td	(working copy)
@@ -444,16 +444,6 @@
 
 def err_pp_visibility_non_macro : Error<"no macro named %0">;
 
-def err_pp_arc_cf_code_audited_syntax : Error<"expected 'begin' or 'end'">;
-def err_pp_double_begin_of_arc_cf_code_audited : Error<
-  "already inside '#pragma clang arc_cf_code_audited'">;
-def err_pp_unmatched_end_of_arc_cf_code_audited : Error<
-  "not currently inside '#pragma clang arc_cf_code_audited'">;
-def err_pp_include_in_arc_cf_code_audited : Error<
-  "cannot #include files inside '#pragma clang arc_cf_code_audited'">;
-def err_pp_eof_in_arc_cf_code_audited : Error<
-  "'#pragma clang arc_cf_code_audited' was not ended within this file">;
-
 // Module map parsing
 def err_mmap_unknown_token : Error<"skipping stray token">;
 def err_mmap_expected_module : Error<"expected module declaration">;
Index: include/clang/Basic/TokenKinds.def
===================================================================
--- include/clang/Basic/TokenKinds.def	(revision 165458)
+++ include/clang/Basic/TokenKinds.def	(working copy)
@@ -637,6 +637,11 @@
 // handles them.
 ANNOTATION(pragma_opencl_extension)
 
+// Annotation for #pragma clang arc_cf_code_audited...
+// The lexer produces these so that they only take effect when the parser
+// handles them.
+ANNOTATION(pragma_arc_cf_code_audited)
+
 #undef ANNOTATION
 #undef TESTING_KEYWORD
 #undef OBJC2_AT_KEYWORD
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td	(revision 165458)
+++ include/clang/Basic/DiagnosticParseKinds.td	(working copy)
@@ -732,6 +732,9 @@
   "expected '#pragma unused' argument to be a variable name">;
 def warn_pragma_unused_expected_punc : Warning<
   "expected ')' or ',' in '#pragma unused'">;
+// - #pragma clang arc_cf_code_audited
+def err_pragma_expected_begin_end : Error<
+  "expected 'begin' or 'end' after '#pragma clang arc_cf_code_audited'">;
 
 // OpenCL Section 6.8.g
 def err_not_opencl_storage_class_specifier : Error<
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td	(revision 165458)
+++ include/clang/Basic/DiagnosticSemaKinds.td	(working copy)
@@ -2101,6 +2101,19 @@
   InGroup<DiagGroup<"requires-super-attribute">>;
 def note_protocol_decl : Note<
   "protocol is declared here">;
+def err_arc_cf_code_audited_end_different_file : Error<
+  "'#pragma clang arc_cf_code_audited' was opened in a different file">;
+def err_arc_cf_code_audited_end_not_open : Error<
+  "'#pragma clang arc_cf_code_audited' is not open here">;
+def err_arc_cf_code_audited_begin_already_open : Error<
+  "'#pragma clang arc_cf_code_audited' is already open here">;
+def err_arc_cf_code_audited_decl_different_file : Error<
+  "'#pragma clang arc_cf_code_audited' cannot be applied to a declaration "
+  "from a different file">;
+def err_arc_cf_code_audited_unclosed : Error<
+  "'#pragma clang arc_cf_code_audited' was opened but never closed">;
+def note_previous_arc_cf_code_audited_begin : Note<
+  "'#pragma clang arc_cf_code_audited' entered here">;
 
 def err_ns_bridged_not_interface : Error<
   "parameter of 'ns_bridged' attribute does not name an Objective-C class">;
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h	(revision 165458)
+++ include/clang/Sema/Sema.h	(working copy)
@@ -6390,6 +6390,16 @@
   /// \#pragma {STDC,OPENCL} FP_CONTRACT
   void ActOnPragmaFPContract(tok::OnOffSwitch OOS);
 
+  /// ActOnPragmaARCCFCodeAudited - Called on well formed
+  /// \#pragma clang arc_cf_code_audited...
+  void ActOnPragmaARCCFCodeAudited(bool IsBegin, SourceLocation PragmaLoc);
+
+private:
+  /// PragmaARCCFCodeAuditedLoc - Tracks the current open
+  /// \#pragma clang arc_cf_code_audited...
+  SourceLocation PragmaARCCFCodeAuditedLoc;
+public:
+
   /// AddAlignmentAttributesForRecord - Adds any needed alignment attributes to
   /// a the record decl, to handle '\#pragma pack' and '\#pragma options align'.
   void AddAlignmentAttributesForRecord(RecordDecl *RD);
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h	(revision 165458)
+++ include/clang/Lex/Preprocessor.h	(working copy)
@@ -218,10 +218,6 @@
   /// \brief Whether the module import expectes an identifier next. Otherwise,
   /// it expects a '.' or ';'.
   bool ModuleImportExpectsIdentifier;
-  
-  /// \brief The source location of the currently-active
-  /// #pragma clang arc_cf_code_audited begin.
-  SourceLocation PragmaARCCFCodeAuditedLoc;
 
   /// \brief True if we hit the code-completion point.
   bool CodeCompletionReached;
@@ -830,19 +826,6 @@
     getDiagnostics().setSuppressAllDiagnostics(true);
   }
 
-  /// \brief The location of the currently-active \#pragma clang
-  /// arc_cf_code_audited begin.  Returns an invalid location if there
-  /// is no such pragma active.
-  SourceLocation getPragmaARCCFCodeAuditedLoc() const {
-    return PragmaARCCFCodeAuditedLoc;
-  }
-
-  /// \brief Set the location of the currently-active \#pragma clang
-  /// arc_cf_code_audited begin.  An invalid location ends the pragma.
-  void setPragmaARCCFCodeAuditedLoc(SourceLocation Loc) {
-    PragmaARCCFCodeAuditedLoc = Loc;
-  }
-
   /// \brief Instruct the preprocessor to skip part of the main source file.
   ///
   /// \param Bytes The number of bytes in the preamble to skip.
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h	(revision 165458)
+++ include/clang/Parse/Parser.h	(working copy)
@@ -178,6 +178,7 @@
   OwningPtr<PragmaHandler> RedefineExtnameHandler;
   OwningPtr<PragmaHandler> FPContractHandler;
   OwningPtr<PragmaHandler> OpenCLExtensionHandler;
+  OwningPtr<PragmaHandler> ARCCFCodeAuditedHandler;
   OwningPtr<CommentHandler> CommentSemaHandler;
 
   /// Whether the '>' token acts as an operator or not. This will be
@@ -469,6 +470,10 @@
   /// #pragma OPENCL EXTENSION...
   void HandlePragmaOpenCLExtension();
 
+  /// \brief Handle the annotation token produced for
+  /// #pragma clang arc_cf_code_audited...
+  void HandlePragmaARCCFCodeAudited();
+
   /// GetLookAheadToken - This peeks ahead N tokens and returns that token
   /// without consuming any tokens.  LookAhead(0) returns 'Tok', LookAhead(1)
   /// returns the token after Tok, etc.
Index: lib/Sema/SemaAttr.cpp
===================================================================
--- lib/Sema/SemaAttr.cpp	(revision 165458)
+++ lib/Sema/SemaAttr.cpp	(working copy)
@@ -291,15 +291,27 @@
 }
 
 void Sema::AddCFAuditedAttribute(Decl *D) {
-  SourceLocation Loc = PP.getPragmaARCCFCodeAuditedLoc();
-  if (!Loc.isValid()) return;
+  if (!PragmaARCCFCodeAuditedLoc.isValid()) return;
 
   // Don't add a redundant or conflicting attribute.
   if (D->hasAttr<CFAuditedTransferAttr>() ||
       D->hasAttr<CFUnknownTransferAttr>())
     return;
 
-  D->addAttr(::new (Context) CFAuditedTransferAttr(Loc, Context));
+  // Make sure the decl and the #pragma are in the same file.
+  SourceLocation PragmaFileLoc =
+    SourceMgr.getExpansionLoc(PragmaARCCFCodeAuditedLoc);
+  SourceLocation DeclLoc = D->getLocation();
+  SourceLocation DeclFileLoc = SourceMgr.getExpansionLoc(DeclLoc);
+  if (!SourceMgr.isFromSameFile(PragmaFileLoc, DeclFileLoc)) {
+    Diag(DeclLoc, diag::err_arc_cf_code_audited_decl_different_file);
+    Diag(PragmaARCCFCodeAuditedLoc,
+         diag::note_previous_arc_cf_code_audited_begin);
+    return;
+  }
+
+  D->addAttr(::new (Context) CFAuditedTransferAttr(PragmaARCCFCodeAuditedLoc,
+                                                   Context));
 }
 
 typedef std::vector<std::pair<unsigned, SourceLocation> > VisStack;
@@ -376,6 +388,40 @@
   }
 }
 
+void Sema::ActOnPragmaARCCFCodeAudited(bool IsBegin, SourceLocation PragmaLoc) {
+  if (IsBegin) {
+    // #pragma clang arc_cf_code_audited begin
+    if (PragmaARCCFCodeAuditedLoc.isValid()) {
+      Diag(PragmaLoc, diag::err_arc_cf_code_audited_begin_already_open);
+      Diag(PragmaARCCFCodeAuditedLoc,
+           diag::note_previous_arc_cf_code_audited_begin);
+    }
+    PragmaARCCFCodeAuditedLoc = PragmaLoc;
+    return;
+  }
+
+  // #pragma clang arc_cf_code_audited end
+  // We can't close arc_cf_code_audited if it isn't open.
+  if (!PragmaARCCFCodeAuditedLoc.isValid()) {
+    PP.Diag(PragmaLoc, diag::err_arc_cf_code_audited_end_not_open);
+    return;
+  }
+
+  // We can only close arc_cf_code_audited from the same file, for sanity.
+  SourceLocation BeginFileLoc =
+    SourceMgr.getExpansionLoc(PragmaARCCFCodeAuditedLoc);
+  SourceLocation EndFileLoc = SourceMgr.getExpansionLoc(PragmaLoc);
+  if (!SourceMgr.isFromSameFile(BeginFileLoc, EndFileLoc)) {
+    Diag(PragmaLoc, diag::err_arc_cf_code_audited_end_different_file);
+    Diag(PragmaARCCFCodeAuditedLoc,
+         diag::note_previous_arc_cf_code_audited_begin);
+    return;
+  }
+
+  PragmaARCCFCodeAuditedLoc = SourceLocation();
+  return;
+}
+
 void Sema::PushNamespaceVisibilityAttr(const VisibilityAttr *Attr,
                                        SourceLocation Loc) {
   // Visibility calculations will consider the namespace's visibility.
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp	(revision 165458)
+++ lib/Sema/Sema.cpp	(working copy)
@@ -543,6 +543,10 @@
                                            this)),
                               UnusedFileScopedDecls.end());
 
+  // Check for unclosed "#pragma clang arc_cf_code_audited begin".
+  if (PragmaARCCFCodeAuditedLoc.isValid())
+    Diag(PragmaARCCFCodeAuditedLoc, diag::err_arc_cf_code_audited_unclosed);
+
   if (TUKind == TU_Prefix) {
     // Translation unit prefixes don't need any of the checking below.
     TUScope = 0;
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp	(revision 165458)
+++ lib/Lex/PPDirectives.cpp	(working copy)
@@ -1338,15 +1338,6 @@
     return;
   }
 
-  // Complain about attempts to #include files in an audit pragma.
-  if (PragmaARCCFCodeAuditedLoc.isValid()) {
-    Diag(HashLoc, diag::err_pp_include_in_arc_cf_code_audited);
-    Diag(PragmaARCCFCodeAuditedLoc, diag::note_pragma_entered_here);
-
-    // Immediately leave the pragma.
-    PragmaARCCFCodeAuditedLoc = SourceLocation();
-  }
-
   if (HeaderInfo.HasIncludeAliasMap()) {
     // Map the filename with the brackets still attached.  If the name doesn't 
     // map to anything, fall back on the filename we've already gotten the 
Index: lib/Lex/Pragma.cpp
===================================================================
--- lib/Lex/Pragma.cpp	(revision 165458)
+++ lib/Lex/Pragma.cpp	(working copy)
@@ -1224,60 +1224,6 @@
   }
 };
 
-/// PragmaARCCFCodeAuditedHandler - 
-///   \#pragma clang arc_cf_code_audited begin/end
-struct PragmaARCCFCodeAuditedHandler : public PragmaHandler {
-  PragmaARCCFCodeAuditedHandler() : PragmaHandler("arc_cf_code_audited") {}
-  virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-                            Token &NameTok) {
-    SourceLocation Loc = NameTok.getLocation();
-    bool IsBegin;
-
-    Token Tok;
-
-    // Lex the 'begin' or 'end'.
-    PP.LexUnexpandedToken(Tok);
-    const IdentifierInfo *BeginEnd = Tok.getIdentifierInfo();
-    if (BeginEnd && BeginEnd->isStr("begin")) {
-      IsBegin = true;
-    } else if (BeginEnd && BeginEnd->isStr("end")) {
-      IsBegin = false;
-    } else {
-      PP.Diag(Tok.getLocation(), diag::err_pp_arc_cf_code_audited_syntax);
-      return;
-    }
-
-    // Verify that this is followed by EOD.
-    PP.LexUnexpandedToken(Tok);
-    if (Tok.isNot(tok::eod))
-      PP.Diag(Tok, diag::ext_pp_extra_tokens_at_eol) << "pragma";
-
-    // The start location of the active audit.
-    SourceLocation BeginLoc = PP.getPragmaARCCFCodeAuditedLoc();
-
-    // The start location we want after processing this.
-    SourceLocation NewLoc;
-
-    if (IsBegin) {
-      // Complain about attempts to re-enter an audit.
-      if (BeginLoc.isValid()) {
-        PP.Diag(Loc, diag::err_pp_double_begin_of_arc_cf_code_audited);
-        PP.Diag(BeginLoc, diag::note_pragma_entered_here);
-      }
-      NewLoc = Loc;
-    } else {
-      // Complain about attempts to leave an audit that doesn't exist.
-      if (!BeginLoc.isValid()) {
-        PP.Diag(Loc, diag::err_pp_unmatched_end_of_arc_cf_code_audited);
-        return;
-      }
-      NewLoc = SourceLocation();
-    }
-
-    PP.setPragmaARCCFCodeAuditedLoc(NewLoc);
-  }
-};
-
 }  // end anonymous namespace
 
 
@@ -1301,7 +1247,6 @@
   AddPragmaHandler("clang", new PragmaDebugHandler());
   AddPragmaHandler("clang", new PragmaDependencyHandler());
   AddPragmaHandler("clang", new PragmaDiagnosticHandler("clang"));
-  AddPragmaHandler("clang", new PragmaARCCFCodeAuditedHandler());
 
   AddPragmaHandler("STDC", new PragmaSTDC_FENV_ACCESSHandler());
   AddPragmaHandler("STDC", new PragmaSTDC_CX_LIMITED_RANGEHandler());
Index: lib/Lex/PPLexerChange.cpp
===================================================================
--- lib/Lex/PPLexerChange.cpp	(revision 165458)
+++ lib/Lex/PPLexerChange.cpp	(working copy)
@@ -245,17 +245,6 @@
     }
   }
 
-  // Complain about reaching a true EOF within arc_cf_code_audited.
-  // We don't want to complain about reaching the end of a macro
-  // instantiation or a _Pragma.
-  if (PragmaARCCFCodeAuditedLoc.isValid() &&
-      !isEndOfMacro && !(CurLexer && CurLexer->Is_PragmaLexer)) {
-    Diag(PragmaARCCFCodeAuditedLoc, diag::err_pp_eof_in_arc_cf_code_audited);
-
-    // Recover by leaving immediately.
-    PragmaARCCFCodeAuditedLoc = SourceLocation();
-  }
-
   // If this is a #include'd file, pop it off the include stack and continue
   // lexing the #includer file.
   if (!IncludeMacroStack.empty()) {
Index: lib/Parse/ParsePragma.cpp
===================================================================
--- lib/Parse/ParsePragma.cpp	(revision 165458)
+++ lib/Parse/ParsePragma.cpp	(working copy)
@@ -151,6 +151,13 @@
   }
 }
 
+void Parser::HandlePragmaARCCFCodeAudited() {
+  bool IsBegin = static_cast<bool>(
+                 reinterpret_cast<uintptr_t>(Tok.getAnnotationValue()));
+  SourceLocation PragmaLoc = ConsumeToken();
+  Actions.ActOnPragmaARCCFCodeAudited(IsBegin, PragmaLoc);
+}
+
 // #pragma GCC visibility comes in two variants:
 //   'push' '(' [visibility] ')'
 //   'pop'
@@ -718,3 +725,38 @@
                       /*OwnsTokens=*/false);
 }
 
+void 
+PragmaARCCFCodeAuditedHandler::HandlePragma(Preprocessor &PP, 
+                                            PragmaIntroducerKind Introducer,
+                                            Token &Tok) {
+  SourceLocation Loc = Tok.getLocation();
+  bool IsBegin;
+
+  // Lex the 'begin' or 'end'.
+  PP.LexUnexpandedToken(Tok);
+  const IdentifierInfo *BeginEnd = Tok.getIdentifierInfo();
+  if (BeginEnd && BeginEnd->isStr("begin")) {
+    IsBegin = true;
+  } else if (BeginEnd && BeginEnd->isStr("end")) {
+    IsBegin = false;
+  } else {
+    PP.Diag(Tok.getLocation(), diag::err_pragma_expected_begin_end);
+    return;
+  }
+
+  PP.LexUnexpandedToken(Tok);
+  if (Tok.isNot(tok::eod)) {
+    PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol) <<
+      "clang arc_cf_code_audited";
+    return;
+  }
+
+  Token *Toks = new Token[1];
+  Toks[0].startToken();
+  Toks[0].setKind(tok::annot_pragma_arc_cf_code_audited);
+  Toks[0].setLocation(Loc);
+  Toks[0].setAnnotationValue(
+                    reinterpret_cast<void*>(static_cast<uintptr_t>(IsBegin)));
+  PP.EnterTokenStream(Toks, 1, /*DisableMacroExpansion=*/true,
+                      /*OwnsTokens=*/true);
+}
Index: lib/Parse/ParsePragma.h
===================================================================
--- lib/Parse/ParsePragma.h	(revision 165458)
+++ lib/Parse/ParsePragma.h	(working copy)
@@ -99,6 +99,12 @@
                             Token &FirstToken);
 };
   
+class PragmaARCCFCodeAuditedHandler : public PragmaHandler {
+public:
+  PragmaARCCFCodeAuditedHandler() : PragmaHandler("arc_cf_code_audited") {}
+  virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+                            Token &FirstToken);
+};
 
 }  // end namespace clang
 
Index: lib/Parse/Parser.cpp
===================================================================
--- lib/Parse/Parser.cpp	(revision 165458)
+++ lib/Parse/Parser.cpp	(working copy)
@@ -94,6 +94,9 @@
     PP.AddPragmaHandler("OPENCL", FPContractHandler.get());
   }
 
+  ARCCFCodeAuditedHandler.reset(new PragmaARCCFCodeAuditedHandler());
+  PP.AddPragmaHandler("clang", ARCCFCodeAuditedHandler.get());
+
   CommentSemaHandler.reset(new ActionCommentHandler(actions));
   PP.addCommentHandler(CommentSemaHandler.get());
 
@@ -452,6 +455,9 @@
   PP.RemovePragmaHandler("STDC", FPContractHandler.get());
   FPContractHandler.reset();
 
+  PP.RemovePragmaHandler("clang", ARCCFCodeAuditedHandler.get());
+  FPContractHandler.reset();
+
   PP.removeCommentHandler(CommentSemaHandler.get());
 
   PP.clearCodeCompletionHandler();
@@ -656,6 +662,9 @@
   case tok::annot_pragma_opencl_extension:
     HandlePragmaOpenCLExtension();
     return DeclGroupPtrTy();
+  case tok::annot_pragma_arc_cf_code_audited:
+    HandlePragmaARCCFCodeAudited();
+    return DeclGroupPtrTy();
   case tok::semi:
     ConsumeExtraSemi(OutsideFunction);
     // TODO: Invoke action for top-level semicolon.


More information about the cfe-commits mailing list