From e665d6929e11796620ff799bc0186ebd747bfc76 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Sat, 18 Jun 2011 00:53:41 +0000 Subject: [PATCH] [arcmt] Fix the ARC migrator. -arcmt-modify requires running before the initialization of SourceManager because it is going to modify the input file. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@133323 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/ARCMigrate/ARCMTActions.h | 4 +- include/clang/Frontend/FrontendAction.h | 9 ++++ lib/ARCMigrate/ARCMT.cpp | 10 +++- lib/ARCMigrate/ARCMTActions.cpp | 17 +++---- lib/ARCMigrate/Internals.h | 2 + lib/Frontend/FrontendAction.cpp | 8 +++ test/ARCMT/alloc-with-zone-check.m | 2 +- test/ARCMT/atautorelease-check.m | 2 +- test/ARCMT/checking.m | 2 +- test/ARCMT/cxx-checking.mm | 2 +- test/ARCMT/nonobjc-to-objc-cast-2.m | 2 +- test/ARCMT/releases-driver.m | 68 +++++++++++++++++++++++++ test/ARCMT/releases-driver.m.result | 61 ++++++++++++++++++++++ tools/arcmt-test/arcmt-test.cpp | 9 ++-- 14 files changed, 175 insertions(+), 23 deletions(-) create mode 100644 test/ARCMT/releases-driver.m create mode 100644 test/ARCMT/releases-driver.m.result diff --git a/include/clang/ARCMigrate/ARCMTActions.h b/include/clang/ARCMigrate/ARCMTActions.h index 2dd57ab88b..fd85a0836a 100644 --- a/include/clang/ARCMigrate/ARCMTActions.h +++ b/include/clang/ARCMigrate/ARCMTActions.h @@ -18,7 +18,7 @@ namespace arcmt { class CheckAction : public WrapperFrontendAction { protected: - virtual void ExecuteAction(); + virtual bool BeginInvocation(CompilerInstance &CI); public: CheckAction(FrontendAction *WrappedAction); @@ -26,7 +26,7 @@ class CheckAction : public WrapperFrontendAction { class TransformationAction : public WrapperFrontendAction { protected: - virtual void ExecuteAction(); + virtual bool BeginInvocation(CompilerInstance &CI); public: TransformationAction(FrontendAction *WrappedAction); diff --git a/include/clang/Frontend/FrontendAction.h b/include/clang/Frontend/FrontendAction.h index 2538f55aaf..f335475665 100644 --- a/include/clang/Frontend/FrontendAction.h +++ b/include/clang/Frontend/FrontendAction.h @@ -78,6 +78,14 @@ class FrontendAction { virtual ASTConsumer *CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) = 0; + /// \brief Callback before starting processing a single input, giving the + /// opportunity to modify the CompilerInvocation or do some other action + /// before BeginSourceFileAction is called. + /// + /// \return True on success; on failure \see BeginSourceFileAction() and + /// ExecutionAction() and EndSourceFileAction() will not be called. + virtual bool BeginInvocation(CompilerInstance &CI) { return true; } + /// BeginSourceFileAction - Callback at the start of processing a single /// input. /// @@ -265,6 +273,7 @@ class WrapperFrontendAction : public FrontendAction { protected: virtual ASTConsumer *CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile); + virtual bool BeginInvocation(CompilerInstance &CI); virtual bool BeginSourceFileAction(CompilerInstance &CI, llvm::StringRef Filename); virtual void ExecuteAction(); diff --git a/lib/ARCMigrate/ARCMT.cpp b/lib/ARCMigrate/ARCMT.cpp index a93f1c62c0..99980345c4 100644 --- a/lib/ARCMigrate/ARCMT.cpp +++ b/lib/ARCMigrate/ARCMT.cpp @@ -79,6 +79,14 @@ void CapturedDiagList::reportDiagnostics(Diagnostic &Diags) const { Diags.Report(*I); } +bool CapturedDiagList::hasErrors() const { + for (ListTy::const_iterator I = List.begin(), E = List.end(); I != E; ++I) + if (I->getLevel() >= Diagnostic::Error) + return true; + + return false; +} + namespace { class CaptureDiagnosticClient : public DiagnosticClient { @@ -236,7 +244,7 @@ bool arcmt::checkForManualIssues(CompilerInvocation &origCI, DiagClient->EndSourceFile(); - return Diags->getClient()->getNumErrors() > 0; + return capturedDiags.hasErrors(); } //===----------------------------------------------------------------------===// diff --git a/lib/ARCMigrate/ARCMTActions.cpp b/lib/ARCMigrate/ARCMTActions.cpp index 4da928a0eb..7de62d289c 100644 --- a/lib/ARCMigrate/ARCMTActions.cpp +++ b/lib/ARCMigrate/ARCMTActions.cpp @@ -14,29 +14,24 @@ using namespace clang; using namespace arcmt; -void CheckAction::ExecuteAction() { - CompilerInstance &CI = getCompilerInstance(); +bool CheckAction::BeginInvocation(CompilerInstance &CI) { if (arcmt::checkForManualIssues(CI.getInvocation(), getCurrentFile(), getCurrentFileKind(), CI.getDiagnostics().getClient())) - return; + return false; // errors, stop the action. // We only want to see warnings reported from arcmt::checkForManualIssues. CI.getDiagnostics().setIgnoreAllWarnings(true); - WrapperFrontendAction::ExecuteAction(); + return true; } CheckAction::CheckAction(FrontendAction *WrappedAction) : WrapperFrontendAction(WrappedAction) {} -void TransformationAction::ExecuteAction() { - CompilerInstance &CI = getCompilerInstance(); - if (arcmt::applyTransformations(CI.getInvocation(), getCurrentFile(), +bool TransformationAction::BeginInvocation(CompilerInstance &CI) { + return !arcmt::applyTransformations(CI.getInvocation(), getCurrentFile(), getCurrentFileKind(), - CI.getDiagnostics().getClient())) - return; - - WrapperFrontendAction::ExecuteAction(); + CI.getDiagnostics().getClient()); } TransformationAction::TransformationAction(FrontendAction *WrappedAction) diff --git a/lib/ARCMigrate/Internals.h b/lib/ARCMigrate/Internals.h index d0d545ec51..4f9b138a06 100644 --- a/lib/ARCMigrate/Internals.h +++ b/lib/ARCMigrate/Internals.h @@ -30,6 +30,8 @@ class CapturedDiagList { bool hasDiagnostic(llvm::ArrayRef IDs, SourceRange range) const; void reportDiagnostics(Diagnostic &diags) const; + + bool hasErrors() const; }; class TransformActions { diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp index 0024818ad8..0128d6ee05 100644 --- a/lib/Frontend/FrontendAction.cpp +++ b/lib/Frontend/FrontendAction.cpp @@ -130,6 +130,9 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, setCurrentFile(Filename, InputKind); setCompilerInstance(&CI); + if (!BeginInvocation(CI)) + goto failure; + // AST files follow a very different path, since they share objects via the // AST unit. if (InputKind == IK_AST) { @@ -386,8 +389,13 @@ ASTConsumer *WrapperFrontendAction::CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) { return WrappedAction->CreateASTConsumer(CI, InFile); } +bool WrapperFrontendAction::BeginInvocation(CompilerInstance &CI) { + return WrappedAction->BeginInvocation(CI); +} bool WrapperFrontendAction::BeginSourceFileAction(CompilerInstance &CI, llvm::StringRef Filename) { + WrappedAction->setCurrentFile(getCurrentFile(), getCurrentFileKind()); + WrappedAction->setCompilerInstance(&CI); return WrappedAction->BeginSourceFileAction(CI, Filename); } void WrapperFrontendAction::ExecuteAction() { diff --git a/test/ARCMT/alloc-with-zone-check.m b/test/ARCMT/alloc-with-zone-check.m index a987fa8b84..01924a9987 100644 --- a/test/ARCMT/alloc-with-zone-check.m +++ b/test/ARCMT/alloc-with-zone-check.m @@ -1,4 +1,4 @@ -// RUN: arcmt-test -check-only -verify --args %s +// RUN: %clang_cc1 -arcmt-check -verify -triple x86_64-apple-darwin10 -fobjc-nonfragile-abi %s #if __has_feature(objc_arr) #define NS_AUTOMATED_REFCOUNT_UNAVAILABLE __attribute__((unavailable("not available in automatic reference counting mode"))) diff --git a/test/ARCMT/atautorelease-check.m b/test/ARCMT/atautorelease-check.m index 126642cd38..6528f3e62d 100644 --- a/test/ARCMT/atautorelease-check.m +++ b/test/ARCMT/atautorelease-check.m @@ -1,4 +1,4 @@ -// RUN: arcmt-test -check-only -verify --args %s +// RUN: %clang_cc1 -arcmt-check -verify -triple x86_64-apple-darwin10 -fobjc-nonfragile-abi %s #if __has_feature(objc_arr) #define NS_AUTOMATED_REFCOUNT_UNAVAILABLE __attribute__((unavailable("not available in automatic reference counting mode"))) diff --git a/test/ARCMT/checking.m b/test/ARCMT/checking.m index ffb245e8bb..62387fa23b 100644 --- a/test/ARCMT/checking.m +++ b/test/ARCMT/checking.m @@ -1,4 +1,4 @@ -// RUN: arcmt-test -check-only -verify --args %s +// RUN: %clang_cc1 -arcmt-check -verify -triple x86_64-apple-darwin10 -fobjc-nonfragile-abi %s #include "Common.h" diff --git a/test/ARCMT/cxx-checking.mm b/test/ARCMT/cxx-checking.mm index 667138c974..c298b94b82 100644 --- a/test/ARCMT/cxx-checking.mm +++ b/test/ARCMT/cxx-checking.mm @@ -1,4 +1,4 @@ -// RUN: arcmt-test -check-only -verify --args -triple x86_64-apple-darwin10 -fobjc-nonfragile-abi -fsyntax-only -fblocks -Warc-abi %s +// RUN: %clang_cc1 -arcmt-check -verify -triple x86_64-apple-darwin10 -fobjc-nonfragile-abi -fsyntax-only -fblocks -Warc-abi %s // Classes that have an Objective-C object pointer. struct HasObjectMember0 { // expected-warning{{'HasObjectMember0' cannot be shared between ARC and non-ARC code; add a copy constructor, a copy assignment operator, and a destructor to make it ABI-compatible}} diff --git a/test/ARCMT/nonobjc-to-objc-cast-2.m b/test/ARCMT/nonobjc-to-objc-cast-2.m index dba5227fae..46ef02400e 100644 --- a/test/ARCMT/nonobjc-to-objc-cast-2.m +++ b/test/ARCMT/nonobjc-to-objc-cast-2.m @@ -1,4 +1,4 @@ -// RUN: arcmt-test -check-only -verify --args %s +// RUN: %clang_cc1 -arcmt-check -verify -triple x86_64-apple-darwin10 -fobjc-nonfragile-abi %s typedef int BOOL; typedef const struct __CFString * CFStringRef; diff --git a/test/ARCMT/releases-driver.m b/test/ARCMT/releases-driver.m new file mode 100644 index 0000000000..a016b26f44 --- /dev/null +++ b/test/ARCMT/releases-driver.m @@ -0,0 +1,68 @@ +// RUN: %clang_cc1 -fobjc-nonfragile-abi -fblocks -fsyntax-only -fobjc-arc -x objective-c %s.result +// RUN: cp %s %t +// RUN: %clang_cc1 -arcmt-modify -triple x86_64-apple-macosx10.6 -fobjc-nonfragile-abi -x objective-c %t +// RUN: diff %t %s.result +// RUN: rm %t + +typedef int BOOL; + +id IhaveSideEffect(); + +@protocol NSObject +- (BOOL)isEqual:(id)object; +- (id)retain; +- (oneway void)release; +@end + +@interface NSObject {} +@end + +@interface Foo : NSObject { + id bar; +} +@property (retain) id bar; +-(void)test:(id)obj; +@end + +@implementation Foo + +@synthesize bar; + +-(void)test:(id)obj { + id x = self.bar; + [x retain]; + self.bar = obj; + // do stuff with x; + [x release]; + + [IhaveSideEffect() release]; + + [x release], x = 0; +} + +@end + +void func(Foo *p) { + [p release]; + (([p release])); +} + +@interface Baz { + id _foo; +} +@end + +@implementation Baz +- dealloc { + [_foo release]; + return 0; +} +@end + +#define RELEASE_MACRO(x) [x release] +#define RELEASE_MACRO2(x) RELEASE_MACRO(x) + +void test2(id p) { + RELEASE_MACRO(p); + RELEASE_MACRO2(p); +} diff --git a/test/ARCMT/releases-driver.m.result b/test/ARCMT/releases-driver.m.result new file mode 100644 index 0000000000..247cad1612 --- /dev/null +++ b/test/ARCMT/releases-driver.m.result @@ -0,0 +1,61 @@ +// RUN: %clang_cc1 -fobjc-nonfragile-abi -fblocks -fsyntax-only -fobjc-arc -x objective-c %s.result +// RUN: cp %s %t +// RUN: %clang_cc1 -arcmt-modify -triple x86_64-apple-macosx10.6 -fobjc-nonfragile-abi -x objective-c %t +// RUN: diff %t %s.result +// RUN: rm %t + +typedef int BOOL; + +id IhaveSideEffect(); + +@protocol NSObject +- (BOOL)isEqual:(id)object; +- (id)retain; +- (oneway void)release; +@end + +@interface NSObject {} +@end + +@interface Foo : NSObject { + id bar; +} +@property (retain) id bar; +-(void)test:(id)obj; +@end + +@implementation Foo + +@synthesize bar; + +-(void)test:(id)obj { + id x = self.bar; + self.bar = obj; + // do stuff with x; + + IhaveSideEffect(); + + x = 0; +} + +@end + +void func(Foo *p) { +} + +@interface Baz { + id _foo; +} +@end + +@implementation Baz +- dealloc { + return 0; +} +@end + +#define RELEASE_MACRO(x) [x release] +#define RELEASE_MACRO2(x) RELEASE_MACRO(x) + +void test2(id p) { +} diff --git a/tools/arcmt-test/arcmt-test.cpp b/tools/arcmt-test/arcmt-test.cpp index baa3f568c0..702e13a414 100644 --- a/tools/arcmt-test/arcmt-test.cpp +++ b/tools/arcmt-test/arcmt-test.cpp @@ -111,10 +111,11 @@ static bool checkForMigration(llvm::StringRef resourcesPath, if (!CI.getLangOpts().ObjC1) return false; - return arcmt::checkForManualIssues(CI, - CI.getFrontendOpts().Inputs[0].second, - CI.getFrontendOpts().Inputs[0].first, - Diags->getClient()); + arcmt::checkForManualIssues(CI, + CI.getFrontendOpts().Inputs[0].second, + CI.getFrontendOpts().Inputs[0].first, + Diags->getClient()); + return Diags->getClient()->getNumErrors() > 0; } static void printResult(FileRemapper &remapper, llvm::raw_ostream &OS) {