Skip to content

Commit 8bc2416

Browse files
author
Eric Liu
committedMar 21, 2017
[change-namespace] avoid adding leading '::' when possible.
Summary: When changing namespaces, the tool adds leading "::" to references that need to be fully-qualified, which would affect readability. We avoid adding "::" when the symbol name does not conflict with the new namespace name. For example, a symbol name "na::nb::X" conflicts with "ns::na" since it would be resolved to "ns::na::nb::X" in the new namespace. Reviewers: hokein Reviewed By: hokein Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D30493 llvm-svn: 298363
1 parent 3b939d0 commit 8bc2416

File tree

2 files changed

+124
-62
lines changed

2 files changed

+124
-62
lines changed
 

Diff for: ‎clang-tools-extra/change-namespace/ChangeNamespace.cpp

+38-17
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ joinNamespaces(const llvm::SmallVectorImpl<StringRef> &Namespaces) {
2828
return Result;
2929
}
3030

31+
// Given "a::b::c", returns {"a", "b", "c"}.
32+
llvm::SmallVector<llvm::StringRef, 4> splitSymbolName(llvm::StringRef Name) {
33+
llvm::SmallVector<llvm::StringRef, 4> Splitted;
34+
Name.split(Splitted, "::", /*MaxSplit=*/-1,
35+
/*KeepEmpty=*/false);
36+
return Splitted;
37+
}
38+
3139
SourceLocation startLocationForType(TypeLoc TLoc) {
3240
// For elaborated types (e.g. `struct a::A`) we want the portion after the
3341
// `struct` but including the namespace qualifier, `a::`.
@@ -68,9 +76,7 @@ const NamespaceDecl *getOuterNamespace(const NamespaceDecl *InnerNs,
6876
return nullptr;
6977
const auto *CurrentContext = llvm::cast<DeclContext>(InnerNs);
7078
const auto *CurrentNs = InnerNs;
71-
llvm::SmallVector<llvm::StringRef, 4> PartialNsNameSplitted;
72-
PartialNsName.split(PartialNsNameSplitted, "::", /*MaxSplit=*/-1,
73-
/*KeepEmpty=*/false);
79+
auto PartialNsNameSplitted = splitSymbolName(PartialNsName);
7480
while (!PartialNsNameSplitted.empty()) {
7581
// Get the inner-most namespace in CurrentContext.
7682
while (CurrentContext && !llvm::isa<NamespaceDecl>(CurrentContext))
@@ -208,12 +214,8 @@ std::string getShortestQualifiedNameInNamespace(llvm::StringRef DeclName,
208214
if (DeclName.find(':') == llvm::StringRef::npos)
209215
return DeclName;
210216

211-
llvm::SmallVector<llvm::StringRef, 4> NsNameSplitted;
212-
NsName.split(NsNameSplitted, "::", /*MaxSplit=*/-1,
213-
/*KeepEmpty=*/false);
214-
llvm::SmallVector<llvm::StringRef, 4> DeclNsSplitted;
215-
DeclName.split(DeclNsSplitted, "::", /*MaxSplit=*/-1,
216-
/*KeepEmpty=*/false);
217+
auto NsNameSplitted = splitSymbolName(NsName);
218+
auto DeclNsSplitted = splitSymbolName(DeclName);
217219
llvm::StringRef UnqualifiedDeclName = DeclNsSplitted.pop_back_val();
218220
// If the Decl is in global namespace, there is no need to shorten it.
219221
if (DeclNsSplitted.empty())
@@ -249,9 +251,7 @@ std::string getShortestQualifiedNameInNamespace(llvm::StringRef DeclName,
249251
std::string wrapCodeInNamespace(StringRef NestedNs, std::string Code) {
250252
if (Code.back() != '\n')
251253
Code += "\n";
252-
llvm::SmallVector<StringRef, 4> NsSplitted;
253-
NestedNs.split(NsSplitted, "::", /*MaxSplit=*/-1,
254-
/*KeepEmpty=*/false);
254+
auto NsSplitted = splitSymbolName(NestedNs);
255255
while (!NsSplitted.empty()) {
256256
// FIXME: consider code style for comments.
257257
Code = ("namespace " + NsSplitted.back() + " {\n" + Code +
@@ -282,6 +282,28 @@ bool isDeclVisibleAtLocation(const SourceManager &SM, const Decl *D,
282282
isNestedDeclContext(DeclCtx, D->getDeclContext()));
283283
}
284284

285+
// Given a qualified symbol name, returns true if the symbol will be
286+
// incorrectly qualified without leading "::".
287+
bool conflictInNamespace(llvm::StringRef QualifiedSymbol,
288+
llvm::StringRef Namespace) {
289+
auto SymbolSplitted = splitSymbolName(QualifiedSymbol.trim(":"));
290+
assert(!SymbolSplitted.empty());
291+
SymbolSplitted.pop_back(); // We are only interested in namespaces.
292+
293+
if (SymbolSplitted.size() > 1 && !Namespace.empty()) {
294+
auto NsSplitted = splitSymbolName(Namespace.trim(":"));
295+
assert(!NsSplitted.empty());
296+
// We do not check the outermost namespace since it would not be a conflict
297+
// if it equals to the symbol's outermost namespace and the symbol name
298+
// would have been shortened.
299+
for (auto I = NsSplitted.begin() + 1, E = NsSplitted.end(); I != E; ++I) {
300+
if (*I == SymbolSplitted.front())
301+
return true;
302+
}
303+
}
304+
return false;
305+
}
306+
285307
AST_MATCHER(EnumDecl, isScoped) {
286308
return Node.isScoped();
287309
}
@@ -306,10 +328,8 @@ ChangeNamespaceTool::ChangeNamespaceTool(
306328
OldNamespace(OldNs.ltrim(':')), NewNamespace(NewNs.ltrim(':')),
307329
FilePattern(FilePattern), FilePatternRE(FilePattern) {
308330
FileToReplacements->clear();
309-
llvm::SmallVector<llvm::StringRef, 4> OldNsSplitted;
310-
llvm::SmallVector<llvm::StringRef, 4> NewNsSplitted;
311-
llvm::StringRef(OldNamespace).split(OldNsSplitted, "::");
312-
llvm::StringRef(NewNamespace).split(NewNsSplitted, "::");
331+
auto OldNsSplitted = splitSymbolName(OldNamespace);
332+
auto NewNsSplitted = splitSymbolName(NewNamespace);
313333
// Calculates `DiffOldNamespace` and `DiffNewNamespace`.
314334
while (!OldNsSplitted.empty() && !NewNsSplitted.empty() &&
315335
OldNsSplitted.front() == NewNsSplitted.front()) {
@@ -825,7 +845,8 @@ void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext(
825845
return;
826846
// If the reference need to be fully-qualified, add a leading "::" unless
827847
// NewNamespace is the global namespace.
828-
if (ReplaceName == FromDeclName && !NewNamespace.empty())
848+
if (ReplaceName == FromDeclName && !NewNamespace.empty() &&
849+
conflictInNamespace(ReplaceName, NewNamespace))
829850
ReplaceName = "::" + ReplaceName;
830851
addReplacementOrDie(Start, End, ReplaceName, *Result.SourceManager,
831852
&FileToReplacements);

0 commit comments

Comments
 (0)
Please sign in to comment.