aaronpuchert added a comment.
In https://reviews.llvm.org/D52398#1255148, @hokein wrote:
> Hi, @aaronpuchert, the patch has caused many new warnings in our internal
> codebase, below is a reduced one. Do you mind reverting the patch?
>
> // if the condition is not true, CHECK will terminate th
sammccall updated this revision to Diff 168288.
sammccall added a comment.
Rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52250
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Merge.cpp
cla
Author: sammccall
Date: Thu Oct 4 07:20:22 2018
New Revision: 343780
URL: http://llvm.org/viewvc/llvm-project?rev=343780&view=rev
Log:
[clangd] expose MergedIndex class
Summary:
This allows inheriting from it, so index() can ga away and allowing
TestTU::index) to be fixed.
Reviewers: ioeric
Su
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343780: [clangd] expose MergedIndex class (authored by
sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52250?vs=168288&id=168289#toc
Repository:
rCTE Clang Tools Extra
sammccall updated this revision to Diff 168290.
sammccall added a comment.
Rebase, and enable test that is now waiting on this patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52796
Files:
clangd/index/dex/Dex.cpp
clangd/index/dex/Dex.h
clangd/index/dex/Trigram.cpp
xbolva00 added a comment.
Ok now?
https://reviews.llvm.org/D52835
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh added a comment.
Have you figured out why exactly does the check hang? Disabling it for
-fno-exceptions may just hide a logical problem in the check.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52880
___
cfe-commits maili
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D52880#1255249, @alexfh wrote:
> Have you figured out why exactly does the check hang? Disabling it for
> -fno-exceptions may just hide a logical problem in the che
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG. Thanks
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
hokein added a comment.
In https://reviews.llvm.org/D52398#1255218, @aaronpuchert wrote:
> In https://reviews.llvm.org/D52398#1255148, @hokein wrote:
>
> > Hi, @aaronpuchert, the patch has caused many new warnings in our internal
> > codebase, below is a reduced one. Do you mind reverting the pa
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52882
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52688
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, delesley, hokein.
Herald added a subscriber: cfe-commits.
We unwrap conditional expressions containing try-lock functions.
Additionally we don't branch on conditionals, since that is usually not
helpful. When joining
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
LG once other comments are addressed.
https://reviews.llvm.org/D51332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.o
aaronpuchert added a comment.
@hokein Please have a look at https://reviews.llvm.org/D52888, maybe you can
try it out already. The problem was that `?:` expressions are considered a
branch point and when merging both branches the warning was emitted. Before
this change, we couldn't look into `_
aaronpuchert added a comment.
By the way, the tests also work with `__builtin_expect`, but I didn't want to
blow up the code too much.
Repository:
rC Clang
https://reviews.llvm.org/D52888
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
ffigueras added a comment.
Thanks @alexfh!
Could you please commit it for me?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52882
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
Perhaps libc++ could provide a __version file that contains the headers,
and use #include <__version> internally? We'd still need a header
that just includes <__version> for conformance, but that should be after
user headers on the include path so shouldn't break things.
On Thu, 4 Oct 2018, 07:10
JonasToth added a comment.
In https://reviews.llvm.org/D52880#1255250, @alexfh wrote:
> In https://reviews.llvm.org/D52880#1255249, @alexfh wrote:
>
> > Have you figured out why exactly does the check hang? Disabling it for
> > -fno-exceptions may just hide a logical problem in the check.
>
>
>
ilya-biryukov added a comment.
Overall LGTM, just a quick question to make sure I get what's going on.
Comment at: clangd/index/dex/Dex.cpp:184
+ScopeIterators.push_back(iterator(Token(Token::Kind::Scope, Scope)));
+ if (Req.AnyScope || /*legacy*/ Req.Scopes.empty())
Author: jonastoth
Date: Thu Oct 4 08:47:57 2018
New Revision: 343788
URL: http://llvm.org/viewvc/llvm-project?rev=343788&view=rev
Log:
[clang-tidy] Added pointer types to clang-tidy readability-identifier-naming
check.
Summary:
Option to check for different naming conventions on the following
Author: jonastoth
Date: Thu Oct 4 08:49:25 2018
New Revision: 343789
URL: http://llvm.org/viewvc/llvm-project?rev=343789&view=rev
Log:
[clang-tidy] fix PR39167, bugprone-exception-escape hangs-up
Summary:
The check bugprone-exception-escape should not register
if -fno-exceptions is set for the c
Author: ldionne
Date: Thu Oct 4 08:49:42 2018
New Revision: 343790
URL: http://llvm.org/viewvc/llvm-project?rev=343790&view=rev
Log:
[clang] Add the exclude_from_explicit_instantiation attribute
Summary:
This attribute allows excluding a member of a class template from being part
of an explicit
ldionne marked an inline comment as done.
ldionne added inline comments.
Comment at: clang/lib/Sema/Sema.cpp:648
+ !FD->getMostRecentDecl()->isInlined() &&
+ !FD->hasAttr())
continue;
rsmith wrote:
> What's the purpose of this change?
I
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343788: [clang-tidy] Added pointer types to clang-tidy
readability-identifier-naming… (authored by JonasToth, committed by ).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52882
Files
JonasToth added a comment.
Committed on your behalf
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52882
___
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 rL343789: [clang-tidy] fix PR39167, bugprone-exception-escape
hangs-up (authored by JonasToth, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D52880
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343789: [clang-tidy] fix PR39167,
bugprone-exception-escape hangs-up (authored by JonasToth, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52880?vs=168262&id=168301#toc
Repositor
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343790: [clang] Add the exclude_from_explicit_instantiation
attribute (authored by ldionne, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51789?vs=166590&id=168302#toc
Repository:
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343790: [clang] Add the exclude_from_explicit_instantiation
attribute (authored by ldionne, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51789?vs=166590&id=168303#toc
Repository:
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
Putting a stamp, assuming is answer is "yes, it's a legacy way to specify all
scopes"
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52796
__
JonasToth added a comment.
from my side is nothing outstanding
https://reviews.llvm.org/D51332
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: jonastoth
Date: Thu Oct 4 08:55:37 2018
New Revision: 343791
URL: http://llvm.org/viewvc/llvm-project?rev=343791&view=rev
Log:
[clang-tidy] NFC use CHECK-NOTES in tests for performance-move-constructor-init
Reviewers: alexfh, aaron.ballman, hokein
Reviewed By: alexfh
Subscribers: lebed
kadircet created this revision.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
kadircet edited the summary of this revision.
kadircet added reviewers: sammccall, ilya-biryukov.
kadircet added a dependency: D52890: Also report range for the name token on
SamMaier updated this revision to Diff 168304.
SamMaier marked 11 inline comments as done.
SamMaier added a comment.
Addressing comments
Repository:
rC Clang
https://reviews.llvm.org/D52800
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
unittests/Format/CMakeLists.txt
uni
SamMaier added inline comments.
Comment at: lib/Format/Format.cpp:1932
+bool IsStatic = false;
+if (Static.contains("static")) {
+ IsStatic = true;
krasimir wrote:
> Hm, this would also pick up the `static` in `import a.*; // static`, rig
kadircet created this revision.
kadircet added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, ioeric.
kadircet added a dependent revision: D52889: [clangd] Add new test to cover
no_member diag..
[clang] Report range of the missing-memmber on no_member diagnostic.
Curr
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343791: [clang-tidy] NFC use CHECK-NOTES in tests for
performance-move-constructor-init (authored by JonasToth, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52691?vs=167934&id=16
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343791: [clang-tidy] NFC use CHECK-NOTES in tests for
performance-move-constructor-init (authored by JonasToth, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://revi
Author: jonastoth
Date: Thu Oct 4 08:59:30 2018
New Revision: 343792
URL: http://llvm.org/viewvc/llvm-project?rev=343792&view=rev
Log:
[clang-tidy] NFC use CHECK-NOTES in tests for fuchsia-default-arguments
Reviewers: alexfh, aaron.ballman, hokein
Reviewed By: alexfh
Subscribers: xazax.hun, cf
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343792: [clang-tidy] NFC use CHECK-NOTES in tests for
fuchsia-default-arguments (authored by JonasToth, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52688?vs=168092&id=168310#toc
Author: sammccall
Date: Thu Oct 4 09:05:22 2018
New Revision: 343793
URL: http://llvm.org/viewvc/llvm-project?rev=343793&view=rev
Log:
[clangd] Fix ambiguous constructor in DexTest
Modified:
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
Modified: clang-tools-extra/trunk/unittests/cl
scott.linder created this revision.
scott.linder added reviewers: yaxunl, kzhuravl, arsenm.
Herald added subscribers: cfe-commits, Anastasia, tpr, dstuttard, nhaehnle,
wdng, jvesely.
Controls the visibility of non-kernel functions when compiling for AMDGPU
targets. Defaults to the default value
scott.linder added a comment.
I don't know who else to add as a reviewer; Sam, is there someone else outside
of AMD that would be interested in reviewing this?
Repository:
rC Clang
https://reviews.llvm.org/D52891
___
cfe-commits mailing list
cfe
scott.linder updated this revision to Diff 168314.
scott.linder added a comment.
Update docs
https://reviews.llvm.org/D52891
Files:
docs/ClangCommandLineReference.rst
include/clang/Basic/LangOptions.def
include/clang/Driver/CC1Options.td
include/clang/Driver/Options.td
lib/CodeGen/Tar
Author: jonastoth
Date: Thu Oct 4 09:29:58 2018
New Revision: 343796
URL: http://llvm.org/viewvc/llvm-project?rev=343796&view=rev
Log:
[clangd] fix another ambigous constructor in DexTest
Modified:
clang-tools-extra/trunk/unittests/clangd/DexTests.cpp
Modified: clang-tools-extra/trunk/unitt
Hi Sam,
Your change is causing a build failure on one of the build bots. Can you take a
look?
http://lab.llvm.org:8011/builders/clang-x86_64-linux-abi-test/builds/33139
FAILED:
tools/clang/tools/extra/unittests/clangd/CMakeFiles/ClangdTests.dir/TestTU.cpp.o
/usr/bin/c++ -DGTEST_HAS_RTTI=0
hokein added a comment.
Thanks for the quick fix! I'm not the right person to do the review, but I
verified that the warnings are gone with your patch, thanks!
Repository:
rC Clang
https://reviews.llvm.org/D52888
___
cfe-commits mailing list
cfe
christophe-calmejane added a comment.
I fixed it like this (not sure it's 100% correct though!!)
State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount >
1;
if (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
Current.Tok.getKind() == tok::Toke
Author: jonastoth
Date: Thu Oct 4 09:39:41 2018
New Revision: 343797
URL: http://llvm.org/viewvc/llvm-project?rev=343797&view=rev
Log:
[clang-tidy] fix failing unit tests
The removal from the FIX-IT notes through the check-clang-tidy
script was done incorrect. I did not detect beforehand but adj
Szelethus updated this revision to Diff 168315.
https://reviews.llvm.org/D52790
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expan
IdrissRio created this revision.
IdrissRio added reviewers: JonasToth, aaron.ballman, alexfh.
Herald added subscribers: cfe-commits, mgorny.
Hello, i want to propose this check suggested by @EugeneZelenko.
I have found it in the beginner tag of llvm-bugzilla repository.
This check looks for nume
lebedev.ri added a comment.
The check itself isn't in the diff.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52892
___
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 168317.
Szelethus added a comment.
Changed the plist output.
https://reviews.llvm.org/D52790
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticA
IdrissRio updated this revision to Diff 168321.
IdrissRio added a comment.
Herald added a subscriber: jfb.
Sorry i have forgot to add the source code.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52892
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/Nu
Szelethus updated this revision to Diff 168319.
https://reviews.llvm.org/D52790
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expan
Szelethus updated this revision to Diff 168320.
Szelethus marked an inline comment as done.
https://reviews.llvm.org/D52742
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Cor
aaron.ballman added inline comments.
Comment at: lib/Analysis/ThreadSafety.cpp:1445
+ if (!TCond && FCond) {
+Negate = !Negate;
+return getTrylockCallExpr(COP->getCond(), C, Negate);
Rather than do an assignment here, why not just pass `!Nega
Author: sammccall
Date: Thu Oct 4 10:15:41 2018
New Revision: 343800
URL: http://llvm.org/viewvc/llvm-project?rev=343800&view=rev
Log:
[clangd] Add std::move for converting-return to satisfy older compilers
Modified:
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
Modified: clang-tools-
Sorry! r343800 should fix.
On Thu, Oct 4, 2018 at 6:33 PM wrote:
> Hi Sam,
>
> Your change is causing a build failure on one of the build bots. Can you
> take a look?
>
> http://lab.llvm.org:8011/builders/clang-x86_64-linux-abi-test/builds/33139
>
> FAILED:
> tools/clang/tools/extra/unittests/cl
sammccall added inline comments.
Comment at: clangd/index/dex/Dex.cpp:184
+ScopeIterators.push_back(iterator(Token(Token::Kind::Scope, Scope)));
+ if (Req.AnyScope || /*legacy*/ Req.Scopes.empty())
ScopeIterators.push_back(
ilya-biryukov wrote:
> Why do
Author: sammccall
Date: Thu Oct 4 10:18:55 2018
New Revision: 343802
URL: http://llvm.org/viewvc/llvm-project?rev=343802&view=rev
Log:
[clangd] Simplify Dex query tree logic and fix missing-posting-list bug
Summary:
The bug being fixed: when a posting list doesn't exist in the index, it
was prev
Author: sammccall
Date: Thu Oct 4 10:18:49 2018
New Revision: 343801
URL: http://llvm.org/viewvc/llvm-project?rev=343801&view=rev
Log:
[clangd] Dex: FALSE iterator, peephole optimizations, fix AND bug
Summary:
The FALSE iterator will be used in a followup patch to fix a logic bug in Dex
(current
JonasToth added a comment.
Hi IdrissRio,
thanks for working on this! I have one question: Why are variables _not_
considered in the check but only constants? IMHO it would make sense to
transform these as well.
I dont have time for long review today anymore, I will continue tomorrow.
=
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE343802: [clangd] Simplify Dex query tree logic and fix
missing-posting-list bug (authored by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52796?vs=168290&id=168328#toc
Eugene.Zelenko added a comment.
I think modernize is better module for this check.
Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:58
+bool &IL, SourceLocation *IS)
+: IL(&IL), AtLeastOneInclude(false), IS(IS){};
+
Semicolon at end
thakis added a comment.
In https://reviews.llvm.org/D52773#1255093, @zturner wrote:
> I agree magic environment variables are bad, but without it we don’t
> address the only current actual use we have for this, which is making the
> vs integration print filenames. Detecting compiler version fro
Eugene.Zelenko added a comment.
It'll be good idea to handle climits constants too.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52892
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/l
jfb added a comment.
Can you also catch casts such as `(unsigned)-1` ?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52892
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
craig.topper added inline comments.
Comment at: include/clang/Basic/BuiltinsWebAssembly.def:55
+BUILTIN(__builtin_wasm_replace_lane_i32x4, "V4iV4iIii", "ncV:128:")
+BUILTIN(__builtin_wasm_replace_lane_i64x2, "V2WiV2WiIiWi", "ncV:128:")
+BUILTIN(__builtin_wasm_replace_lane_f32x4,
aaron.ballman added inline comments.
Comment at: lib/AST/NestedNameSpecifier.cpp:342
void NestedNameSpecifier::dump(const LangOptions &LO) const {
+ dump(llvm::errs(), LO);
`LLVM_DUMP_METHOD` ?
Comment at: lib/AST/NestedNameSpecifier.cpp:34
Szelethus updated this revision to Diff 168334.
https://reviews.llvm.org/D52794
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-wit
zturner added a comment.
In https://reviews.llvm.org/D52773#1255491, @thakis wrote:
> In https://reviews.llvm.org/D52773#1255093, @zturner wrote:
>
> > I agree magic environment variables are bad, but without it we don’t
> > address the only current actual use we have for this, which is making t
steveire updated this revision to Diff 168338.
steveire added a comment.
Format
Repository:
rC Clang
https://reviews.llvm.org/D52870
Files:
include/clang/AST/NestedNameSpecifier.h
lib/AST/NestedNameSpecifier.cpp
Index: lib/AST/NestedNameSpecifier.cpp
===
rnk added a comment.
It doesn't address the version skew issue, but my blanket argument against any
new environment variable is the existance of things like `_CL_` and
`CCC_OVERRIDE_OPTIONS`. You can set any compiler flag you want from the
environment, so we should standardize on one way of set
aaronpuchert added inline comments.
Comment at: lib/Analysis/ThreadSafety.cpp:1445
+ if (!TCond && FCond) {
+Negate = !Negate;
+return getTrylockCallExpr(COP->getCond(), C, Negate);
aaron.ballman wrote:
> Rather than do an assignment here, wh
aaron.ballman added inline comments.
Comment at: lib/Analysis/ThreadSafety.cpp:1445
+ if (!TCond && FCond) {
+Negate = !Negate;
+return getTrylockCallExpr(COP->getCond(), C, Negate);
aaronpuchert wrote:
> aaron.ballman wrote:
> > Rather than
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
Repository:
rC Clang
https://reviews.llvm.org/D52870
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.
Author: steveire
Date: Thu Oct 4 12:22:00 2018
New Revision: 343807
URL: http://llvm.org/viewvc/llvm-project?rev=343807&view=rev
Log:
[NestedNameSpecifier] Add missing stream-specific dump methods
Reviewers: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org
leonardchan added a comment.
In https://reviews.llvm.org/D52814#1254901, @philip.pfaffe wrote:
> Is this the right place in the pipeline to put the passes? The legacy PM
> inserts the asan passes at EP_OptimizerLast, why not do the same thing?
I noticed this also and thought adding this using
This revision was automatically updated to reflect the committed changes.
Closed by commit rL343807: [NestedNameSpecifier] Add missing stream-specific
dump methods (authored by steveire, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D5287
leonardchan updated this revision to Diff 168348.
leonardchan marked an inline comment as done.
Repository:
rC Clang
https://reviews.llvm.org/D52814
Files:
lib/CodeGen/BackendUtil.cpp
test/CodeGen/asan-new-pm.ll
Index: test/CodeGen/asan-new-pm.ll
=
nickdesaulniers added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:15604
+public EvaluatedExprVisitor {
+ Sema &S;
+
../tools/clang/lib/Sema/SemaExpr.cpp:15619:13: warning: private field 'S' is
not used [-Wunused-private-field]
Sema &S;
efriedma added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:6589
+std::string Reg = StrVal == "31" ? "sp" :
+ "x" + std::string(StrVal.c_str());
+
Could you just write this as `std::string Reg = Value == 31 ? "sp" : "x" +
Value
simark added a comment.
So I revisited this today. In the end, restarting clangd in our IDE works
well. It should be merged anytime soon:
https://github.com/theia-ide/theia/pull/3017
But I am wondering what to do with the feature that allows clangd to change
compilation database path using t
mgrang updated this revision to Diff 168353.
https://reviews.llvm.org/D52838
Files:
include/clang/Basic/BuiltinsAArch64.def
lib/CodeGen/CGBuiltin.cpp
lib/Headers/intrin.h
lib/Sema/SemaChecking.cpp
test/CodeGen/arm64-microsoft-intrinsics.c
Index: test/CodeGen/arm64-microsoft-intrinsics
erik.pilkington updated this revision to Diff 168352.
erik.pilkington added a comment.
Address @rsmith's review comments.
https://reviews.llvm.org/D52521
Files:
clang/include/clang/AST/DeclTemplate.h
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaCXX/member-spec-dr727.cpp
Inde
erik.pilkington added inline comments.
Comment at: clang/include/clang/AST/DeclTemplate.h:1771-1779
+ void setIsSpecializedMember(bool V = true) { IsSpecializedMember = V; }
+
+ /// If this class template specialization was declared at class scope
(DR727).
+ /// For example,
Author: smeenai
Date: Thu Oct 4 12:50:14 2018
New Revision: 343808
URL: http://llvm.org/viewvc/llvm-project?rev=343808&view=rev
Log:
[AST] Revert mangling changes from r339428
As discussed in https://reviews.llvm.org/D50144, we want Obj-C classes
to have the same mangling as C++ structs, to supp
This revision was automatically updated to reflect the committed changes.
Closed by commit rC343808: [AST] Revert mangling changes from r339428 (authored
by smeenai, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52581?vs=167219&id=168354#toc
Repository:
rC Clang
https:/
delesley added a comment.
For future patches, please add Richard Trieu (rtr...@google.com) as a
subscriber. I will continue to try and do code reviews, but Richard is on the
team that actually rolls out new compiler changes. Thanks!
BTW, the issue is not just that changes may introduce false
efriedma added a comment.
Needs a testcase for the error message like we have for other intrinsics in
test/Sema/builtins-arm64.c . Otherwise LGTM.
https://reviews.llvm.org/D52838
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.cpp:44
+SrcMgr::CharacteristicKind FileType) {
+
+ if (FileName == "limits") {
Unnecessary empty line.
Repository:
rCTE Clang Tools Extra
https://rev
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm, sorry it got lost.
Repository:
rC Clang
https://reviews.llvm.org/D52418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
nickdesaulniers added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:15621-15623
+for (InitListExpr::iterator II = E->begin(), IE = E->end();
+ II != IE; ++II)
+ Visit(*II);
`std::for_each`?
https://reviews.llvm.org/D52854
__
void updated this revision to Diff 168356.
void added a comment.
Removed unused field.
https://reviews.llvm.org/D52854
Files:
include/clang/AST/Expr.h
include/clang/Sema/Sema.h
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaT
nickdesaulniers added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:15621-15623
+for (InitListExpr::iterator II = E->begin(), IE = E->end();
+ II != IE; ++II)
+ Visit(*II);
nickdesaulniers wrote:
> `std::for_each`?
Sorry, posted
void added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:15621-15623
+for (InitListExpr::iterator II = E->begin(), IE = E->end();
+ II != IE; ++II)
+ Visit(*II);
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > `std::for_each`
nickdesaulniers added inline comments.
Comment at: lib/Sema/SemaExpr.cpp:15621-15623
+for (InitListExpr::iterator II = E->begin(), IE = E->end();
+ II != IE; ++II)
+ Visit(*II);
void wrote:
> nickdesaulniers wrote:
> > nickdesaulniers
aaronpuchert added inline comments.
Comment at: test/SemaCXX/warn-thread-safety-analysis.cpp:1879-1880
+ void foo13() {
+if (mu.TryLock() ? 1 : 0)
+ mu.Unlock();
+ }
aaron.ballman wrote:
> aaronpuchert wrote:
> > aaron.ballman wrote:
> > > Can you add
101 - 200 of 271 matches
Mail list logo