Skip to content

[SystemZ][z/OS] Propagate IsText parameter to open text files as text #107906

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/FS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ PreambleFileStatusCache::getProducingFS(
: ProxyFileSystem(std::move(FS)), StatCache(StatCache) {}

llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const llvm::Twine &Path) override {
auto File = getUnderlyingFS().openFileForRead(Path);
openFileForRead(const llvm::Twine &Path, bool IsText = true) override {
auto File = getUnderlyingFS().openFileForRead(Path, IsText);
if (!File || !*File)
return File;
// Eagerly stat opened file, as the followup `status` call on the file
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/Preamble.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -479,9 +479,9 @@ class TimerFS : public llvm::vfs::ProxyFileSystem {
: ProxyFileSystem(std::move(FS)) {}

llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const llvm::Twine &Path) override {
openFileForRead(const llvm::Twine &Path, bool IsText = true) override {
WallTimerRegion T(Timer);
auto FileOr = getUnderlyingFS().openFileForRead(Path);
auto FileOr = getUnderlyingFS().openFileForRead(Path, IsText);
if (!FileOr)
return FileOr;
return std::make_unique<TimerFile>(Timer, std::move(FileOr.get()));
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/support/ThreadsafeFS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class VolatileFileSystem : public llvm::vfs::ProxyFileSystem {
: ProxyFileSystem(std::move(FS)) {}

llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const llvm::Twine &InPath) override {
openFileForRead(const llvm::Twine &InPath, bool IsText = true) override {
llvm::SmallString<128> Path;
InPath.toVector(Path);

Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/unittests/ClangdTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1010,7 +1010,7 @@ TEST(ClangdTests, PreambleVFSStatCache) {
: ProxyFileSystem(std::move(FS)), CountStats(CountStats) {}

llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const Twine &Path) override {
openFileForRead(const Twine &Path, bool IsText = true) override {
++CountStats[llvm::sys::path::filename(Path.str())];
return ProxyFileSystem::openFileForRead(Path);
}
Expand Down
8 changes: 4 additions & 4 deletions clang/include/clang/Basic/FileManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -286,21 +286,21 @@ class FileManager : public RefCountedBase<FileManager> {
/// MemoryBuffer if successful, otherwise returning null.
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
getBufferForFile(FileEntryRef Entry, bool isVolatile = false,
bool RequiresNullTerminator = true,
bool RequiresNullTerminator = true, bool IsText = true,
std::optional<int64_t> MaybeLimit = std::nullopt);
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
getBufferForFile(StringRef Filename, bool isVolatile = false,
bool RequiresNullTerminator = true,
bool RequiresNullTerminator = true, bool IsText = true,
std::optional<int64_t> MaybeLimit = std::nullopt) const {
return getBufferForFileImpl(Filename,
/*FileSize=*/(MaybeLimit ? *MaybeLimit : -1),
isVolatile, RequiresNullTerminator);
isVolatile, RequiresNullTerminator, IsText);
}

private:
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
getBufferForFileImpl(StringRef Filename, int64_t FileSize, bool isVolatile,
bool RequiresNullTerminator) const;
bool RequiresNullTerminator, bool IsText) const;

DirectoryEntry *&getRealDirEntry(const llvm::vfs::Status &Status);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ class DependencyScanningWorkerFilesystem

llvm::ErrorOr<llvm::vfs::Status> status(const Twine &Path) override;
llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const Twine &Path) override;
openFileForRead(const Twine &Path, bool IsText = true) override;

std::error_code getRealPath(const Twine &Path,
SmallVectorImpl<char> &Output) override;
Expand Down
12 changes: 6 additions & 6 deletions clang/lib/Basic/FileManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ void FileManager::fillRealPathName(FileEntry *UFE, llvm::StringRef FileName) {

llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
FileManager::getBufferForFile(FileEntryRef FE, bool isVolatile,
bool RequiresNullTerminator,
bool RequiresNullTerminator, bool IsText,
std::optional<int64_t> MaybeLimit) {
const FileEntry *Entry = &FE.getFileEntry();
// If the content is living on the file entry, return a reference to it.
Expand Down Expand Up @@ -558,21 +558,21 @@ FileManager::getBufferForFile(FileEntryRef FE, bool isVolatile,

// Otherwise, open the file.
return getBufferForFileImpl(Filename, FileSize, isVolatile,
RequiresNullTerminator);
RequiresNullTerminator, IsText);
}

llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
FileManager::getBufferForFileImpl(StringRef Filename, int64_t FileSize,
bool isVolatile,
bool RequiresNullTerminator) const {
bool isVolatile, bool RequiresNullTerminator,
bool IsText) const {
if (FileSystemOpts.WorkingDir.empty())
return FS->getBufferForFile(Filename, FileSize, RequiresNullTerminator,
isVolatile);
isVolatile, IsText);

SmallString<128> FilePath(Filename);
FixupRelativePath(FilePath);
return FS->getBufferForFile(FilePath, FileSize, RequiresNullTerminator,
isVolatile);
isVolatile, IsText);
}

/// getStatValue - Get the 'stat' information for the specified path,
Expand Down
14 changes: 12 additions & 2 deletions clang/lib/Basic/SourceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,18 @@ ContentCache::getBufferOrNone(DiagnosticsEngine &Diag, FileManager &FM,
// Start with the assumption that the buffer is invalid to simplify early
// return paths.
IsBufferInvalid = true;

auto BufferOrError = FM.getBufferForFile(*ContentsEntry, IsFileVolatile);
bool IsText = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in all other code paths, IsText always defaults to true, but you're making it default to false here, is this intentional ?


#ifdef __MVS__
// If the file is tagged with a text ccsid, it may require autoconversion.
llvm::ErrorOr<bool> IsFileText =
llvm::iszOSTextFile(ContentsEntry->getName().data());
if (IsFileText)
IsText = *IsFileText;
#endif

auto BufferOrError = FM.getBufferForFile(
*ContentsEntry, IsFileVolatile, /*RequiresNullTerminator=*/true, IsText);

// If we were unable to open the file, then we are in an inconsistent
// situation where the content cache referenced a file which no longer
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5322,7 +5322,8 @@ std::string ASTReader::getOriginalSourceFile(
const PCHContainerReader &PCHContainerRdr, DiagnosticsEngine &Diags) {
// Open the AST file.
auto Buffer = FileMgr.getBufferForFile(ASTFileName, /*IsVolatile=*/false,
/*RequiresNullTerminator=*/false);
/*RequiresNullTerminator=*/false,
/*IsText=*/true);
if (!Buffer) {
Diags.Report(diag::err_fe_unable_to_read_pch_file)
<< ASTFileName << Buffer.getError().message();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,12 +353,13 @@ DepScanFile::create(EntryRef Entry) {
}

llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
DependencyScanningWorkerFilesystem::openFileForRead(const Twine &Path) {
DependencyScanningWorkerFilesystem::openFileForRead(const Twine &Path,
bool IsText) {
SmallString<256> OwnedFilename;
StringRef Filename = Path.toStringRef(OwnedFilename);

if (shouldBypass(Filename))
return getUnderlyingFS().openFileForRead(Path);
return getUnderlyingFS().openFileForRead(Path, IsText);

llvm::ErrorOr<EntryRef> Result = getOrCreateFileSystemEntry(Filename);
if (!Result)
Expand Down
4 changes: 2 additions & 2 deletions clang/unittests/Driver/DistroTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,9 @@ TEST(DistroTest, DetectWindowsAndCrossCompile) {
}

llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const llvm::Twine &Path) override {
openFileForRead(const llvm::Twine &Path, bool IsText = true) override {
++Count;
return llvm::vfs::ProxyFileSystem::openFileForRead(Path);
return llvm::vfs::ProxyFileSystem::openFileForRead(Path, IsText);
}

unsigned Count{};
Expand Down
2 changes: 1 addition & 1 deletion clang/unittests/Driver/ToolChainTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ struct FileSystemWithError : public llvm::vfs::FileSystem {
return std::make_error_code(std::errc::no_such_file_or_directory);
}
llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const Twine &Path) override {
openFileForRead(const Twine &Path, bool IsText = true) override {
return std::make_error_code(std::errc::permission_denied);
}
llvm::vfs::directory_iterator dir_begin(const Twine &Dir,
Expand Down
6 changes: 3 additions & 3 deletions clang/unittests/Frontend/PCHPreambleTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ class ReadCountingInMemoryFileSystem : public vfs::InMemoryFileSystem
std::map<std::string, unsigned> ReadCounts;

public:
ErrorOr<std::unique_ptr<vfs::File>> openFileForRead(const Twine &Path) override
{
ErrorOr<std::unique_ptr<vfs::File>>
openFileForRead(const Twine &Path, bool IsText = true) override {
++ReadCounts[Canonicalize(Path)];
return InMemoryFileSystem::openFileForRead(Path);
return InMemoryFileSystem::openFileForRead(Path, IsText);
}

unsigned GetReadCount(const Twine &Path) const
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,9 @@ TEST(DependencyScanner, ScanDepsWithModuleLookup) {
}

llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const Twine &Path) override {
openFileForRead(const Twine &Path, bool IsText = true) override {
ReadFiles.push_back(Path.str());
return ProxyFileSystem::openFileForRead(Path);
return ProxyFileSystem::openFileForRead(Path, IsText);
}
};

Expand Down
2 changes: 1 addition & 1 deletion lldb/unittests/Host/FileSystemTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class DummyFileSystem : public vfs::FileSystem {
return I->second;
}
ErrorOr<std::unique_ptr<vfs::File>>
openFileForRead(const Twine &Path) override {
openFileForRead(const Twine &Path, bool Text = true) override {
auto S = status(Path);
if (S)
return std::unique_ptr<vfs::File>(new DummyFile{*S});
Expand Down
2 changes: 1 addition & 1 deletion lldb/unittests/Utility/MockSymlinkFileSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class MockSymlinkFileSystem : public llvm::vfs::FileSystem {
return llvm::errc::operation_not_permitted;
}
llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const llvm::Twine &Path) override {
openFileForRead(const llvm::Twine &Path, bool IsText = true) override {
return llvm::errc::operation_not_permitted;
}
llvm::vfs::directory_iterator dir_begin(const llvm::Twine &Dir,
Expand Down
6 changes: 6 additions & 0 deletions llvm/include/llvm/Support/AutoConvert.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ std::error_code restorezOSStdHandleAutoConversion(int FD);
/// \brief Set the tag information for a file descriptor.
std::error_code setzOSFileTag(int FD, int CCSID, bool Text);

// Get the the tag ccsid for a file name or a file descriptor.
ErrorOr<__ccsid_t> getzOSFileTag(const char *FileName, const int FD = -1);

// Query the file tag to determine if the file is a text file.
ErrorOr<bool> iszOSTextFile(const char *Filename, const int FD = -1);

} // namespace llvm
#endif // __cplusplus

Expand Down
21 changes: 12 additions & 9 deletions llvm/include/llvm/Support/VirtualFileSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,13 +273,14 @@ class FileSystem : public llvm::ThreadSafeRefCountedBase<FileSystem>,

/// Get a \p File object for the file at \p Path, if one exists.
virtual llvm::ErrorOr<std::unique_ptr<File>>
openFileForRead(const Twine &Path) = 0;
openFileForRead(const Twine &Path, bool IsText = true) = 0;

/// This is a convenience method that opens a file, gets its content and then
/// closes the file.
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
getBufferForFile(const Twine &Name, int64_t FileSize = -1,
bool RequiresNullTerminator = true, bool IsVolatile = false);
bool RequiresNullTerminator = true, bool IsVolatile = false,
bool IsText = true);

/// Get a directory_iterator for \p Dir.
/// \note The 'end' iterator is directory_iterator().
Expand Down Expand Up @@ -392,7 +393,7 @@ class OverlayFileSystem : public RTTIExtends<OverlayFileSystem, FileSystem> {
llvm::ErrorOr<Status> status(const Twine &Path) override;
bool exists(const Twine &Path) override;
llvm::ErrorOr<std::unique_ptr<File>>
openFileForRead(const Twine &Path) override;
openFileForRead(const Twine &Path, bool IsText = true) override;
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;
llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override;
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
Expand Down Expand Up @@ -446,8 +447,8 @@ class ProxyFileSystem : public RTTIExtends<ProxyFileSystem, FileSystem> {
}
bool exists(const Twine &Path) override { return FS->exists(Path); }
llvm::ErrorOr<std::unique_ptr<File>>
openFileForRead(const Twine &Path) override {
return FS->openFileForRead(Path);
openFileForRead(const Twine &Path, bool IsText = true) override {
return FS->openFileForRead(Path, IsText);
}
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override {
return FS->dir_begin(Dir, EC);
Expand Down Expand Up @@ -615,7 +616,7 @@ class InMemoryFileSystem : public RTTIExtends<InMemoryFileSystem, FileSystem> {

llvm::ErrorOr<Status> status(const Twine &Path) override;
llvm::ErrorOr<std::unique_ptr<File>>
openFileForRead(const Twine &Path) override;
openFileForRead(const Twine &Path, bool IsText = true) override;
directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override;

llvm::ErrorOr<std::string> getCurrentWorkingDirectory() const override {
Expand Down Expand Up @@ -1051,7 +1052,8 @@ class RedirectingFileSystem

ErrorOr<Status> status(const Twine &Path) override;
bool exists(const Twine &Path) override;
ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override;
ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path,
bool IsText = true) override;

std::error_code getRealPath(const Twine &Path,
SmallVectorImpl<char> &Output) override;
Expand Down Expand Up @@ -1155,9 +1157,10 @@ class TracingFileSystem
return ProxyFileSystem::status(Path);
}

ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path) override {
ErrorOr<std::unique_ptr<File>> openFileForRead(const Twine &Path,
bool IsText = true) override {
++NumOpenFileForReadCalls;
return ProxyFileSystem::openFileForRead(Path);
return ProxyFileSystem::openFileForRead(Path, IsText);
}

directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) override {
Expand Down
26 changes: 26 additions & 0 deletions llvm/lib/Support/AutoConvert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,4 +116,30 @@ std::error_code llvm::setzOSFileTag(int FD, int CCSID, bool Text) {
return std::error_code();
}

ErrorOr<__ccsid_t> llvm::getzOSFileTag(const char *FileName, const int FD) {
// If we have a file descriptor, use it to find out file tagging. Otherwise we
// need to use stat() with the file path.
if (FD != -1) {
struct f_cnvrt Query = {
QUERYCVT, // cvtcmd
0, // pccsid
0, // fccsid
};
if (fcntl(FD, F_CONTROL_CVT, &Query) == -1)
return std::error_code(errno, std::generic_category());
return Query.fccsid;
}
struct stat Attr;
if (stat(FileName, &Attr) == -1)
return std::error_code(errno, std::generic_category());
return Attr.st_tag.ft_ccsid;
}

ErrorOr<bool> llvm::iszOSTextFile(const char *Filename, const int FD) {
ErrorOr<__ccsid_t> Ccsid = getzOSFileTag(Filename, FD);
if (std::error_code EC = Ccsid.getError())
return EC;
return *Ccsid != FT_BINARY;
}

#endif // __MVS__
4 changes: 2 additions & 2 deletions llvm/lib/Support/FileCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ class FileCollectorFileSystem : public vfs::FileSystem {
}

llvm::ErrorOr<std::unique_ptr<llvm::vfs::File>>
openFileForRead(const Twine &Path) override {
auto Result = FS->openFileForRead(Path);
openFileForRead(const Twine &Path, bool IsText) override {
auto Result = FS->openFileForRead(Path, IsText);
if (Result && *Result)
Collector->addFile(Path);
return Result;
Expand Down
Loading
Loading