<div dir="ltr">It's not something we do in clangd, it's something we do in our buildsystem for usual compilations as well. And the idea is not to avoid warnings from headers (we want those), but to avoid warnings from headers of third_party dependencies (those we usually don't control, so we can't fix them).<div><div><div><br></div></div></div></div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 25, 2018 at 8:31 AM Ivan Donchevskii <<a href="mailto:ivan.donchevskii@qt.io">ivan.donchevskii@qt.io</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div id="m_-5188644897855734428divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif" dir="ltr">
<p style="margin-top:0;margin-bottom:0">There are different ways to suppress warnings other than passing -isystem so we're trying to make fewer -isystem paths (for example <a href="https://reviews.llvm.org/D48116" class="m_-5188644897855734428OWAAutoLink" id="m_-5188644897855734428LPlnk454646" target="_blank">https://reviews.llvm.org/D48116</a> which
i really like and am planing to update in the nearest future)</p>
</div>
<hr style="display:inline-block;width:98%">
<div id="m_-5188644897855734428divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Ilya Biryukov <<a href="mailto:ibiryukov@google.com" target="_blank">ibiryukov@google.com</a>><br>
<b>Sent:</b> Wednesday, July 25, 2018 8:20:40 AM<br>
<b>To:</b> Ivan Donchevskii<br>
<b>Cc:</b> <a href="mailto:Dmitry.Kozhevnikov@jetbrains.com" target="_blank">Dmitry.Kozhevnikov@jetbrains.com</a>; via cfe-dev<br>
<b>Subject:</b> Re: [cfe-dev] Locking files on Windows with clangd</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div>On Wed, Jul 25, 2018 at 8:09 AM Ivan Donchevskii <<a href="mailto:ivan.donchevskii@qt.io" target="_blank">ivan.donchevskii@qt.io</a>> wrote:<br>
</div>
<div class="m_-5188644897855734428x_gmail_quote">
<blockquote class="m_-5188644897855734428x_gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div id="m_-5188644897855734428x_gmail-m_4174978096202631882divtagdefaultwrapper" dir="ltr" style="font-size:12pt;color:rgb(0,0,0);font-family:Calibri,Helvetica,sans-serif">
<p style="margin-top:0px;margin-bottom:0px"><span style="font-size:12pt">There's no way I know to fix it because AFAIK windows always locks mmapped files.</span><br>
</p>
</div>
</div>
</blockquote>
<div>That's unfortunate, have you seen any pointers of this behavior in official docs?</div>
<div>I think I was able to modify the mmapped file when I was playing around with Windows APIs while looking at D35200, but I'll need to rerun my experiments and make them reproducible.</div>
<div> </div>
<blockquote class="m_-5188644897855734428x_gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div id="m_-5188644897855734428x_gmail-m_4174978096202631882divtagdefaultwrapper" dir="ltr" style="font-size:12pt;color:rgb(0,0,0);font-family:Calibri,Helvetica,sans-serif">
<p style="margin-top:0px;margin-bottom:0px"><span style="font-size:12pt">But i can't say that this behavior is broken because you are not supposed to mark file 'system' if you plan to modify it. What would I do if I had bugreport with '</span><span style="color:rgb(33,33,33);font-family:wf_segoe-ui_normal,"Segoe UI","Segoe WP",Tahoma,Arial,sans-serif,serif,EmojiFont;font-size:15px">#pragma
system_header' - I would simply remove that line from the unsaved file content I pass to clang.</span><br>
</p>
</div>
</div>
</blockquote>
<div>I'm not sure there's a single definition of what a "system" header is, e.g. we pass -isystem when adding include paths to third_party code to suppress warnings in that code.</div>
<div> </div>
<blockquote class="m_-5188644897855734428x_gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div id="m_-5188644897855734428x_gmail-m_4174978096202631882divtagdefaultwrapper" dir="ltr" style="font-size:12pt;color:rgb(0,0,0);font-family:Calibri,Helvetica,sans-serif">
<p style="margin-top:0px;margin-bottom:0px"><span><span style="color:rgb(33,33,33);font-family:wf_segoe-ui_normal,"Segoe UI","Segoe WP",Tahoma,Arial,sans-serif,serif,EmojiFont;font-size:15px"></span></span></p>
<p style="margin-top:0px;margin-bottom:0px"><span><span style="color:rgb(33,33,33);font-family:wf_segoe-ui_normal,"Segoe UI","Segoe WP",Tahoma,Arial,sans-serif,serif,EmojiFont;font-size:15px">Regards,</span></span></p>
<p style="margin-top:0px;margin-bottom:0px"><span><span style="color:rgb(33,33,33);font-family:wf_segoe-ui_normal,"Segoe UI","Segoe WP",Tahoma,Arial,sans-serif,serif,EmojiFont;font-size:15px">Ivan</span></span></p>
<br>
</div>
<hr style="display:inline-block;width:98%">
<div id="m_-5188644897855734428x_gmail-m_4174978096202631882divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Ilya Biryukov <<a href="mailto:ibiryukov@google.com" target="_blank">ibiryukov@google.com</a>><br>
<b>Sent:</b> Tuesday, July 24, 2018 9:13:47 AM<br>
<b>To:</b> <a href="mailto:Dmitry.Kozhevnikov@jetbrains.com" target="_blank">Dmitry.Kozhevnikov@jetbrains.com</a>; Ivan Donchevskii<br>
<b>Cc:</b> via cfe-dev<br>
<b>Subject:</b> Re: [cfe-dev] Locking files on Windows with clangd</font>
<div> </div>
</div>
<div>
<div dir="ltr">Hi Dmitry,
<div><br>
</div>
<div><span style="font-size:small;background-color:rgb(255,255,255);float:none;display:inline">We've had a pull request with a similar fix at some point: <a href="https://reviews.llvm.org/D35200" target="_blank">https://reviews.llvm.org/D35200</a>, but</span> it
was abandoned and never made it in.</div>
<div><a class="m_-5188644897855734428x_gmail-m_4174978096202631882x_gmail_plusreply" id="m_-5188644897855734428x_gmail-m_4174978096202631882x_gmail-plusReplyChip-3" href="mailto:ivan.donchevskii@qt.io" target="_blank">@ivan.donchevskii@qt.io</a>, did you manage to figure out a way to make this work on
Windows? Any ideas on what's the best way to fix this?<br>
</div>
<div><br>
</div>
</div>
<br>
<div class="m_-5188644897855734428x_gmail-m_4174978096202631882x_gmail_quote">
<div dir="ltr">On Sun, Jul 22, 2018 at 7:22 PM <a href="mailto:Dmitry.Kozhevnikov@jetbrains.com" target="_blank">
Dmitry.Kozhevnikov@jetbrains.com</a> via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
</div>
<blockquote class="m_-5188644897855734428x_gmail-m_4174978096202631882x_gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">Hi!<br>
<br>
A short background: we at JetBrains are experimenting with using clangd with<br>
CLion, and we're very happy with the results so far. Unfortunately, we've hit<br>
a problem when header files are locked on Windows, so they can't be saved,<br>
they break `git rebase` (leading to the loss of work), etc.<br>
<br>
It happens when the file is opened using a memory mapped file. It's fairly<br>
easy to reproduce with clangd:<br>
<br>
1. Create a header big enough to pass the threshold in `shouldUseMmap()`<br>
(lib/Support/MemoryBuffer.cpp)<br>
2. Include it in a source file<br>
3. Keep this source file opened via clangd<br>
4. Now an attempt to modify this header will fail<br>
<br>
The situation is especially unpleasant because the handle is locked not only<br>
during the parse, but for all the time clangd holds the preamble containing<br>
this header.<br>
<br>
This issue is mitigated to an extent in libclang: non-system files are<br>
considered "volatile" (see r160074, r334070), so memory mapped files are not<br>
used for them. However, I feel like it's not enough: you can easily have a<br>
`#pragma system_header` in your codebase (IIUC it affects the mentioned<br>
behavior), locking such file and i.e. losing user's work during `git rebase`<br>
is still unacceptable.<br>
<br>
Also, I think the fact that the proper compiler has this behavior is also<br>
unfortunate (forgotten build in the background might lead to the loss of data).<br>
<br>
IIUC the main reason for having memory mapped files is to reduce the memory<br>
footprint. However, for a regular translation unit, headers usually take several<br>
megabytes, which IMO is tolerable, and is usually topped by other per-TU data<br>
anyway.<br>
<br>
Hence, what do you think about not using memory mapped files on Windows at all<br>
for source files? Are there any implications that I'm not aware of?<br>
<br>
The naive patch (obviously not for commit, just for the illustration):<br>
<br>
--- a/lib/Basic/SourceManager.cpp<br>
+++ b/lib/Basic/SourceManager.cpp<br>
@@ -109,7 +109,12 @@ llvm::MemoryBuffer *ContentCache::getBuffer(DiagnosticsEngine &Diag,<br>
return Buffer.getPointer();<br>
}<br>
<br>
+#ifndef _WIN32<br>
bool isVolatile = SM.userFilesAreVolatile() && !IsSystemFile;<br>
+#else<br>
+ bool isVolatile = true;<br>
+#endif<br>
+<br>
auto BufferOrError =<br>
SM.getFileManager().getBufferForFile(ContentsEntry, isVolatile);<br>
<br>
If I've not missed something, this would effectively disable memory mapped<br>
files for source files on Windows, but they would still be used for<br>
other potentially large files (including PCHs and preambles).<br>
<br>
If this is unacceptable, at the very least, clangd should treat user files<br>
similar to libclang (we can work on a patch in this case).<br>
<br>
Note that there is a similar bug report for QtCreator/libclang with some<br>
interesting comments: <a href="https://bugreports.qt.io/browse/QTCREATORBUG-15449" target="_blank">https://bugreports.qt.io/browse/QTCREATORBUG-15449</a><br>
<br>
Cheers,<br>
Dmitry.</div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_-5188644897855734428x_gmail-m_4174978096202631882x_gmail_signature">
<div dir="ltr">
<div>
<div dir="ltr">
<div>Regards,</div>
<div>Ilya Biryukov</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<br clear="all">
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_-5188644897855734428x_gmail_signature">
<div dir="ltr">
<div>
<div dir="ltr">
<div>Regards,</div>
<div>Ilya Biryukov</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>