Skip to content
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

[filesystems] Filesystem library edits/issues #1246

Open
burblebee opened this issue Dec 14, 2016 · 6 comments
Open

[filesystems] Filesystem library edits/issues #1246

burblebee opened this issue Dec 14, 2016 · 6 comments
Assignees
Labels
lwg Issue must be reviewed by LWG.

Comments

@burblebee
Copy link
Contributor

As suggested by Jens, these issues with the FS TS are being added as a
single issue so they don't get lost - they should be reviewed after the
NB comments for C++17 are applied.

email from Jens:
The fundamentals TS wording has had ~100 NB comments, and I'd
guess some of your issues are duplicates of these. I'd suggest
to postpone them to post-C++17, since the filesystem NB
resolutions will hit the draft only post-Kona.
(Essentially, this area has a high risk of conflicting
edits.)

Dawn, so that we have this list in a permanent record,
could you open one summary editorial issue with a cut&paste
of your filesystems list?

Jens

The e-mail below contains the list of FS issues as emailed to lwgchair:

After reviewing the applied wording of several LWG motions and making some editorial fixes, I have the remaining list of issues which I'd like LWG and/or the authors to review. While some are definately LWG issues, many of these could probably be resolved editorially, but I wasn't comfortable doing that without guidance or an OK from the author(s), so what remains is where that e-mail correspondence hit a dead end. If folks don't have time to review them now, please open LWG issues for them so that this list won't be forgotten.

Thank you!
-Dawn

Notes on list:

  • WD refers to spec in post-JAX mailing n4582.
  • if section names don't match WD, check FS paper (some changed).
  • => means "replace with".
  • e* 'e' before issue means issue could likely be resolved editorially if reworded or references added.
  • i* 'i' before issue means issue is most likely a LWG issue.

list of issues:


LWG Motion 7 P0218R0 Adopt the File System TS for C++17.
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4100.pdf
http://wiki.edg.com/pub/Wg21jacksonville/StrawPolls/P0218r1.html
Beman bdawes@acm.org Jonathan cxx@kayari.org
NOTE: paragraphs refer to the wording in the WD of the post-JAX mailing;
paragraph numbers in the git master source differ.

e* In 27.10.2.1 of WD [fs.conform.9945],
Inconsistent naming for section. What is 9945?
Section name should be [fs.conform.posix]?

e* In 27.10.4 of WD [fs.definitions],
Should definitions begin with lower case and not end with period as
in [intro.defs]? If so, how to handle defs with multiple sentences?

e* In 27.10.4.18 of WD [fs.def.pathres],
Include version in reference "section 4.11, Pathname resolution".?

e* In 27.10.4.18 of WD [fs.def.pathres],
Example should be a note.

e* In 27.10.4.19 of WD [fs.def.relative-path],
Note should be an example because '.' and '..' are
only otherwise meaningful as filenames in the generic path grammar.
=> \enterexample Pathnames .'' and ..'' are relative paths
on POSIX and Windows systems.
\examplenote

e* In 27.10.4.20 of WD [fs.def.symlink],
Wording in definition is unclear.

i* In 27.10.6 of WD [fs.filesystem.syn],
No details for "size_t hash_value(const path& p) noexcept;"

e* In 27.10.8p2 of WD [class.path],
In "value_type is a typedef for the operating system dependent
encoded character type used to represent pathnames",
is the "character type" the "native encoding" in ([fs.def.native.encode])?
Or is it the character type used to form a NTCTS ([fs.def.ntcts])?
Add reference to clarify the "encoded character type":
=> "character type used to represent pathnames,
where the encoding is the native encoding ([fs.def.native.encode])."
or => "character type used to form a NTCTS ([fs.def.ntcts])"
Same for "encoding"s throughout - which ones are we talking about?

i* In 27.10.8.1p1 of WD [path.generic],
Statement doesn't make sense when talking about a grammar term.
Should a new term "directory separator" be defined for this?
References to grammar term directory-separator throughout should be checked
to see if they should be replaced with this new term.

i* In 27.10.8.2.1p1 of WD [path.fmt.cvt],
bullet 1.1:
Note says "The generic format is acceptable as a native path"
but this is not true on Windows because the grammar includes
'/' as a possible directory-separator
('d1/d2' is not a valid path on Windows).
=> "A native path can be parsed by the generic grammar"

e* In 27.10.8.2.1p4 of WD [path.fmt.cvt],
Wording is inconsistent in use of "regular file" vs. "non-directory file" throughout spec, and "regular file" is never defined.
Should it be defined to be a file that is not a directory, or POSIX S_ISREG?
Inconsistent use of "directory path" .vs "directory" throughout.

