dexonsmith marked an inline comment as done.
dexonsmith added a comment.
Thanks for the review!
================
Comment at: clang/lib/Basic/SourceManager.cpp:167
+ // (which may have come from a stat cache).
+ if (ContentsEntry->isNamedPipe()) {
+ // Check the buffer size directly if this is a named pipe.
----------------
arphaman wrote:
> Would it make sense to do the `diagnoseFileTooLarge` check for both files and
> named pipes here unconditionally instead of having two separate checks? It
> appears that it shouldn't really matter if we check the ContentsEntry or the
> Buffer size for regular files, as their sizes are expected to match anyway.
To explain, the reason I split it was to be able to return faster (don't bother
loading the buffer) if it's going to be the wrong size... but an `mmap` is
cheap and the error path isn't worth micro-optimizing.
I agree it's cleaner to just move it. I'll update to do that.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92531/new/
https://reviews.llvm.org/D92531
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
