ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
Thanks! LGTM with a last drop of NITs.
Do you already have commit access or do you want me to submit this change for
you?
Comment at: include/clang/Basic/Virtu
hans added a comment.
Very cool! I only have some minor comments.
Oh, and when it lands, you should probably add an update to
docs/ReleaseNotes.rst about it too.
Comment at: include/clang/Driver/CC1Options.td:613
+ HelpText<"Stop PCH generation after #pragma hdrstop. When u
bogden added a reviewer: efriedma.
bogden added a comment.
Hello Eli -- Sjoerd pointed out that you had concerns about this sort of
command-line manipulation in his patch https://reviews.llvm.org/D50179. I'm
having to do something similar here to deal with +fp16fml being implied by
+fp16 from v
Author: theraven
Date: Tue Sep 4 02:23:18 2018
New Revision: 341350
URL: http://llvm.org/viewvc/llvm-project?rev=341350&view=rev
Log:
Disable the GNUstep v2 ABI on Windows.
The code remains so that we can potentially reenable it in a point
release, but the driver will reject it. Several issues
hokein added a comment.
Thanks for cleaning it up! I admit that `SymbolOccurrences` is a loong name. A
few nits.
Comment at: clangd/index/FileIndex.cpp:120
+ auto &SymRefs = Sym.second;
+ std::sort(SymRefs.begin(), SymRefs.end());
+ std::copy(SymRefs.begin(), Sy
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
nice!
Comment at: clangd/index/Index.cpp:153
+ // We can reuse the arena, as it only has unique strings and we need them
all.
+ // Reallocate the ref lists on the arena to
svenvh added inline comments.
Comment at: lib/Headers/opencl-c-common.h:9
+//===--===//
+
+#ifndef __OPENCL_C_COMMON_H__
Would it be worth having a brief explanation here about what is supposed t
kbobyrev updated this revision to Diff 163773.
kbobyrev marked 12 inline comments as done.
kbobyrev added a comment.
- Rebase on top of the parent patch
- Apply many refactorings where appropriate
- Write more comments and documentation
- Abstract pieces of code which are shared by multiple pieces
Author: theraven
Date: Tue Sep 4 03:07:27 2018
New Revision: 341352
URL: http://llvm.org/viewvc/llvm-project?rev=341352&view=rev
Log:
Revert "Disable the GNUstep v2 ABI on Windows."
This reverts commit b4547c9cadd2f8adfe3f3182e4c56e466c5256cb.
Apparently git llvm push from the monorepo does not
chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.
I mean, sure.
I really don't know that supporting this ever expanding diversity of build
strategies is worth its cost, but I don't see a specific reason to not take
this patch
Rep
whisperity added a comment.
Whew, this is a big one. Generally looks good, although I would separate
implementation detail functions a bit better, with perhaps more comments to
move them apart a bit, it is really harsh to scroll through.
Could you please re-run the findings on the projects? The
ilya-biryukov added inline comments.
Comment at: clangd/ClangdServer.cpp:152
WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
-DynamicIdx ? *DynamicIdx : noopParsingCallbacks(),
+DynamicIdx ? DynamicIdx->makeUpdateC
sammccall marked 7 inline comments as done.
sammccall added a comment.
In https://reviews.llvm.org/D51605#1222636, @hokein wrote:
> Thanks for cleaning it up! I admit that `SymbolOccurrences` is a loong name.
> A few nits.
Yeah, I hope you don't mind! The references stuff is awesome, I'm hopin
sammccall updated this revision to Diff 163784.
sammccall marked 4 inline comments as done.
sammccall added a comment.
Address comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51605
Files:
clangd/ClangdServer.cpp
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
lebedev.ri added inline comments.
Comment at: clangd/index/Index.cpp:158
+auto &SymRefs = Sym.second;
+std::sort(SymRefs.begin(), SymRefs.end());
+// TODO: do we really need to dedup?
I noticed this by accident, but i'm pretty sure `std::sort()` shoul
joerg added a comment.
Please check the history of the file for some of the problems with the
redefinition. I'm quite against this change.
https://reviews.llvm.org/D43871
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org
whisperity added a comment.
Generally grammar / phrasing things that have caught my eye.
The rest of the code looks okay, bar my previous comment about better
commenting / separating chunks of helper functions.
Comment at: clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:
usaxena95 updated this revision to Diff 163785.
usaxena95 marked 9 inline comments as done.
usaxena95 added a comment.
- fixed the style issues
Repository:
rC Clang
https://reviews.llvm.org/D51359
Files:
include/clang/Basic/VirtualFileSystem.h
lib/Basic/VirtualFileSystem.cpp
unittests/
usaxena95 added a comment.
I don't have commit access. Can you please submit this ?
Repository:
rC Clang
https://reviews.llvm.org/D51359
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
sammccall marked an inline comment as done.
sammccall added inline comments.
Comment at: clangd/index/Index.cpp:158
+auto &SymRefs = Sym.second;
+std::sort(SymRefs.begin(), SymRefs.end());
+// TODO: do we really need to dedup?
lebedev.ri wrote:
> I no
kbobyrev updated this revision to Diff 163786.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.
Rebase on top of recent revisions.
https://reviews.llvm.org/D51539
Files:
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools
hokein accepted this revision.
hokein added a comment.
LGTM, thanks!
Comment at: clangd/index/FileIndex.cpp:120
+ auto &SymRefs = Sym.second;
+ std::sort(SymRefs.begin(), SymRefs.end());
+ std::copy(SymRefs.begin(), SymRefs.end(), back_inserter(RefsStorage));
---
ilya-biryukov added a comment.
In https://reviews.llvm.org/D51359#1222823, @usaxena95 wrote:
> I don't have commit access. Can you please submit this ?
Will do.
Repository:
rC Clang
https://reviews.llvm.org/D51359
___
cfe-commits mailing list
whisperity added a comment.
Minor comments in-line, looking good on first glance.
I'm not sure if we discussed, has this checker been tried on some in-the-wild
examples? To see if there are some real findings or crashes?
There is a good selection of projects here in this tool:
https://github.co
whisperity added a project: clang.
whisperity added a comment.
@Szelethus `clang-query` seems to sometimes not include matcher functions that
are perfectly available in the code... I recently had some issue with using
`isUserDefined()`, it was available in my code, but `clang-query` always
reje
whisperity added a comment.
Ping.
Shouldn't let this thing go to waste.
https://reviews.llvm.org/D46081
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: rksimon
Date: Tue Sep 4 05:17:10 2018
New Revision: 341362
URL: http://llvm.org/viewvc/llvm-project?rev=341362&view=rev
Log:
Remove lambda default parameter to silence -Wpedantic warning. NFCI.
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
Modified: clang-tools-extra/tru
chandlerc marked 5 inline comments as done.
chandlerc added a comment.
All outstanding comments addressed, and landing this. Thanks all for the
reviews and let me know if I missed anything.
Comment at: llvm/include/llvm/IR/Attributes.td:181-185
+/// Note that this uses the def
hokein added a comment.
In https://reviews.llvm.org/D50958#149, @sammccall wrote:
> Looking forward to using this!
>
> Unless you object, I'd like to address the remaining comments (and get a
> review), so you can make the most of your leave!
Thanks, feel free to pick it up. The comments m
Author: chandlerc
Date: Tue Sep 4 05:38:00 2018
New Revision: 341363
URL: http://llvm.org/viewvc/llvm-project?rev=341363&view=rev
Log:
[x86/SLH] Add a real Clang flag and LLVM IR attribute for Speculative
Load Hardening.
Wires up the existing pass to work with a proper IR attribute rather
than j
This revision was automatically updated to reflect the committed changes.
chandlerc marked an inline comment as done.
Closed by commit rL341363: [x86/SLH] Add a real Clang flag and LLVM IR
attribute for Speculative (authored by chandlerc, committed by ).
Changed prior to commit:
https://reviews
Hahnfeld added a comment.
Not needed anymore after the reverts in https://reviews.llvm.org/rC341115 and
https://reviews.llvm.org/rC341118, right? Maybe we should revive the test to
make sure we don't break this in the future?
Repository:
rC Clang
https://reviews.llvm.org/D51446
_
theraven added a comment.
In https://reviews.llvm.org/D50783#1221147, @ahatanak wrote:
> The GNUstep documentation I found replaces '@' with '\1'. Would that fix the
> problem?
>
> https://github.com/gnustep/libobjc2/blob/master/ABIDoc/abi.tex
Yup. If this mangling is needed outside of CGObjC
Hahnfeld added a comment.
Not needed anymore after the reverts in https://reviews.llvm.org/rC341115 and
https://reviews.llvm.org/rC341118, right?
https://reviews.llvm.org/D51501
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
ilya-biryukov added a comment.
In https://reviews.llvm.org/D50462#1214120, @Dmitry.Kozhevnikov wrote:
> When lookin through the list of the fatal errors, I think there are different
> categories:
>
> 1. "Scary" ones - "module was relocated", "invalid vfs overlay file", we
> probably shouldn't t
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
`buildStaticIndex()` is used by two other tools that I'm building, now it's
useful outside of `tool/ClangdMain.cpp`.
https://reviews.llvm.o
kbobyrev updated this revision to Diff 163797.
kbobyrev added a comment.
Move function documentation to the header file.
https://reviews.llvm.org/D51626
Files:
clang-tools-extra/clangd/index/SymbolYAML.cpp
clang-tools-extra/clangd/index/SymbolYAML.h
clang-tools-extra/clangd/tool/ClangdMai
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny.
https://reviews.llvm.org/D51628
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang
kbobyrev updated this revision to Diff 163800.
kbobyrev added a comment.
Cleaned up few minor issues.
https://reviews.llvm.org/D51628
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/dexplorer/CMakeLists.txt
clang-tools-extra/clangd/dexplorer/Dexplorer.cpp
Index: c
eandrews added a comment.
Alright. Thanks for the review. I will abandon this patch
https://reviews.llvm.org/D51545
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
elsteveogrande abandoned this revision.
elsteveogrande added a comment.
Abandoning in favor of https://reviews.llvm.org/D51608 which works better.
Repository:
rC Clang
https://reviews.llvm.org/D50942
___
cfe-commits mailing list
cfe-commits@lists
Author: ibiryukov
Date: Tue Sep 4 07:15:53 2018
New Revision: 341366
URL: http://llvm.org/viewvc/llvm-project?rev=341366&view=rev
Log:
Adding HardLink Support to VirtualFileSystem.
Summary:
Added support of creating a hardlink from one file to another file.
After a hardlink is added between two
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341366: Adding HardLink Support to VirtualFileSystem.
(authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51359?vs=163785&
ilya-biryukov added a comment.
Submitted. Thanks for the change!
Repository:
rL LLVM
https://reviews.llvm.org/D51359
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Szelethus updated this revision to Diff 163813.
Szelethus added a comment.
Fixed a crash where the returned region's type wasn't `CXXRecordDecl`. I'm not
even sure how this is possible -- and unfortunately I've been unable to create
a minimal (not) working example for this, and I wasn't even abl
Author: sammccall
Date: Tue Sep 4 07:39:56 2018
New Revision: 341368
URL: http://llvm.org/viewvc/llvm-project?rev=341368&view=rev
Log:
[clangd] SymbolOccurrences -> Refs and cleanup
Summary:
A few things that I noticed while merging the SwapIndex patch:
- SymbolOccurrences and particularly Symb
This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rCTE341368: [clangd] SymbolOccurrences -> Refs and cleanup
(authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51605?vs=1
ioeric requested changes to this revision.
ioeric added a comment.
This revision now requires changes to proceed.
(and forgot to request changes ;)
https://reviews.llvm.org/D51481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.
ioeric added a comment.
There are a few changes/refactorings which I would suggest splitting from this
patch, as they would require more thoughts and seem unrelated in this patch.
Please see the inline comments :)
Comment at: clang-tools-extra/clangd/Quality.h:130
+/// direct
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tools-extra/clangd/index/SymbolYAML.cpp:193
+ auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
+ SymbolSlab::Builder SymsBuilder;
+ for
kbobyrev updated this revision to Diff 163821.
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
- Rebase on top of new code
- Simplify code structure and get rid of global state (except for two filenames
coming from `main()`)
The only problem now is that the generated outp
sammccall added a comment.
OK thanks, I'll steal this one.
Comment at: clangd/XRefs.cpp:724
+ // traversing the AST, and we don't want to make unnecessary queries to the
+ // index. Howerver, we don't have a reliable way to distinguish file-local
+ // symbols. We conservativ
kbobyrev updated this revision to Diff 163823.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.
Rebase and resolve comments
https://reviews.llvm.org/D51626
Files:
clang-tools-extra/clangd/index/SymbolYAML.cpp
clang-tools-extra/clangd/index/SymbolYAML.h
clang-tools-extr
Author: omtcyfz
Date: Tue Sep 4 08:10:40 2018
New Revision: 341369
URL: http://llvm.org/viewvc/llvm-project?rev=341369&view=rev
Log:
[clangd] Move buildStaticIndex() to SymbolYAML
`buildStaticIndex()` is used by two other tools that I'm building, now
it's useful outside of `tool/ClangdMain.cpp`.
probinson added subscribers: cfe-commits, probinson.
probinson added a comment.
+cfe-commits
https://reviews.llvm.org/D51340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341369: [clangd] Move buildStaticIndex() to SymbolYAML
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51626?vs=163823&i
thakis created this revision.
thakis added a reviewer: hans.
/Brepro currently is just an alias for -mno-incremental-linker-compatible and
before this patch only controlled if we write a timestamp into the output obj
file. But cl.exe also passes it on to link.exe (where it controls whether the
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
For the sake of consistency, `quality()` should return `float` instead of
`double` since all of
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
If it's not too expensive, we can use the symbol quality metrics (from
Quality.h) to do a more accurate prescore. Worth a benchmark :-)
https://reviews.llvm.org/D51636
__
sammccall updated this revision to Diff 163834.
sammccall added a comment.
Address comments and tighten up highlights/refs reuse in XRefs.cpp a bit.
Still to do:
- test interaction with index, including actual data
- maybe add the ClangdServer/LSP binding and lit test, if it's just boilerplate
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341373: Fix the -print-multi-directory flag to print the
selected multilib. (authored by chrib, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51354?vs=162846&id=163835#toc
Reposito
ioeric added a comment.
Super cool! Just a few nits.
Comment at: clangd/RIFF.cpp:58
+ if (RIFF->ID != fourCC("RIFF"))
+return makeError("Extra content after RIFF chunk");
+ if (RIFF->Data.size() < 4)
The error message seems wrong?
Comme
sammccall added a comment.
Oops, and I rebased on head (Occurrences -> Refs) of course.
@ilya-biryukov @ioeric, any interest in taking over review here?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50958
___
cfe-commits mailing l
melver added a comment.
Awaiting remaining reviewer acceptance.
FYI: I do not have commit commit access -- what is the procedure to commit once
diff is accepted?
Many thanks!
Repository:
rC Clang
https://reviews.llvm.org/D51036
___
cfe-commits
aprantl added a comment.
This is DWARF5+ only, right? (We shouldn't change the preference of Apple
accelerator tables for DWARF 4 and earlier).
Repository:
rC Clang
https://reviews.llvm.org/D51576
___
cfe-commits mailing list
cfe-commits@lists.l
Author: chrib
Date: Tue Sep 4 08:22:13 2018
New Revision: 341373
URL: http://llvm.org/viewvc/llvm-project?rev=341373&view=rev
Log:
Fix the -print-multi-directory flag to print the selected multilib.
Summary: Fix -print-multi-directory to print the selected multilib
Reviewers: jroelofs
Reviewed
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.
I suppose the alternative would be to make /Brepro a non-alias flag, expand it
to -mno-incremental-linker-compatible for the cc1 invocation and look for it
for the linker invocation.
But this is
sammccall updated this revision to Diff 163836.
sammccall marked an inline comment as done.
sammccall added a comment.
address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51585
Files:
clangd/CMakeLists.txt
clangd/RIFF.cpp
clangd/RIFF.h
clangd/global-s
sammccall marked 2 inline comments as done.
sammccall added inline comments.
Comment at: clangd/index/Serialization.cpp:50
+
+void writeVar(uint32_t I, raw_ostream &OS) {
+ constexpr static uint8_t More = 1 << 7;
ioeric wrote:
> This function could use a comment
sammccall added a comment.
In https://reviews.llvm.org/D51036#1223230, @melver wrote:
> Awaiting remaining reviewer acceptance.
>
> FYI: I do not have commit commit access -- what is the procedure to commit
> once diff is accepted?
>
> Many thanks!
Anyone with commit access can land it for you
kbobyrev added a comment.
In https://reviews.llvm.org/D51636#1223204, @sammccall wrote:
> If it's not too expensive, we can use the symbol quality metrics (from
> Quality.h) to do a more accurate prescore. Worth a benchmark :-)
Yes, I agree. We can investigate that quality/performance trade-of
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341374: [clangd] NFC: Change quality type to float
(authored by omtcyfz, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51636?vs=163833&id=163837#toc
Repository:
rCTE Clang Tool
Author: omtcyfz
Date: Tue Sep 4 08:45:56 2018
New Revision: 341374
URL: http://llvm.org/viewvc/llvm-project?rev=341374&view=rev
Log:
[clangd] NFC: Change quality type to float
Reviewed by: sammccall
Differential Revision: https://reviews.llvm.org/D51636
Modified:
clang-tools-extra/trunk/cl
Typz added a comment.
clang-format does indeed support .clang-format, which is great for *isolated*
projects, and which is not affected by this patch.
This patch addresses the issue of *centralizing* the definition of styles, e.g.
allowing individual projects to reference externally defined sty
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Like https://reviews.llvm.org/D51475 but simplified based on recent patches.
While here, clarify that loadIndex() takes a filename, not
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lg. Thanks!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51638
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://
ioeric abandoned this revision.
ioeric added a comment.
Dropping this in favor of https://reviews.llvm.org/D51638
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://li
martong added a comment.
Here is the `Expected` based patch: https://reviews.llvm.org/D51633
Repository:
rC Clang
https://reviews.llvm.org/D50672
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
Author: sammccall
Date: Tue Sep 4 09:16:50 2018
New Revision: 341375
URL: http://llvm.org/viewvc/llvm-project?rev=341375&view=rev
Log:
[clangd] Define a compact binary serialization fomat for symbol slab/index.
Summary:
This is intended to replace the current YAML format for general use.
It's ~1
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341375: [clangd] Define a compact binary serialization fomat
for symbol slab/index. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://revi
Author: sammccall
Date: Tue Sep 4 09:19:40 2018
New Revision: 341376
URL: http://llvm.org/viewvc/llvm-project?rev=341376&view=rev
Log:
[clangd] Load static index asynchronously, add tracing.
Summary:
Like D51475 but simplified based on recent patches.
While here, clarify that loadIndex() takes a
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D51641
Files:
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
=
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341376: [clangd] Load static index asynchronously, add
tracing. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51638?
martong added a comment.
This patch is huge, but we change here almost all implementation functions of
`ASTNodeImporter` to return with either `Error` or with `Expected`.
We could not really come up with a cohesive but smaller patch because of the
recursive nature of the importer. (We are open t
sammccall added inline comments.
Comment at: lib/Basic/VirtualFileSystem.cpp:275
return EC;
- return Dir.str().str();
+ CWDCache = Dir.str();
+ return CWDCache;
Doesn't this need to be guarded by a lock? I know the current version is
thread-hostile in pr
tra abandoned this revision.
tra added a comment.
> Not needed anymore after the reverts in https://reviews.llvm.org/rC341115 and
> https://reviews.llvm.org/rC341118, right?
Correct.
https://reviews.llvm.org/D51501
___
cfe-commits mailing list
cfe
cdavis5x created this revision.
cdavis5x added reviewers: mstorsjo, rnk.
Herald added subscribers: cfe-commits, christof, mgorny.
This variable is never defined, so its value is always empty. Since
`libunwind` is needed to build the C++ ABI library in the first place,
it should never be linked to
mstorsjo added inline comments.
Comment at: src/CMakeLists.txt:54
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
Is there any point in this line at all now, or can it be removed
cdavis5x added inline comments.
Comment at: src/CMakeLists.txt:54
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
mstorsjo wrote:
> Is there any point in this line at all now, or
ioeric updated this revision to Diff 163858.
ioeric marked an inline comment as done.
ioeric added a comment.
- Guard CWDCache with mutex.
Repository:
rC Clang
https://reviews.llvm.org/D51641
Files:
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
==
ioeric added inline comments.
Comment at: lib/Basic/VirtualFileSystem.cpp:275
return EC;
- return Dir.str().str();
+ CWDCache = Dir.str();
+ return CWDCache;
sammccall wrote:
> Doesn't this need to be guarded by a lock? I know the current version is
> th
mstorsjo accepted this revision.
mstorsjo added inline comments.
This revision is now accepted and ready to land.
Comment at: src/CMakeLists.txt:54
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
NoQ added a comment.
In https://reviews.llvm.org/D51300#1220537, @Szelethus wrote:
> Would you be comfortable me commiting this without that assert
Yup sure.
In https://reviews.llvm.org/D51300#1223042, @Szelethus wrote:
> I'm not even sure how this is possible -- and unfortunately I've been u
Author: cdavis
Date: Tue Sep 4 10:40:26 2018
New Revision: 341388
URL: http://llvm.org/viewvc/llvm-project?rev=341388&view=rev
Log:
[CMake] Remove variable reference that isn't used.
Summary:
This variable is never defined, so its value is always empty. Since
`libunwind` is needed to build the C
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341388: [CMake] Remove variable reference that isn't
used. (authored by cdavis, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D51644
Files:
li
cdavis5x created this revision.
cdavis5x added reviewers: mstorsjo, rnk.
Herald added subscribers: cfe-commits, christof, mgorny, dberris.
This switch only has an effect at link time. It changes the default
compiler support library to `compiler-rt`. With `-nodefaultlibs`, this
library won't get li
efriedma added a comment.
Do you expect that the regression tests will be affected by the TargetParser
fixes? If not, I guess this is okay... but please add clear comments
explaining which bits of code you expect to go away after the TargetParser
rewrite.
Repository:
rC Clang
https://revi
Author: nico
Date: Tue Sep 4 11:00:14 2018
New Revision: 341390
URL: http://llvm.org/viewvc/llvm-project?rev=341390&view=rev
Log:
clang-cl: Pass /Brepro to linker if it was passed to the compiler
Differential Revision: https://reviews.llvm.org/D51635
Modified:
cfe/trunk/lib/Driver/ToolChain
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341390: clang-cl: Pass /Brepro to linker if it was passed to
the compiler (authored by nico, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51635?vs=163829&id=163870#toc
Repository:
1 - 100 of 161 matches
Mail list logo
