kadircet wrote:
Sorry for not responding here for a while.
> These change mirror interface to getFileOrSTDIN() which has a IsText
> parameter. This does touch a number of places but I feel the changes are in
> line with the rest of the file I/O functions in llvm.
I think there's a huge differ
https://github.com/kadircet requested changes to this pull request.
sorry this is same as https://github.com/llvm/llvm-project/pull/107906 (with a
bigger impact radius, as you're also changing getBufferForFile) and doesn't
address any of the issues mention about explaining the semantics of `IsT
kadircet wrote:
i think limiting this to `RealFileSystem::openFileForRead` LG, but can we make
sure `IsText` defaults to `false` and we can only set it in `#ifdef __MVS__`
block to make sure this is really a no-op for other platforms. e.g.:
```cppp
auto OpenFlags = sys::fs::OF_None;
#ifdef __MV
kadircet wrote:
thanks a lot for the swift response!
I can see how none of these implementations are not using those flags _today_,
but we're changing the observable behavior for them as well, and if some of
those implementations decides to give meaning to these flags, it might be
impossible
kadircet wrote:
i've also just noticed
https://github.com/llvm/llvm-project/commit/3b3accb598ec87a6a30b0e18ded06071030bb78f,
which seem to be pushed without review and any tests, changing behavior more
in a non-obvious way. Please not that logic you have in:
```
Expected FDOrErr = sys::fs::o
@@ -323,10 +325,11 @@ ErrorOr RealFileSystem::status(const Twine &Path)
{
}
ErrorOr>
-RealFileSystem::openFileForRead(const Twine &Name) {
+RealFileSystem::openFileForRead(const Twine &Name, bool IsText) {
SmallString<256> RealName, Storage;
Expected FDOrErr = sys::fs::
@@ -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, I
https://github.com/kadircet commented:
hi sorry for missing this during the review time, i believe this change is
changing some of the core support library interfaces in a way that's not
justified.
this is an interface implemented by quite a lot of both upstream and downstream
clients, but th
https://github.com/kadircet edited
https://github.com/llvm/llvm-project/pull/107906
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
It was reverted in base repo, https://reviews.llvm.org/rL346500
On Fri, Nov 9, 2018 at 5:09 PM Davide Italiano
wrote:
> On Fri, Nov 9, 2018 at 7:20 AM Kadir Cetinkaya via lldb-commits
> wrote:
> >
> > Author: kadircet
> > Date: Fri Nov 9 07:18:02 2018
> > New Revision: 346502
> >
> > URL: http
10 matches
Mail list logo
