Skip to content

Commit 4ec65cf

Browse files
committed
refactor: Add a continuation verification function to
report errors for the "Generate Missing Function Definitions" action that can be propagated back to SK rdar://33403708 apple-llvm-split-commit: 414251c9711ba8fa6781f2f899d855a9579b91d5 apple-llvm-split-dir: clang/
1 parent c37c959 commit 4ec65cf

File tree

9 files changed

+119
-4
lines changed

9 files changed

+119
-4
lines changed

clang/include/clang-c/Refactor.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,6 +1169,18 @@ CINDEX_LINKAGE
11691169
CXIndexerQuery clang_RefactoringContinuation_getIndexerQuery(
11701170
CXRefactoringContinuation Continuation, unsigned Index);
11711171

1172+
/**
1173+
* \brief Verify that the all of the indexer queries are satisfied by the
1174+
* continuation.
1175+
*
1176+
* \returns Null if all of the queries are satisfied an no errors have been
1177+
* reported, or a set of diagnostics that describes why the continuation should
1178+
* not be run.
1179+
*/
1180+
CINDEX_LINKAGE
1181+
CXDiagnosticSet clang_RefactoringContinuation_verifyBeforeFinalizing(
1182+
CXRefactoringContinuation Continuation);
1183+
11721184
/**
11731185
* \brief Terminate the connection between the initiation TU and the refactoring
11741186
* continuation.

clang/include/clang/Basic/DiagnosticRefactoringKinds.td

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,16 @@ def err_rename_external_source_symbol : Error<"%0 is declared in a %1 file; "
2424
"rename can be initiated in a %1 file only">;
2525
}
2626

27+
let CategoryName = "Refactoring Continuation Issue" in {
28+
29+
def err_ref_continuation_missing_implementation : Error<
30+
"no %select{implementation file|@implementation declaration}0 for the "
31+
"selected %select{declaration|@interface}0 %1; please add one and run the "
32+
"refactoring action again">;
33+
34+
def err_implement_declared_methods_all_implemented : Error<
35+
"the selected methods are already implemented">;
36+
37+
}
38+
2739
} // end of Refactoring diagnostics

clang/include/clang/Tooling/Refactor/IndexerQuery.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ class IndexerQuery {
3636

3737
virtual void invalidateTUSpecificState() = 0;
3838

39+
/// Checks if this query was satisfied. Returns true if it wasn't and reports
40+
/// appropriate errors.
41+
virtual bool verify(ASTContext &) { return false; }
42+
3943
// Mainly used for testing.
4044
static llvm::Error loadResultsFromYAML(StringRef Source,
4145
ArrayRef<IndexerQuery *> Queries);
@@ -82,6 +86,8 @@ class ASTUnitForImplementationOfDeclarationQuery final
8286

8387
void setResult(PersistentFileID File) { Result = std::move(File); }
8488

89+
bool verify(ASTContext &Context) override;
90+
8591
const PersistentFileID &getResult() const { return Result; }
8692

8793
static bool classof(const IndexerQuery *D) {
@@ -230,6 +236,8 @@ class DeclarationsQuery : public IndexerQuery {
230236

231237
void invalidateTUSpecificState() override { Input.clear(); }
232238

239+
bool verify(ASTContext &Context) override;
240+
233241
void setOutput(std::vector<Indexed<PersistentDeclRef<Decl>>> Output) {
234242
this->Output = Output;
235243
}

clang/lib/Tooling/Refactor/IndexerQueries.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//
88
//===----------------------------------------------------------------------===//
99

10+
#include "clang/Tooling/Core/RefactoringDiagnostic.h"
1011
#include "clang/Tooling/Refactor/IndexerQuery.h"
1112
#include "llvm/Support/Errc.h"
1213
#include "llvm/Support/Error.h"
@@ -44,6 +45,40 @@ clang::tooling::indexer::fileThatShouldContainImplementationOf(const Decl *D) {
4445
return llvm::make_unique<ASTUnitForImplementationOfDeclarationQuery>(D);
4546
}
4647

48+
bool ASTUnitForImplementationOfDeclarationQuery::verify(ASTContext &Context) {
49+
if (!D) {
50+
assert(false && "Query should be verified before persisting");
51+
return false;
52+
}
53+
// Check if we've got the filename.
54+
if (!Result.Filename.empty())
55+
return false;
56+
Context.getDiagnostics().Report(
57+
D->getLocation(), diag::err_ref_continuation_missing_implementation)
58+
<< isa<ObjCContainerDecl>(D) << cast<NamedDecl>(D);
59+
return true;
60+
}
61+
62+
bool DeclarationsQuery::verify(ASTContext &Context) {
63+
if (Input.empty()) {
64+
assert(false && "Query should be verified before persisting");
65+
return false;
66+
}
67+
if (!Output.empty()) {
68+
// At least one output declaration must be valid.
69+
for (const auto &Ref : Output) {
70+
if (!Ref.Decl.USR.empty())
71+
return false;
72+
}
73+
}
74+
// FIXME: This is too specific, the new refactoring engine at llvm.org should
75+
// generalize this.
76+
Context.getDiagnostics().Report(
77+
Input[0]->getLocation(),
78+
diag::err_implement_declared_methods_all_implemented);
79+
return true;
80+
}
81+
4782
namespace {
4883

4984
struct QueryPredicateNode {

clang/test/Refactor/ImplementDeclaredMethods/implement-declared-methods.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@ struct }
8585
// query-mix-impl-header: [ { name: ast.producer.query, filenameResult: "%S/classInHeader.h" } , { name: decl.query , predicateResults: [{name: decl.isDefined, intValues: [0, 1, 0, 1] }] }]
8686
// RUN: clang-refactor-test perform -action implement-declared-methods -selected=class-in-header -continuation-file=%S/Inputs/classInHeader.cpp -query-results=query-mix-impl-header %s | FileCheck %S/Inputs/classInHeader.h
8787

88+
// query-no-impl: [ { name: ast.producer.query, filenameResult: "%s" } , { name: decl.query , predicateResults: [{name: decl.isDefined, intValues: [1, 1, 1, 1] }] }]
89+
// RUN: not clang-refactor-test perform -action implement-declared-methods -selected=class-in-header -continuation-file=%S/Inputs/classInHeader.cpp -query-results=query-no-impl %s 2>&1 | FileCheck --check-prefix=ALL-IMPLEMENTED-ERROR %s
90+
// ALL-IMPLEMENTED-ERROR: error: continuation failed: the selected methods are already implemented
91+
8892
// Ensure that the methods which are placed right after the record are placed
8993
// after the outermost record:
9094
namespace ns {

clang/test/Refactor/ImplementDeclaredMethods/implement-declared-methods.m

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ - (void)someOtherMethod { }
4747
// RUN: not clang-refactor-test perform -action implement-declared-methods -selected=all-methods -continuation-file=%s -query-results=query-all-impl %s -DNO_IMPL 2>&1 | FileCheck --check-prefix=CHECK-EMPTY-ERR %s
4848
// CHECK-EMPTY-ERR: failed to perform the refactoring continuation (the target @interface is not implemented in the continuation AST unit)!
4949

50+
// query-no-file-all-impl: [ { name: ast.producer.query, filenameResult: "" } , { name: decl.query , predicateResults: [{name: decl.isDefined, intValues: [0, 0, 0, 0] }] }]
51+
// RUN: not clang-refactor-test perform -action implement-declared-methods -selected=all-methods -continuation-file=%s -query-results=query-no-file-all-impl %s -DNO_IMPL 2>&1 | FileCheck --check-prefix=CHECK-NO-IMPLEMENTATION-ERROR %s
52+
// CHECK-NO-IMPLEMENTATION-ERROR: error: continuation failed: no @implementation declaration for the selected @interface 'MyClass'; please add one and run the refactoring action again
53+
5054
// RUN: clang-refactor-test perform -action implement-declared-methods -selected=all-methods -continuation-file=%S/Inputs/objcClass.m -query-results=query-all-impl %s -DNO_IMPL | FileCheck --check-prefix=CHECK1 %S/Inputs/objcClass.m
5155
// RUN: clang-refactor-test perform -action implement-declared-methods -selected=all-methods -continuation-file=%S/Inputs/objcClass.m -query-results=query-mix-impl %s -DNO_IMPL -DMIX_IMPL | FileCheck --check-prefix=CHECK2 %S/Inputs/objcClass.m
5256

clang/tools/clang-refactor-test/ClangRefactorTest.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,19 @@ bool performOperation(CXRefactoringAction Action, ArrayRef<const char *> Args,
11091109
/*FileSubstitution=*/opts::initiateAndPerform::ContinuationFile);
11101110
clang_RefactoringContinuation_loadSerializedIndexerQueryResults(
11111111
Continuation, /*Source=*/QueryResults.c_str());
1112+
CXDiagnosticSet Diags =
1113+
clang_RefactoringContinuation_verifyBeforeFinalizing(Continuation);
1114+
if (Diags) {
1115+
llvm::errs() << "error: continuation failed: ";
1116+
for (unsigned I = 0, E = clang_getNumDiagnosticsInSet(Diags); I != E; ++I) {
1117+
CXDiagnostic Diag = clang_getDiagnosticInSet(Diags, I);
1118+
CXString Spelling = clang_getDiagnosticSpelling(Diag);
1119+
errs() << clang_getCString(Spelling) << "\n";
1120+
clang_disposeString(Spelling);
1121+
}
1122+
clang_RefactoringContinuation_dispose(Continuation);
1123+
return true;
1124+
}
11121125
clang_RefactoringContinuation_finalizeEvaluationInInitationTU(Continuation);
11131126
// Load the continuation TU.
11141127
CXTranslationUnit ContinuationTU;

