> On Jul 18, 2016, at 3:21 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
>
> On Wed, Feb 26, 2014 at 4:25 PM, Ben Langmuir <blangm...@apple.com
> <mailto:blangm...@apple.com>> wrote:
> Author: benlangmuir
> Date: Wed Feb 26 18:25:12 2014
> New Revision: 202329
>
> URL: http://llvm.org/viewvc/llvm-project?rev=202329&view=rev
> <http://llvm.org/viewvc/llvm-project?rev=202329&view=rev>
> Log:
> Add a 'use-external-names' option to VFS overlay files
>
> When true, sets the name of the file to be the name from
> 'external-contents'. Otherwise, you get the virtual path that the file
> was looked up by. This will not affect any non-virtual paths, or fully
> virtual paths (for which there is no reasonable 'external' name anyway).
>
> The setting is available globally, but can be overriden on a per-file
> basis.
>
> The goal is that this setting will control which path you see in debug
> info, diagnostics, etc. which are sensitive to which path is used. That
> will come in future patches that pass the name through to FileManager.
>
> Modified:
> cfe/trunk/include/clang/Basic/VirtualFileSystem.h
> cfe/trunk/lib/Basic/VirtualFileSystem.cpp
> cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
>
> Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=202329&r1=202328&r2=202329&view=diff
>
> <http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=202329&r1=202328&r2=202329&view=diff>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
> +++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Wed Feb 26 18:25:12 2014
> @@ -29,7 +29,6 @@ namespace vfs {
> /// \brief The result of a \p status operation.
> class Status {
> std::string Name;
> - std::string ExternalName;
> llvm::sys::fs::UniqueID UID;
> llvm::sys::TimeValue MTime;
> uint32_t User;
> @@ -46,16 +45,9 @@ public:
> uint64_t Size, llvm::sys::fs::file_type Type,
> llvm::sys::fs::perms Perms);
>
> - /// \brief Returns the name this status was looked up by.
> + /// \brief Returns the name that should be used for this file or directory.
> StringRef getName() const { return Name; }
> -
> - /// \brief Returns the name to use outside the compiler.
> - ///
> - /// For example, in diagnostics or debug info we should use this name.
> - StringRef getExternalName() const { return ExternalName; }
> -
> void setName(StringRef N) { Name = N; }
> - void setExternalName(StringRef N) { ExternalName = N; }
>
> /// @name Status interface from llvm::sys::fs
> /// @{
>
> Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=202329&r1=202328&r2=202329&view=diff
>
> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=202329&r1=202328&r2=202329&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
> +++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Wed Feb 26 18:25:12 2014
> @@ -35,8 +35,8 @@ Status::Status(const file_status &Status
> Status::Status(StringRef Name, StringRef ExternalName, UniqueID UID,
> sys::TimeValue MTime, uint32_t User, uint32_t Group,
> uint64_t Size, file_type Type, perms Perms)
> - : Name(Name), ExternalName(ExternalName), UID(UID), MTime(MTime),
> - User(User), Group(Group), Size(Size), Type(Type), Perms(Perms) {}
> + : Name(Name), UID(UID), MTime(MTime), User(User), Group(Group),
> Size(Size),
> + Type(Type), Perms(Perms) {}
>
> bool Status::equivalent(const Status &Other) const {
> return getUniqueID() == Other.getUniqueID();
> @@ -145,7 +145,6 @@ ErrorOr<Status> RealFileSystem::status(c
> return EC;
> Status Result(RealStatus);
> Result.setName(Path.str());
> - Result.setExternalName(Path.str());
> return Result;
> }
>
> @@ -253,12 +252,22 @@ public:
> };
>
> class FileEntry : public Entry {
> +public:
> + enum NameKind {
> + NK_NotSet,
> + NK_External,
> + NK_Virtual
> + };
> +private:
> std::string ExternalContentsPath;
> -
> + NameKind UseName;
> public:
> - FileEntry(StringRef Name, StringRef ExternalContentsPath)
> - : Entry(EK_File, Name), ExternalContentsPath(ExternalContentsPath) {}
> + FileEntry(StringRef Name, StringRef ExternalContentsPath, NameKind UseName)
> + : Entry(EK_File, Name), ExternalContentsPath(ExternalContentsPath),
> + UseName(UseName) {}
> StringRef getExternalContentsPath() const { return ExternalContentsPath; }
> + /// \brief whether to use the external path as the name for this file.
> + NameKind useName() const { return UseName; }
> static bool classof(const Entry *E) { return E->getKind() == EK_File; }
> };
>
> @@ -280,6 +289,7 @@ public:
> ///
> /// All configuration options are optional.
> /// 'case-sensitive': <boolean, default=true>
> +/// 'use-external-names': <boolean, default=true>
> ///
> /// Virtual directories are represented as
> /// \verbatim
> @@ -304,6 +314,7 @@ public:
> /// {
> /// 'type': 'file',
> /// 'name': <string>,
> +/// 'use-external-name': <boolean> # Optional
> /// 'external-contents': <path to external file>)
> /// }
> /// \endverbatim
> @@ -324,14 +335,18 @@ class VFSFromYAML : public vfs::FileSyst
> /// \brief Whether to perform case-sensitive comparisons.
> ///
> /// Currently, case-insensitive matching only works correctly with ASCII.
> - bool CaseSensitive; ///< Whether to perform case-sensitive comparisons.
> + bool CaseSensitive;
> +
> + /// \brief Whether to use to use the value of 'external-contents' for the
> + /// names of files. This global value is overridable on a per-file basis.
> + bool UseExternalNames;
> /// @}
>
> friend class VFSFromYAMLParser;
>
> private:
> VFSFromYAML(IntrusiveRefCntPtr<FileSystem> ExternalFS)
> - : ExternalFS(ExternalFS), CaseSensitive(true) {}
> + : ExternalFS(ExternalFS), CaseSensitive(true), UseExternalNames(true)
> {}
>
> /// \brief Looks up \p Path in \c Roots.
> ErrorOr<Entry *> lookupPath(const Twine &Path);
> @@ -446,7 +461,8 @@ class VFSFromYAMLParser {
> KeyStatusPair("name", true),
> KeyStatusPair("type", true),
> KeyStatusPair("contents", false),
> - KeyStatusPair("external-contents", false)
> + KeyStatusPair("external-contents", false),
> + KeyStatusPair("use-external-name", false),
> };
>
> DenseMap<StringRef, KeyStatus> Keys(
> @@ -456,6 +472,7 @@ class VFSFromYAMLParser {
> std::vector<Entry *> EntryArrayContents;
> std::string ExternalContentsPath;
> std::string Name;
> + FileEntry::NameKind UseExternalName = FileEntry::NK_NotSet;
> EntryKind Kind;
>
> for (yaml::MappingNode::iterator I = M->begin(), E = M->end(); I != E;
> @@ -519,6 +536,11 @@ class VFSFromYAMLParser {
> if (!parseScalarString(I->getValue(), Value, Buffer))
> return NULL;
> ExternalContentsPath = Value;
> + } else if (Key == "use-external-name") {
> + bool Val;
> + if (!parseScalarBool(I->getValue(), Val))
> + return NULL;
> + UseExternalName = Val ? FileEntry::NK_External :
> FileEntry::NK_Virtual;
> } else {
> llvm_unreachable("key missing from Keys");
> }
> @@ -535,6 +557,12 @@ class VFSFromYAMLParser {
> if (!checkMissingKeys(N, Keys))
> return NULL;
>
> + // check invalid configuration
> + if (Kind == EK_Directory && UseExternalName != FileEntry::NK_NotSet) {
>
> [Found by Coverity]
>
> If the YAML entry has a name field and a contents field but no kind field, we
> can reach this line with Kind uninitialized, resulting in UB.
This should be caught right above here with “checkMissingKeys”. The field
“type” which we use to set “Kind” is a required field. False positive?
Ben
>
> + error(N, "'use-external-name' is not supported for directories");
> + return NULL;
> + }
> +
> // Remove trailing slash(es)
> StringRef Trimmed(Name);
> while (Trimmed.size() > 1 && sys::path::is_separator(Trimmed.back()))
> @@ -545,7 +573,8 @@ class VFSFromYAMLParser {
> Entry *Result = 0;
> switch (Kind) {
> case EK_File:
> - Result = new FileEntry(LastComponent, llvm_move(ExternalContentsPath));
> + Result = new FileEntry(LastComponent, llvm_move(ExternalContentsPath),
> + UseExternalName);
> break;
> case EK_Directory:
> Result = new DirectoryEntry(LastComponent,
> llvm_move(EntryArrayContents),
> @@ -583,6 +612,7 @@ public:
> KeyStatusPair Fields[] = {
> KeyStatusPair("version", true),
> KeyStatusPair("case-sensitive", false),
> + KeyStatusPair("use-external-names", false),
> KeyStatusPair("roots", true),
> };
>
> @@ -635,6 +665,9 @@ public:
> } else if (Key == "case-sensitive") {
> if (!parseScalarBool(I->getValue(), FS->CaseSensitive))
> return false;
> + } else if (Key == "use-external-names") {
> + if (!parseScalarBool(I->getValue(), FS->UseExternalNames))
> + return false;
> } else {
> llvm_unreachable("key missing from Keys");
> }
> @@ -736,17 +769,15 @@ ErrorOr<Status> VFSFromYAML::status(cons
> std::string PathStr(Path.str());
> if (FileEntry *F = dyn_cast<FileEntry>(*Result)) {
> ErrorOr<Status> S = ExternalFS->status(F->getExternalContentsPath());
> - if (S) {
> - assert(S->getName() == S->getExternalName() &&
> - S->getName() == F->getExternalContentsPath());
> + assert(!S || S->getName() == F->getExternalContentsPath());
> + if (S && (F->useName() == FileEntry::NK_Virtual ||
> + (F->useName() == FileEntry::NK_NotSet && !UseExternalNames)))
> S->setName(PathStr);
> - }
> return S;
> } else { // directory
> DirectoryEntry *DE = cast<DirectoryEntry>(*Result);
> Status S = DE->getStatus();
> S.setName(PathStr);
> - S.setExternalName(PathStr);
> return S;
> }
> }
>
> Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=202329&r1=202328&r2=202329&view=diff
>
> <http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=202329&r1=202328&r2=202329&view=diff>
> ==============================================================================
> --- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
> +++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Wed Feb 26 18:25:12
> 2014
> @@ -290,8 +290,7 @@ TEST_F(VFSFromYAMLTest, MappedFiles) {
> // file
> ErrorOr<vfs::Status> S = O->status("/file1");
> ASSERT_EQ(errc::success, S.getError());
> - EXPECT_EQ("/file1", S->getName());
> - EXPECT_EQ("/foo/bar/a", S->getExternalName());
> + EXPECT_EQ("/foo/bar/a", S->getName());
>
> ErrorOr<vfs::Status> SLower = O->status("/foo/bar/a");
> EXPECT_EQ("/foo/bar/a", SLower->getName());
> @@ -467,6 +466,57 @@ TEST_F(VFSFromYAMLTest, IllegalVFSFile)
> EXPECT_EQ(24, NumDiagnostics);
> }
>
> +TEST_F(VFSFromYAMLTest, UseExternalName) {
> + IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
> + Lower->addRegularFile("/external/file");
> +
> + IntrusiveRefCntPtr<vfs::FileSystem> FS = getFromYAMLString(
> + "{ 'roots': [\n"
> + " { 'type': 'file', 'name': '/A',\n"
> + " 'external-contents': '/external/file'\n"
> + " },\n"
> + " { 'type': 'file', 'name': '/B',\n"
> + " 'use-external-name': true,\n"
> + " 'external-contents': '/external/file'\n"
> + " },\n"
> + " { 'type': 'file', 'name': '/C',\n"
> + " 'use-external-name': false,\n"
> + " 'external-contents': '/external/file'\n"
> + " }\n"
> + "] }", Lower);
> + ASSERT_TRUE(NULL != FS.getPtr());
> +
> + // default true
> + EXPECT_EQ("/external/file", FS->status("/A")->getName());
> + // explicit
> + EXPECT_EQ("/external/file", FS->status("/B")->getName());
> + EXPECT_EQ("/C", FS->status("/C")->getName());
> +
> + // global configuration
> + FS = getFromYAMLString(
> + "{ 'use-external-names': false,\n"
> + " 'roots': [\n"
> + " { 'type': 'file', 'name': '/A',\n"
> + " 'external-contents': '/external/file'\n"
> + " },\n"
> + " { 'type': 'file', 'name': '/B',\n"
> + " 'use-external-name': true,\n"
> + " 'external-contents': '/external/file'\n"
> + " },\n"
> + " { 'type': 'file', 'name': '/C',\n"
> + " 'use-external-name': false,\n"
> + " 'external-contents': '/external/file'\n"
> + " }\n"
> + "] }", Lower);
> + ASSERT_TRUE(NULL != FS.getPtr());
> +
> + // default
> + EXPECT_EQ("/A", FS->status("/A")->getName());
> + // explicit
> + EXPECT_EQ("/external/file", FS->status("/B")->getName());
> + EXPECT_EQ("/C", FS->status("/C")->getName());
> +}
> +
> TEST_F(VFSFromYAMLTest, MultiComponentPath) {
> IntrusiveRefCntPtr<DummyFileSystem> Lower(new DummyFileSystem());
> Lower->addRegularFile("/other");
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-comm...@cs.uiuc.edu <mailto:cfe-comm...@cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
