[PATCH] D117549: [clangd] Sort targets before printing for tests
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 18 02:06:48 PST 2022
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman, mgrang.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Targets are not necessarily inserted in the order they appear in source
code. For example we could traverse overload sets, or selectively insert
template patterns after all other decls.
So order the targets before printing to make sure tests are not dependent on
such implementation details. We can also do it in production, but that might be
wasteful as we haven't seen any complaints in the wild around these orderings
yet.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D117549
Files:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1284,10 +1284,6 @@
"1: targets = {vector}\n"
"2: targets = {x}\n"},
// Handle UnresolvedLookupExpr.
-// FIXME
-// This case fails when expensive checks are enabled.
-// Seems like the order of ns1::func and ns2::func isn't defined.
-#ifndef EXPENSIVE_CHECKS
{R"cpp(
namespace ns1 { void func(char*); }
namespace ns2 { void func(int*); }
@@ -1301,7 +1297,6 @@
)cpp",
"0: targets = {ns1::func, ns2::func}\n"
"1: targets = {t}\n"},
-#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
struct X {
Index: clang-tools-extra/clangd/FindTarget.cpp
===================================================================
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -33,6 +33,7 @@
#include "clang/Basic/LangOptions.h"
#include "clang/Basic/OperatorKinds.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Basic/Specifiers.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
@@ -1170,6 +1171,13 @@
// note we cannot print R.NameLoc without a source manager.
OS << "targets = {";
bool First = true;
+ if (!R.Targets.empty()) {
+ const auto &SM = R.Targets.front()->getASTContext().getSourceManager();
+ llvm::sort(R.Targets, [&SM](const NamedDecl *LHS, const NamedDecl *RHS) {
+ return SM.isBeforeInTranslationUnit(LHS->getLocation(),
+ RHS->getLocation());
+ });
+ }
for (const NamedDecl *T : R.Targets) {
if (!First)
OS << ", ";
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117549.400774.patch
Type: text/x-patch
Size: 1870 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220118/4964966f/attachment.bin>
More information about the cfe-commits
mailing list