i* In 27.10.8.2.2p1 of WD [path.type.cvt],
In "For member function arguments that take character sequences",
these are the functions that take Source arguments, right?
Where is that stated?
What about the non-member functions?

e* In 27.10.8.2.2p2 of WD [path.type.cvt],
what are "source characters"?
=> "any of the characters in the argument or return value"?

e* In 27.10.8.3p1 of WD [path.req],
What's an "effective range"? (used throughout)

e* In 27.10.8.3p2 of WD [path.req],
In "the value types above and their encodings", what encodings?
(was "these value types and ...")

e* In 27.10.8.4.1p6 of WD [path.construct],
Bullet 6.2:
What "current narrow encoding"?
=> "current native narrow encoding~(\ref{fs.def.native.encode})"?
Check uses elsewhere.
Be consistent throughout in uses of
"[current] [native] [narrow/wide] encoding"

e* In 27.10.8.4.5p11 of WD [path.modifiers],
Bullet 11.1: what's "a dot character"?
Be consistent in use of "dot" vs. "period" vs. "'.'",
and use /dot/ when referring to /filename/ in generic grammar.

e* In 27.10.8.4.6 of WD [path.native.obs],
Section is WD [path.native.objs] in paper but [path.native.obs] in WD. Ok?

e* In 27.10.8.4.7p2 of WD [path.generic.obs],
The reference to 27.10.8.1 is to the generic pathname format, but it doesn't say how to "return strings formatted according to" this generic pathname format".
Where is this (re)formatting described? [path.fmt.cvt] doesn't appear to to it.
Same with "reformatted according to" in 27.10.8.4.7p6 of WD [path.generic.obs].

e* In 27.10.8.4.8p2 of WD [path.compare],
What "elements"?

e* In 27.10.8.4.9p5 of WD [path.decompose],
How does one "compose" a path from pathname?
What if /root-path/ is empty?
I think we need to say something like:
=> "A path P, where P is composed from the |pathname| as follows:
If !empty(), let R be the /pathname/ of |pathname|. Then
- if R contains a /root-path/, P is the substring of |pathname|
starting with the first /filename/ in R after the /root-path/,
- otherwise, P is the substring of |pathname|
starting with the first /filename/ in R.
Otherwise, P = path()."

e* In 27.10.8.4.9p9 of WD [path.decompose],
Use grammar term for /dot/ and /dot-dot/:
"if filename() contains a period but does not
consist solely of one or two periods"
=> "if filename() contains a period but is not /dot/ or /dot-dot/"
Same for other Returns throughout paper.
Fix consistency in use of 'period' vs. 'dot' vs. '.' throughout.

e* In 27.10.8.4.9p11 of WD [path.decompose],
What's an "empty path"? Is it the path "" or an empty path object?
Check 3 uses throughout spec.
Also fix inconsistencies with "\tcode{path} object" vs. "path object"

e* In 27.10.8.5p4 of WD [path.itr],
Bullet 4.4: clarify "trailing non-root /slash/ characters".
What about the /preferred-separator/?
Check uses of /slash/ for cases where /directory-separator/ should be used.

i* In 27.10.13p7 of WD [class.directory_iterator],
What are "the current (dot) and parent (dot-dot) directories"?

e* In 27.10.13.1p21 of WD [directory_iterator.members],
"the current depth" is described in a note which is non-normative.
=> "The current depth of the directory tree being traversed, where
the initial directory passed to the constructor is at depth 0,
it's immediate subdirectories are at depth 1, and so forth."

e* In 27.10.15.2p1 of WD [fs.op.canonical],
"an absolute path that has no symbolic link, ".", or ".." elements."
=> "a canonical path (27.10.4.2 [fs.def.canonical.path])"

e* In 27.10.15.3p3 of WD [fs.op.copy],
What is meant by "Before the first use of f and t"?
=> "Let f and t be as follows"?
Bullet 3.1: "if needed"? How to know when auto t is needed?

e* In 27.10.15.3p4 of WD [fs.op.copy],
"error is reported" => "error is handled"?
Errors are handled, not reported.
Same throughout FS.

