Szelethus added a comment.
We had a meeting outside phabricator, here is the gist of it:
- It'd be better to have your initially proposed schrödinger-like `NoteTag`s.
- The intent is to emit a warning for each of the error states in
`StreamState`. It seems to me that this is not what we're doing now, but,
similarly to adding warnings fro FERROR and being able to clear the
indeterminate file position indicator state, would truly display how `NoteTag`s
might change their message depending on the `BugType`.
- There are numerous discussions to be had on how to handle `fseek`, how much
do FERROR and the file position indicator imply one another, but for this
patch, adding debug functions such as `void
StreamCheckerTest_remove_indeterminate_file_pos_tag(FILE *)` or `void
StreamCheckerTest_warn_if_in_ferror(FILE *)` would allow as to test the
functionality added by this patch, and land it.
As such, I'd like if you implemented the above mentioned functions with their
expected functionality. I'd like to test cases where the very same function
call gets a different note message depending on the `BugType`:
void fseek_caused_feof() {
FILE *F;
char Buf[10];
F = fopen("foo1.c", "r");
if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is
non-null}}
return;
fseek(F, 1, SEEK_SET); // expected-note {{Stream reaches end-of-file here}}
<============= (*)
if (ferror(F)) // expected-note {{Assuming the error flag is not set on the
stream}}
// expected-note@-1 {{Taking false branch}}
return;
StreamCheckerTest_remove_indeterminate_file_pos_tag(F);
fread(Buf, 1, 1, F); // expected-warning{{Read function called when stream
is in EOF state. Function has no effect}}
// expected-note@-1{{Read function called when stream is in EOF state.
Function has no effect}}
fclose(F);
}
void fseek_caused_ferror() {
FILE *F;
char Buf[10];
F = fopen("foo1.c", "r");
if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is
non-null}}
return;
fseek(F, 1, SEEK_SET); // expected-note {{Stream has its error flag set}}
<============= (*)
if (feof(F)) // expected-note {{Assuming the end-of-file flag is not set on
the stream}}
// expected-note@-1 {{Taking false branch}}
return;
StreamCheckerTest_remove_indeterminate_file_pos_tag(F);
StreamCheckerTest_warn_if_in_ferror(F); // expected-warning{{Read function
called when stream is in FERROR state.}}
// expected-note@-1{Read function called when stream is in FERROR state.}
fclose(F);
}
void fseek_caused_indeterminate_file_pos() {
FILE *F;
char Buf[10];
F = fopen("foo1.c", "r");
if (!F) // expected-note {{Taking false branch}} expected-note {{'F' is
non-null}}
return;
fseek(F, 1, SEEK_SET); // expected-note {{Stream operation leaves the file
position indicator indeterminate}} <============= (*)
clearerr(F);
fread(Buf, 1, 1, F); // expected-warning{{File position of the stream might
be 'indeterminate' after a failed operation. Can cause undefined behavior}}
// expected-note@-1{{File position of the stream might be 'indeterminate'
after a failed operation. Can cause undefined behavior}}
fclose(F);
}
Maybe some others in the spirit of these 3.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106262/new/
https://reviews.llvm.org/D106262
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
