lebedev.ri added subscribers: shuaiwang, JonasToth, lebedev.ri.
lebedev.ri added a comment.
Regression is very broad term. Is it a false-positive?
Perhaps it is better to address the issue in the ExprMutationAnalyzer itself?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50447
lebedev.ri added a subscriber: EricWF.
lebedev.ri added a comment.
In https://reviews.llvm.org/D50447#1192316, @hokein wrote:
> In https://reviews.llvm.org/D50447#1192280, @JonasToth wrote:
>
> > If whitelisting works, thats fine. But I agree with @lebedev.ri that a
> > contribution/improvement
lebedev.ri added a comment.
It seems this ended up silently being a catch-all, with no option to control
this behavior, and i don't see any comments discussing this..
Repository:
rL LLVM
https://reviews.llvm.org/D50447
___
cfe-commits mailing li
lebedev.ri updated this revision to Diff 160106.
lebedev.ri added a comment.
Rebase (ugh, bitrot), port `test/clang-tidy/hicpp-exception-baseclass.cpp`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D36892
Files:
test/clang-tidy/check_clang_tidy.py
test/clang-tidy/hicpp-exc
lebedev.ri updated this revision to Diff 160107.
lebedev.ri added a comment.
Add docs note.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D36892
Files:
docs/clang-tidy/index.rst
test/clang-tidy/check_clang_tidy.py
test/clang-tidy/hicpp-exception-baseclass.cpp
Index: test
lebedev.ri added a comment.
> We are aware that this test will cause warnings on users code through their
> dependencies on abseil.
> However, from what we know it seems like these warnings are normally
> suppressed.
> If anyone has a good idea on how to avoid this/has insight on whether this
lebedev.ri added a comment.
I suspect this might deserve a wider discussion. At least cfe-dev, perhaps?
Repository:
rCXX libc++
https://reviews.llvm.org/D50652
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/
lebedev.ri updated this revision to Diff 160458.
lebedev.ri added a comment.
Ping.
Rebased, now that the https://reviews.llvm.org/D50465 has landed, and we are
now able to properly optimize the ugliest case:
> This comes with `Implicit Conversion Sanitizer - integer sign change`
> (https://rev
lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.
Looks like NFC.
https://reviews.llvm.org/D50709
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
lebedev.ri added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7872-7874
-def warn_ret_addr_label : Warning<
- "returning address of label, which is local">,
- InGroup;
Why completely drop the diagnostic just because it is undesi
lebedev.ri added inline comments.
Comment at: include/bit:99
+ unsigned long where;
+ // Search from LSB to MSB for first set bit.
+ // Returns zero if no set bit is found.
Pretty sure this is the other way around here.
```
// Search from MSB to LSB for first
lebedev.ri added inline comments.
Comment at: llvm/include/llvm/Support/YAMLTraits.h:453
-inline bool isNumber(StringRef S) {
- static const char OctalChars[] = "01234567";
- if (S.startswith("0") &&
- S.drop_front().find_first_not_of(OctalChars) == StringRef::npos)
-
lebedev.ri requested changes to this revision.
lebedev.ri added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D50805#1202578, @Meinersbur wrote:
> In https://reviews.llvm.org/D50805#1201956, @Quuxplusone wrote:
>
> > If you added a new option `-Wret-addr-la
lebedev.ri added inline comments.
Comment at: llvm/unittests/Support/YAMLIOTest.cpp:2626
+ //
+ // * Sexagecimal numbers
+ // * Decimal numbers with comma s the delimiter
Spelling
Comment at: llvm/unittests/Support/YAMLIOTest.cpp:2627
+ //
lebedev.ri added a comment.
1. Please always upload all patches with full context.
2. There already is `modernize-use-auto`. Does it handle this case? Then this
should be just an alias to that check. Else, i think it would be best to extend
that one, and still alias.
https://reviews.llvm.org/D
lebedev.ri added a comment.
In https://reviews.llvm.org/D50852#1202781, @hugoeg wrote:
> what do you mean by "with full context"?
`-U9` when generating the diff.
https://reviews.llvm.org/D50852
___
cfe-commits mailing list
cfe-commits@lists.l
lebedev.ri added reviewers: aaron.ballman, zinovy.nis.
lebedev.ri added a comment.
In https://reviews.llvm.org/D50852#1203009, @hugoeg wrote:
> In https://reviews.llvm.org/D50852#1202774, @lebedev.ri wrote:
>
> > 1. Please always upload all patches with full context.
> > 2. There already is `mode
lebedev.ri added a comment.
In https://reviews.llvm.org/D50616#1203751, @rjmccall wrote:
> In https://reviews.llvm.org/D50616#1203692, @ebevhan wrote:
>
> >
>
>
> Has anyone actually asked LLVM whether they would accept fixed-point types
> into IR? I'm just a frontend guy, but it seems to me th
lebedev.ri added inline comments.
Comment at: include/bit:117
+ unsigned long __where;
// Search from LSB to MSB for first set bit.
// Returns zero if no set bit is found.
Like i commented in the original review, this should probably be
```
// Search from
lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, vsk, Sanitizers.
lebedev.ri added projects: Sanitizers, clang.
As per IRC disscussion, it seems we really want to have more fine-grained
`-fsanitize=implicit-integer-truncation`:
- A check when both of the types are unsigned.
lebedev.ri planned changes to this revision.
lebedev.ri added a comment.
Depends on https://reviews.llvm.org/D50901.
(which should land first, ideally.)
Repository:
rC Clang
https://reviews.llvm.org/D50250
___
cfe-commits mailing list
cfe-commits
lebedev.ri added a comment.
Looks ok to me.
https://reviews.llvm.org/D50924
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added inline comments.
Comment at: clang-tidy/abseil/AutoMakeUniqueCheck.cpp:21
+void AutoMakeUniqueCheck::registerMatchers(MatchFinder* finder) {
+ if (!getLangOpts().CPlusPlus) return;
+
zinovy.nis wrote:
> JonasToth wrote:
> > Please clang-format,
lebedev.ri added inline comments.
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+ auto Filename = FileEntry->getName();
+ llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");
hokein
lebedev.ri added inline comments.
Comment at: clang-tidy/abseil/AbseilMatcher.h:32
+ auto Filename = FileEntry->getName();
+ llvm::Regex RE("absl/(base|container|debugging|memory|meta|numeric|strings|"
+ "synchronization|types|utiliy)");
hokein
lebedev.ri added a comment.
Test coverage?
Also, please always upload all patches with full context (`-U9`)
https://reviews.llvm.org/D50766
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
lebedev.ri added inline comments.
Comment at:
test/CodeGen/catch-implicit-integer-truncations-basics-negatives.c:23
+__attribute__((no_sanitize("integer"))) unsigned char
blacklist_1_convert_unsigned_int_to_unsigned_char(unsigned int x) {
+ // CHECK: }
+ return x;
---
lebedev.ri updated this revision to Diff 162040.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.
Rebased, addressed @vsk review note, should be NFC.
@vsk thank you for taking a look!
> LebedevRI: It looks like it's in good shape. I wasn't around for
> the discussion re
lebedev.ri updated this revision to Diff 162050.
lebedev.ri added a subscriber: chandlerc.
lebedev.ri added a comment.
Rebased ontop of https://reviews.llvm.org/D50901, added
- ``-fsanitize=implicit-integer-arithmetic-value-change``: Catches implicit
conversions that change the arithmetic
lebedev.ri added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1050-1051
+// NOTE: if it is unsigned, then the comparison is naturally always
'false'.
+llvm::ICmpInst::Predicate Pred =
+VSigned ? llvm::ICmpInst::ICMP_SLT : llvm::ICmpInst::ICMP_ULT;
+
lebedev.ri created this revision.
lebedev.ri added reviewers: alexfh, sbenza.
Herald added subscribers: mgrang, xazax.hun.
Continuation of https://reviews.llvm.org/D46504.
Example output:
$ clang-tidy -enable-check-profile -store-check-profile=.
-store-check-profile-elide-prefix=. -checks=-*,
lebedev.ri created this revision.
lebedev.ri added reviewers: alexfh, sbenza, bkramer, george.karpenkov.
This is needed for the continuation of https://reviews.llvm.org/D46504,
to be able to store the timings as CSV.
The floating-point values are dumped with no precision loss.
See dependent diff
lebedev.ri added subscribers: cfe-commits, alexfh, lebedev.ri.
lebedev.ri raised a concern with this commit.
lebedev.ri added a comment.
Every single `.clang-tidy` config file that still happens to contain
`AnalyzeTemporaryDtors: true/false` param specified is now **silently** (!)
ignored,
and a
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1092084, @Eugene.Zelenko wrote:
> I think will be good idea to store data in JSON format too.
Yeah, i have thought about it, and i'm not sure.
The output is so dumb so there isn't even much point in using anything more
advanced tha
lebedev.ri added a comment.
In https://reviews.llvm.org/D46603#1092135, @george.karpenkov wrote:
> @lebedev.ri LLVM already has facilities for outputting statistics in JSON, it
> seems that would be sufficient for your purposes?
> I did something similar for the static analyzer in
> https://re
lebedev.ri planned changes to this revision.
lebedev.ri added inline comments.
Comment at: docs/clang-tidy/index.rst:762
+
+To enable profiling info collection, use ``-enable-check-profile`` argument.
+The timings will be outputted to the ``stderr`` as a table. Example output:
--
lebedev.ri updated this revision to Diff 145900.
lebedev.ri retitled this revision from "[Support] Print TimeRecord as CSV" to
"[Support] TimerGroup changes".
lebedev.ri edited the summary of this revision.
lebedev.ri added a reviewer: NoQ.
lebedev.ri added subscribers: xazax.hun, szepet, a.sidori
lebedev.ri updated this revision to Diff 145901.
lebedev.ri retitled this revision from "[clang-tidy] Store checks profiling
info as CSV files" to "[clang-tidy] Store checks profiling info as YAML files".
lebedev.ri added reviewers: george.karpenkov, NoQ.
lebedev.ri added a comment.
- Deduplicate
lebedev.ri updated this revision to Diff 145903.
lebedev.ri added a comment.
Add lock to now-public `printJSONValues()`.
I have no understanding of the `static TimerGroupList`,
but this is symmetrical with other [public] `TimerGroup::*print*` methods.
Repository:
rL LLVM
https://reviews.llvm
lebedev.ri updated this revision to Diff 145906.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
Herald added a subscriber: llvm-commits.
- Produce valid-er YAML.
Repository:
rL LLVM
https://reviews.llvm.org/D46602
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/Cl
lebedev.ri added inline comments.
Comment at: docs/clang-tidy/index.rst:783
+ {
+"time.clang-tidy.readability-function-size.wall": 1.0421266555786133e+00,
+"time.clang-tidy.readability-function-size.user": 9.208840005421e-01,
Hmm, thinking about it a
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1092883, @alexfh wrote:
> In https://reviews.llvm.org/D46602#1092111, @rja wrote:
>
> > +1 for JSON
>
>
> Could you explain why would you use YAML or JSON for this? How are you going
> to use/process this data?
I personally don't h
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1092890, @alexfh wrote:
> Roman, it looks to me that a simpler storage scheme would be sufficient. For
> example, MMDDhhmmss-InputFileName.cpp.csv.
> Main things are:
>
> 1. include a timestamp, so there's no need to overwrite o
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1092902, @lebedev.ri wrote:
> In https://reviews.llvm.org/D46602#1092890, @alexfh wrote:
>
> > Roman, it looks to me that a simpler storage scheme would be sufficient.
> > For example, MMDDhhmmss-InputFileName.cpp.csv.
> > Main
lebedev.ri updated this revision to Diff 145994.
lebedev.ri added a comment.
- Use sane (not the same as the one right before..) suffix for when
json-printing mem usage. Admittedly, i haven't tried it, but it just does not
make sense otherwise.
Repository:
rL LLVM
https://reviews.llvm.org/D
lebedev.ri updated this revision to Diff 145995.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
- Make json less flat, store source filename in it.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46602
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/C
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1095960, @alexfh wrote:
> In https://reviews.llvm.org/D46602#1092902, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D46602#1092890, @alexfh wrote:
> >
> > > Roman, it looks to me that a simpler storage scheme would be sufficien
lebedev.ri added inline comments.
Comment at: lib/Frontend/FrontendActions.cpp:779
+ {
+std::string Str;
+#define FEATURE(Name, Predicate)
\
This is likely to do an allocation for each feature.
Maybe consider `l
lebedev.ri added a comment.
1. Please always upload all patches with full context.
2. tests?
Repository:
rC Clang
https://reviews.llvm.org/D46805
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
lebedev.ri added inline comments.
Comment at: lib/Frontend/FrontendActions.cpp:779
+ {
+std::string Str;
+#define FEATURE(Name, Predicate)
\
aaron.ballman wrote:
> lebedev.ri wrote:
> > This is likely to do an a
lebedev.ri added a comment.
In https://reviews.llvm.org/D45766#1097736, @ksu.shadura wrote:
> Hi,
Hi.
> we see the false-positive behavior of -Wno-self-assign-overloaded flag in
> case of subtraction assignment operator.
> The minimal reproducer that we got: https://godbolt.org/g/8PQMpR
>
lebedev.ri added a comment.
In https://reviews.llvm.org/D45766#1097797, @ksu.shadura wrote:
> Thank you for the test example! I got your point, but I wanted to ask if it
> should be like this for commutative operations?
> In our case it is actually matrix, and subtraction of matrices is not
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1097902, @alexfh wrote:
> In https://reviews.llvm.org/D46602#1095980, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D46602#1095960, @alexfh wrote:
> >
> > > In https://reviews.llvm.org/D46602#1092902, @lebedev.ri wrote:
> > >
>
lebedev.ri updated this revision to Diff 146644.
lebedev.ri edited the summary of this revision.
lebedev.ri added subscribers: aaron.ballman, JonasToth.
lebedev.ri added a comment.
- Get rid of 'prefix elision'
- Don't use full source file name, only the filename
- Prefix that filename with ISO860
lebedev.ri added a project: clang-tools-extra.
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1098092, @alexfh wrote:
> In https://reviews.llvm.org/D46602#1097954, @lebedev.ri wrote:
>
> > How do i reflect that in tests? The output name will basically be random.
>
>
> Why random?
lebedev.ri added a comment.
Ping.
Any thoughts on these Timer changes?
Repository:
rL LLVM
https://reviews.llvm.org/D46603
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added a comment.
Hi @leonardchan.
Could you please prepend some appropriate tag to the subjects of all your
differentials, so it is less hard to read through cfe-commits mails, please?
Repository:
rC Clang
https://reviews.llvm.org/D46927
lebedev.ri created this revision.
lebedev.ri added reviewers: george.karpenkov, NoQ, alexfh, sbenza.
Herald added a subscriber: llvm-commits.
lebedev.ri added a dependent revision: D46938: [Timers] TimerGroup: make
printJSONValues() method public.
lebedev.ri added a dependency: D46936: [Timers] Ti
lebedev.ri created this revision.
lebedev.ri added reviewers: george.karpenkov, NoQ, alexfh, sbenza.
Herald added a subscriber: llvm-commits.
lebedev.ri added a dependent revision: D46937: [Timers]
TimerGroup::printJSONValue(): print doubles with no precision loss.
We have just used `.sys` suffix
lebedev.ri created this revision.
lebedev.ri added reviewers: george.karpenkov, NoQ, alexfh, sbenza.
Herald added a subscriber: llvm-commits.
lebedev.ri added a dependency: D46938: [Timers] TimerGroup: make
printJSONValues() method public.
lebedev.ri added a dependent revision: D46602: [clang-tidy
lebedev.ri created this revision.
lebedev.ri added reviewers: george.karpenkov, NoQ, alexfh, sbenza.
Herald added a subscriber: llvm-commits.
lebedev.ri added a dependent revision: D46939: [Timers] TimerGroup: add
constructor from StringMap.
lebedev.ri added a dependency: D46937: [Timers] TimerGro
lebedev.ri abandoned this revision.
lebedev.ri added a comment.
In https://reviews.llvm.org/D46603#1100455, @george.karpenkov wrote:
> I see four separate changes: s/.sys/mem one (can be committed without
> review), exposing printJSONValues and consequent adding of a lock, adding a
> constructo
lebedev.ri updated this revision to Diff 147071.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
- Rebased
- Slightly refactor how the profile storage params are computed, move that into
`ClangTidyProfiling::StorageParams` helper struct.
- Store timestamp in the json t
lebedev.ri added a comment.
In https://reviews.llvm.org/D45766#1101533, @ksu.shadura wrote:
> Thanks for your opinion, your guess was right! It is a unit-test, so seems we
> need to try to suppress warnings on our side.
Ok, great that this time it got resolved this trivially!
> In https://rev
lebedev.ri updated this revision to Diff 147120.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.
Rename getter/setter functions.
Thank you for taking a look!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46602
Files:
clang-tidy/ClangTidy.cpp
clang
lebedev.ri added a comment.
In https://reviews.llvm.org/D46603#1101047, @lebedev.ri wrote:
> In https://reviews.llvm.org/D46603#1100455, @george.karpenkov wrote:
>
> > I see four separate changes: s/.sys/mem one (can be committed without
> > review), exposing printJSONValues and consequent addin
lebedev.ri added inline comments.
Comment at: clang-tidy/tool/ClangTidyMain.cpp:188
+format to stderr. When this option is passed,
+these per-TU profiles are instead stored as YAML.)"),
+ cl::value_desc("prefix"),
Quux
lebedev.ri added a comment.
No test?
Repository:
rL LLVM
https://reviews.llvm.org/D47191
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added a comment.
That is what happens for "hand-rolled" `abs`, https://godbolt.org/g/6dbgXD
I *think* this makes sense, since **IIRC** LLVM only supports twos-complement
platforms.
But all these attributes are currently beyond me, so i won't click the accept
button :)
https://review
lebedev.ri added a comment.
Ping.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46602
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added reviewers: aaron.ballman, john.brawn, mehdi_amini, sammccall,
zturner, bkramer.
lebedev.ri added a comment.
The testcase sounds reasonable. Can't comment on the actual fix.
Adding some more reviewers that potentially worked on that source file.
Repository:
rC Clang
https://r
lebedev.ri added inline comments.
Comment at: lib/Tooling/CompilationDatabase.cpp:303
+ // Remove temp files.
+ Compilation->CleanupFileList(Compilation->getTempFiles());
+
dstenb wrote:
> erichkeane wrote:
> > It seems to me that this would be best called from
lebedev.ri abandoned this revision.
lebedev.ri added a comment.
https://reviews.llvm.org/D46187#1098571
Sad, but ok.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46188
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:/
lebedev.ri abandoned this revision.
lebedev.ri added a comment.
Sad, but ok.
Repository:
rC Clang
https://reviews.llvm.org/D46187
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added inline comments.
Comment at: include/clang/Driver/Compilation.h:126
+ /// Whether to save temporary files.
+ bool SaveTempsEnabled;
+
`KeepTempFiles`?
Comment at: lib/Driver/Compilation.cpp:276-277
+
+ // Temporary files add
lebedev.ri added a comment.
Please improve test coverage a bit.
`-Wunused-using` is part of `-Wunused`.
And `-Wunused` is part of `-Wall`.
That needs to be tested.
Also, one small test to show that it is not on by default,
and `-Wno-unused-using` actually disables it is needed.
===
lebedev.ri added a comment.
LGTM, but i'm quite unfamiliar with this area of the code,
so please wait for someone else to accept :)
Comment at: lib/Driver/Compilation.cpp:276-277
+
+ // Temporary files added by diagnostics should be kept.
+ SaveTempsEnabled = true;
}
---
lebedev.ri added a comment.
In https://reviews.llvm.org/D45686#1113425, @Ka-Ka wrote:
> In https://reviews.llvm.org/D45686#1113414, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D45686#1109295, @dstenb wrote:
> >
> > > Ping.
> > >
> > > We have added a lit reproducer for this now in cla
lebedev.ri added a comment.
In https://reviews.llvm.org/D38171#880016, @xazax.hun wrote:
> In https://reviews.llvm.org/D38171#878814, @lebedev.ri wrote:
>
> > I feel like the current handling of the clang-tidy check aliases needs
> > adjusting.
> > If one enables all the checks (`Checks: '*'`),
lebedev.ri added a comment.
In https://reviews.llvm.org/D38171#880025, @xazax.hun wrote:
> In https://reviews.llvm.org/D38171#880022, @lebedev.ri wrote:
>
> > A mail [0] posted by @JonasToth is about the similar problem, i think we
> > can continue there.
>
>
> Great! Could you summarize your po
lebedev.ri updated this revision to Diff 116857.
lebedev.ri added a comment.
Rebased, ping.
Vanilla stage-2 build is now warning-clean, the only previous warning was fixed.
Repository:
rL LLVM
https://reviews.llvm.org/D38101
Files:
docs/ReleaseNotes.rst
include/clang/Basic/DiagnosticGro
lebedev.ri added a comment.
Ping.
In https://reviews.llvm.org/D36836#877642, @lebedev.ri wrote:
> In https://reviews.llvm.org/D36836#877636, @alexfh wrote:
>
> > > The metric is implemented as per COGNITIVE COMPLEXITY by SonarSource
> > > specification version 1.2 (19 April 2017),
> >
> > Err,
lebedev.ri added a comment.
Ping :)
Does this need more, different tests?
Should i rewrite that lengthy if-else-if chain somehow differently?
Repository:
rL LLVM
https://reviews.llvm.org/D38101
___
cfe-commits mailing list
cfe-commits@lists.llvm.
lebedev.ri updated this revision to Diff 117710.
lebedev.ri marked 6 inline comments as done.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
@rsmith thank you for the review!
Address review notes:
1. Merge `CheckTautologicalComparisonWithZero()` and
`CheckTautologic
lebedev.ri added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:8697
+
+ return true;
}
If the diagnostic we are about to output is disabled, should we still `return
true;` and suppress potential `-Wsign-compare` warning?
Repository:
rL LLVM
https
lebedev.ri updated this revision to Diff 117802.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a subscriber: rtrieu.
lebedev.ri added a comment.
- Address review notes
- ~~Avoid re-evaluating ICE when called from `DiagnoseOutOfRangeComparison()`
which already knows the evaluated v
lebedev.ri added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:8719
+ // Type limit values are handled later by CheckTautologicalComparison().
+ if (IsTypeLimit(S, Other, Constant, ConstValue) && (!OtherIsBooleanType))
return;
rsmith wrote:
> Is t
lebedev.ri updated this revision to Diff 117824.
lebedev.ri added a comment.
Ping.
- Rebased
- Added the legal status info into `LICENSE.txt`, referencing
`clang-tidy/readability/FunctionCognitiveComplexityCheck.LICENSE.txt`, as
suggested by @aaron.ballman
Question:
Is it expected that clang-t
lebedev.ri added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:8719
+ // Type limit values are handled later by CheckTautologicalComparison().
+ if (IsTypeLimit(S, Other, Constant, ConstValue) && (!OtherIsBooleanType))
return;
lebedev.ri wrote:
>
lebedev.ri added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:8719
+ // Type limit values are handled later by CheckTautologicalComparison().
+ if (IsTypeLimit(S, Other, Constant, ConstValue) && (!OtherIsBooleanType))
return;
lebedev.ri wrote:
>
lebedev.ri updated this revision to Diff 118143.
lebedev.ri added a comment.
Rework `AnalyzeComparison()` and `DiagnoseOutOfRangeComparison()` - avoid that
fast-return path in `DiagnoseOutOfRangeComparison()`
`AnalyzeComparison()` is now a bit smarter, and it
1. calls `CheckTautologicalCompariso
lebedev.ri added inline comments.
Comment at: test/Sema/conditional-expr.c:84
+ //char x;
+ return (((x != ((void *) 0)) ? (*x = ((char) 1)) : (void) ((void *) 0)),
(unsigned long) ((void *) 0)); // expected-warning {{C99 forbids conditional
expressions with only one void sid
lebedev.ri updated this revision to Diff 118793.
lebedev.ri marked 4 inline comments as done.
lebedev.ri added a comment.
@rsmith, thank you for the review!
Rebased, address review notes, simplified all the things even further.
Testing:
- `$ ninja check-clang-sema check-clang-semacxx`
- Stage 2
lebedev.ri added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:8667-8679
+ bool Result; // The result of the comparison
+ if ((Op == BO_GT && ValueType == LimitType::Max && ConstOnRight) ||
+ (Op == BO_GT && ValueType == LimitType::Min && !ConstOnRight) ||
+
lebedev.ri added a comment.
In https://reviews.llvm.org/D36836#889375, @aaron.ballman wrote:
> Adding @dberlin for licensing discussion questions.
Ping.
Yes, i agree that what i have added is not a directory, and not a proper
license.
That is more of a template to hopefully stat moving things
lebedev.ri updated this revision to Diff 118829.
lebedev.ri marked an inline comment as done.
Repository:
rL LLVM
https://reviews.llvm.org/D38101
Files:
docs/ReleaseNotes.rst
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
lebedev.ri added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:8930
+// We only care about expressions where just one side is literal
+if (IsRHSIntegralLiteral ^ IsLHSIntegralLiteral) {
+ // Is the constant on the RHS or LHS?
rsmith wrote:
>
lebedev.ri updated this revision to Diff 118833.
lebedev.ri added a comment.
Getting really weird problems when trying to use arc patch, maybe re-updating
the differential helps
Repository:
rL LLVM
https://reviews.llvm.org/D38101
Files:
docs/ReleaseNotes.rst
include/clang/Basic/Diagnost
lebedev.ri reopened this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.
Reverted due to http://bb9.pgr.jp/#/builders/20/builds/59 that i don't
currently know how to deal with.
It is really sad that i failed to encounter it during testing.
Repository:
rL
lebedev.ri added inline comments.
Comment at: cfe/trunk/test/Sema/tautological-constant-compare.c:23
+
+ if (c > macro(255))
+ return;
I'm having second thoughts about macro handling.
Right now we completely ignore the comparisons when the constant is anyho
1 - 100 of 2250 matches
Mail list logo