e* In in 27.10.15.19 of WD [fs.op.is_empty],
|s| is determined by status(p, ec),
but in 27.10.15.13 of WD [fs.op.equivalent],
|s1| and |s2| are determined by status(p1) and status(p2).
Should both be calling the ec variant or not?
Note that both is_empty and equivalent have both the ec and non-ec variants.
(FYI: the details of these funcs have since been rewritten to introduce
s1, s2 and s so they apply to the whole function
since otherwise they have no meaning outside the Effects pnum).

e* In 27.10.15.26p2 of WD [fs.op.permissions],
What are "effective permissions bits"?

i* In 27.10.15.28 of WD [fs.op.remove],
Do Postconditions only apply if the function succeeded?
If not, what if p is a directory? Then p could not
be removed and the Postcondition would not hold.
Also, the Returns would return true if p is a directory (because p exists),
even though p was not removed.

e* In 27.10.15.29p1 of WD [fs.op.remove_all],
Effects assumes p is a directory - must it be?
If not:
=> "Same as remove() ([fs.op.remove]), except that if p is a directory,
also recursively deletes the contents of p."
If so: add \requires.

e* In 27.10.15.33p9 of WD [fs.op.status],
Unclear. How to reword?

e* In 27.10.15.36p6 of WD [fs.op.system_complete],
Why is this an example? Should be a Note?

i* In 27.10.8.4.9p2 of WD [path.decompose],
What was intended in root_directory()?
As worded, root_directory() will only ever return "" or path().
Note that /root-directory/ is /directory-separator/ in the grammar.
So the /root-directory/ of "/bla/f.cpp" is "/", and the /root-directory/ of "bla/f.cpp" is "", and any leading "/" is removed in p3.
p3 also makes no sense since /name/ can't be part of the /root-directory/.

i* In 27.10.8.6.1p1 of WD [path.io],
Should "Equivelent to" be added after "Effects:"?
If "Equivelent to" isn't correct, is there something else that can be added?
It is odd to have "Effects:" with only code - what is the relationship between calling the function call and the code?
See similar "Effects:" throughout the feature.

i* In 27.10.15.9 of WD [fs.op.create_hard_lk],
What should happen if a hardlink is not supported on the OS?
Same for symlinks in 27.10.15.8 of WD [fs.op.create_dir_symlk]
and in 27.10.15.10 of WD [fs.op.create_symlink],
and other unsupported functions.

i* In 27.10.4.2 of WD [fs.def.canonical.path],
Is canonical path supposed to be unique?
If so:
What about c:\path\ vs. \path\ from drive c: on Windows, or UCN paths?
What about substituted drives on Windows?
Suppose we do this on Windows:
subst g: c:\mydir
g:\foo.cpp and c:\mydir\foo.cpp are not canonical.
I believe the wording would treat them as different files, correct?
Please give examples of canonical paths.

i* "resolves" and "resolves to" are never defined
What's it mean to "resolve" a directory, or what a directory "resolves to"?
Suppose we do this on Windows:
subst g: c:\mydir
g:\foo.cpp and c:\mydir\foo.cpp are not canonical.
Do they "resolve" to the same file?
Please define "resolves" and "resolves to", and give examples.

@jensmaurer jensmaurer changed the title Filesystem TS library edits/issues [filesystems] Filesystem library edits/issues Dec 14, 2016
@jensmaurer
Copy link
Member

To clarify: This issue does not refer to the Filesystem TS, but to the [filesystems] section in the working draft of the standard. The issues were discovered while integrating the Filesystems TS therein.

@jensmaurer
Copy link
Member

A paper addressing the NB comments on filesystem is expected to land post-Kona. In order to avoid merge conflicts, the list above should be addressed post-C++17.

@jensmaurer jensmaurer added this to the C++20 milestone Dec 16, 2016
@jensmaurer
Copy link
Member

@burblebee: Could you please make a pass over the post-Kona draft and remove all issues from your list that have been addressed or mooted?

@jensmaurer
Copy link
Member

@burblebee: There have been a lot of updates in the meantime. Could you please make a pass over your list and remove those that have been addressed or mooted?

@zygoloid zygoloid removed this from the C++20 milestone Mar 12, 2020
@zygoloid
Copy link
Member

Removing milestone: we shipped C++17 like this and it's not clear that these are sufficiently problematic to rush a fix for C++20 (nor even what's left to do here).

@jwakely jwakely added the lwg Issue must be reviewed by LWG. label Mar 12, 2020
@jwakely
Copy link
Member

jwakely commented Mar 12, 2020

Adding the lwg label to try and persuade the LWG chair to review what's left to do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lwg Issue must be reviewed by LWG.
Projects
None yet
Development

No branches or pull requests

4 participants