This is an archive of the discontinued LLVM Phabricator instance.

Differential D154962

[clangd] Use canonical path as resolved path for includes.
ClosedPublic

Authored by VitaNuo on Jul 11 2023, 6:34 AM.

Details

Reviewers
kadircet
Commits
rG7322f2d5ed54: [clangd] Use canonical path as resolved path for includes.

Diff Detail

Event Timeline

VitaNuo created this revision.Jul 11 2023, 6:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 6:34 AM
Herald added subscribers: kadircet, arphaman. · View Herald Transcript
VitaNuo requested review of this revision.Jul 11 2023, 6:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 6:34 AM
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. · View Herald Transcript
VitaNuo added a reviewer: kadircet.Jul 11 2023, 6:35 AM
kadircet accepted this revision.Jul 11 2023, 6:44 AM
kadircet added inline comments.
clang-tools-extra/clangd/Headers.cpp
57–60

File is already a OptionalFileEntryRef, no need to go to a FileEntry and back into a FileEntryRef

This revision is now accepted and ready to land.Jul 11 2023, 6:44 AM
VitaNuo updated this revision to Diff 539121.Jul 11 2023, 8:32 AM

Address comment.

VitaNuo marked an inline comment as done.Jul 11 2023, 8:34 AM

Thanks.

This revision was landed with ongoing or failed builds.Jul 11 2023, 8:36 AM
Closed by commit rG7322f2d5ed54: [clangd] Use canonical path as resolved path for includes. (authored by VitaNuo). · Explain Why
This revision was automatically updated to reflect the committed changes.
VitaNuo added a commit: rG7322f2d5ed54: [clangd] Use canonical path as resolved path for includes..
Harbormaster completed remote builds in B244492: Diff 539121.Jul 11 2023, 11:43 AM

Revision Contents

PathSize
clang-tools-extra/
clangd/
6 lines

Diff 539128

clang-tools-extra/clangd/Headers.cpp

Show All 15 Lines
#include "clang/Lex/PPCallbacks.h" #include "clang/Lex/PPCallbacks.h"
#include "clang/Lex/Preprocessor.h" #include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Inclusions/HeaderAnalysis.h" #include "clang/Tooling/Inclusions/HeaderAnalysis.h"
#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringRef.h"
#include "llvm/Support/Path.h" #include "llvm/Support/Path.h"
#include <cstring> #include <cstring>
#include <optional> #include <optional>
#include <string>
namespace clang { namespace clang {
namespace clangd { namespace clangd {
class IncludeStructure::RecordHeaders : public PPCallbacks { class IncludeStructure::RecordHeaders : public PPCallbacks {
public: public:
RecordHeaders(const CompilerInstance &CI, IncludeStructure *Out) RecordHeaders(const CompilerInstance &CI, IncludeStructure *Out)
: SM(CI.getSourceManager()), : SM(CI.getSourceManager()),
Show All 16 Lines void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
// Record main-file inclusions (including those mapped from the preamble // Record main-file inclusions (including those mapped from the preamble
// patch). // patch).
if (isInsideMainFile(HashLoc, SM)) { if (isInsideMainFile(HashLoc, SM)) {
Out->MainFileIncludes.emplace_back(); Out->MainFileIncludes.emplace_back();
auto &Inc = Out->MainFileIncludes.back(); auto &Inc = Out->MainFileIncludes.back();
Inc.Written = Inc.Written =
(IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").str(); (IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").str();
Inc.Resolved = Inc.Resolved = std::string(
std::string(File ? File->getFileEntry().tryGetRealPathName() : ""); File ? getCanonicalPath(*File, SM.getFileManager()).value_or("")
: "");
Inc.HashOffset = SM.getFileOffset(HashLoc); Inc.HashOffset = SM.getFileOffset(HashLoc);
kadircetUnsubmitted
Done
ReplyInline Actions
(IsAngled ? "<" + FileName + ">" : "\"" + FileName + "\"").str();
- auto CanonicalPath =
- File ? getCanonicalPath(File->getFileEntry().getLastRef(),
- SM.getFileManager())
- : std::nullopt;
- Inc.Resolved = std::string(CanonicalPath ? *CanonicalPath : "");
+ Inc.Resolved = File ? getCanonicalPath(*File, SM.getFileManager()).value_or("") : "";
Inc.HashOffset = SM.getFileOffset(HashLoc);

File is already a OptionalFileEntryRef, no need to go to a FileEntry and back into a FileEntryRef

kadircet: `File` is already a `OptionalFileEntryRef`, no need to go to a `FileEntry` and back into a…
Inc.HashLine = Inc.HashLine =
SM.getLineNumber(SM.getFileID(HashLoc), Inc.HashOffset) - 1; SM.getLineNumber(SM.getFileID(HashLoc), Inc.HashOffset) - 1;
Inc.FileKind = FileKind; Inc.FileKind = FileKind;
Inc.Directive = IncludeTok.getIdentifierInfo()->getPPKeywordID(); Inc.Directive = IncludeTok.getIdentifierInfo()->getPPKeywordID();
if (File) { if (File) {
IncludeStructure::HeaderID HID = Out->getOrCreateID(*File); IncludeStructure::HeaderID HID = Out->getOrCreateID(*File);
Inc.HeaderID = static_cast<unsigned>(HID); Inc.HeaderID = static_cast<unsigned>(HID);
if (IsAngled) if (IsAngled)
▲ Show 20 Lines • Show All 258 Lines • Show Last 20 Lines