abhina.sreeskantharajan added inline comments.
================
Comment at: llvm/lib/Support/Windows/Path.inc:1086
- if (Flags & OF_Text)
+ if (Flags & OF_CRLF)
CrtOpenFlags |= _O_TEXT;
----------------
amccarth wrote:
> I assume it would be weird to set `OF_CRLF` without also setting `OF_Text`,
> so this change seems right for correctness. But maybe this Windows-only code
> would better express the intent if it were written:
>
> ```
> if (Flags & (OF_CRLF | OF_Text))
> ```
>
> Another idea would be to assert if `OF_CRLF` is set but `OF_Text` is not.
>
> Or maybe it's fine as it is.
I chose to create OF_CRLF flag because I only want Windows to turn on text mode
for OF_TextWithCRLF and not OF_Text.
This solution would return true even if Flags was just OF_Text.
```
if (Flags & (OF_CRLF | OF_Text))
```
I think adding an assertion here would be a good idea.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99426/new/
https://reviews.llvm.org/D99426
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