clang/tools/libclang/CRefactor.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,6 +1051,7 @@ class RefactoringDiagnosticConsumer : public DiagnosticConsumer {
10511051
DiagnosticConsumer *PreviousClient;
10521052
std::unique_ptr<DiagnosticConsumer> PreviousClientPtr;
10531053
llvm::SmallVector<StoredDiagnostic, 2> RenameDiagnostics;
1054+
llvm::SmallVector<StoredDiagnostic, 1> ContinuationDiagnostics;
10541055

10551056
public:
10561057
RefactoringDiagnosticConsumer(ASTContext &Context) : Context(Context) {
@@ -1069,17 +1070,24 @@ class RefactoringDiagnosticConsumer : public DiagnosticConsumer {
10691070

10701071
void HandleDiagnostic(DiagnosticsEngine::Level Level,
10711072
const Diagnostic &Info) override {
1072-
if (DiagnosticIDs::getCategoryNumberForDiag(Info.getID()) ==
1073-
diag::DiagCat_Rename_Issue)
1073+
unsigned Cat = DiagnosticIDs::getCategoryNumberForDiag(Info.getID());
1074+
if (Cat == diag::DiagCat_Rename_Issue)
10741075
RenameDiagnostics.push_back(StoredDiagnostic(Level, Info));
1076+
else if (Cat == diag::DiagCat_Refactoring_Continuation_Issue)
1077+
ContinuationDiagnostics.push_back(StoredDiagnostic(Level, Info));
10751078
else
10761079
assert(false && "Unhandled refactoring category");
10771080
}
10781081

10791082
CXDiagnosticSetImpl *createDiags() const {
1080-
if (RenameDiagnostics.empty())
1083+
if (RenameDiagnostics.empty() && ContinuationDiagnostics.empty())
10811084
return nullptr;
1082-
return cxdiag::createStoredDiags(RenameDiagnostics, Context.getLangOpts());
1085+
llvm::SmallVector<StoredDiagnostic, 2> AllDiagnostics;
1086+
for (const auto &D : RenameDiagnostics)
1087+
AllDiagnostics.push_back(D);
1088+
for (const auto &D : ContinuationDiagnostics)
1089+
AllDiagnostics.push_back(D);
1090+
return cxdiag::createStoredDiags(AllDiagnostics, Context.getLangOpts());
10831091
}
10841092

10851093
CXRefactoringActionSetWithDiagnostics createActionSet() const {
@@ -1771,6 +1779,24 @@ CXIndexerQuery clang_RefactoringContinuation_getIndexerQuery(
17711779
return &Wrapper->Queries[Index];
17721780
}
17731781

1782+
CXDiagnosticSet clang_RefactoringContinuation_verifyBeforeFinalizing(
1783+
CXRefactoringContinuation Continuation) {
1784+
if (!Continuation)
1785+
return nullptr;
1786+
auto *Wrapper = static_cast<RefactoringContinuationWrapper *>(Continuation);
1787+
CXTranslationUnit TU = Wrapper->Queries[0].TU;
1788+
ASTUnit *CXXUnit = cxtu::getASTUnit(TU);
1789+
if (!CXXUnit)
1790+
return nullptr;
1791+
ASTContext &Context = CXXUnit->getASTContext();
1792+
RefactoringDiagnosticConsumer DiagConsumer(Context);
1793+
for (const auto &Query : Wrapper->Queries) {
1794+
if (Query.Query->verify(Context))
1795+
break;
1796+
}
1797+
return DiagConsumer.createDiags();
1798+
}
1799+
17741800
void clang_RefactoringContinuation_finalizeEvaluationInInitationTU(
17751801
CXRefactoringContinuation Continuation) {
17761802
if (!Continuation)

clang/tools/libclang/libclang.exports

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ clang_RefactoringResult_dispose
398398
clang_RefactoringContinuation_loadSerializedIndexerQueryResults
399399
clang_RefactoringContinuation_getNumIndexerQueries
400400
clang_RefactoringContinuation_getIndexerQuery
401+
clang_RefactoringContinuation_verifyBeforeFinalizing
401402
clang_RefactoringContinuation_finalizeEvaluationInInitationTU
402403
clang_RefactoringContinuation_continueOperationInTU
403404
clang_RefactoringContinuation_dispose

0 commit comments

Comments
 (0)