elsteveogrande accepted this revision.
elsteveogrande added a reviewer: elsteveogrande.
elsteveogrande added a comment.
This revision is now accepted and ready to land.
Thought I'd take a look at the list of diffs and take a stab at a few (a few
easy-looking and short ones :) ) to unblock some pp
On Mon, Oct 03, 2016 at 07:10:08AM +, Roger Ferrer Ibanez wrote:
> Hi Joerg,
>
> thanks for your comments. I agree that these false positives are annoying.
>
> I submitted a while ago a patch to address those false positives in
> https://reviews.llvm.org/D23657 but it is pending approval.
I'
- Original Message -
> From: "Artem Dergachev"
> To: "Hal Finkel" , "Artem Dergachev"
>
> Cc: cfe-commits@lists.llvm.org
> Sent: Monday, October 3, 2016 3:40:02 PM
> Subject: Re: r283141 - [analyzer] A blind attempt to fix a buildbot after
> r283092.
>
> 03/10/2016 23:29, Hal Finkel пи
aaron.ballman added inline comments.
> SemaChecking.cpp:11284
> +if (ND->getName().empty()) {
> + if (const auto *TypedefDecl = m.RD->getTypedefNameForAnonDecl())
> +ND = TypedefDecl;
Please don't use `auto` here since the type is not spelled directly in the
initialization.
>
javed.absar created this revision.
javed.absar added reviewers: rengolin, jmolloy, t.p.northover.
javed.absar added a subscriber: cfe-commits.
Herald added subscribers: samparker, aemerson.
The following patch adds Cortex-R52, the new ARM real-time processor, to CLANG.
Cortex-R52 implements the
arphaman set the repository for this revision to rL LLVM.
arphaman updated this revision to Diff 73338.
arphaman marked 2 inline comments as done.
arphaman added a comment.
Update to the patch that follows Aaron's comments:
- Uses the explicit type name instead of auto.
- Expands the test by addi
malcolm.parsons added inline comments.
> cppcoreguidelines-one-name-per-declaration.cpp:8
> + {
> +int x = 42, y = 43;
> +// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: Do not declare multiple
> names per declaration [cppcoreguidelines-one-name-per-declaration]
The guideline says "Flag no
rsmith added a comment.
OK, so it seems like all the other approaches we discussed have problems too.
Let's move forward with this for now.
Is there some reasonable base set of functionality between this and
JumpDiagnostics that could be factored out and shared?
> VarBypassDetector.cpp:45
>
kcc added a reviewer: pcc.
kcc added a comment.
In https://reviews.llvm.org/D25199#559407, @vsk wrote:
> It looks like programs which trip -fsanitize-value-after-delete will just
> crash without further reporting, which isn't in keeping with the way other
> ubsan checks are implemented.
>
> IMO
kcc added a comment.
>> will just crash without further reporting
I agree, and we can address that by having special logic in ubsan's segv
handler.
This does not have to be in this patch.
Also, I am not sure about the actual constant.
DEADBEEF is commonly recognized poison valued, but on a
Author: mren
Date: Mon Oct 3 16:26:46 2016
New Revision: 283145
URL: http://llvm.org/viewvc/llvm-project?rev=283145&view=rev
Log:
ObjectiveC: fix a seg fault when deserialing redeclaration of ObjCMethodDecl.
The deserialization of redeclartion can cause seg fault since getCanonicalDecl
of the re
hfinkel added inline comments.
> rjmccall wrote in CGStmt.cpp:525
> It's much more likely that NoAliasScopes will be empty than that MemoryInsts
> will be empty. You should probably fast-path using that, or better yet, with
> the RecordMemoryInsts bit.
I'm not sure that's true; we only record
hfinkel updated this revision to Diff 73344.
hfinkel added a comment.
Rebased; added more comments and addressed other review feedback.
https://reviews.llvm.org/D9403
Files:
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGStmt.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGe
hfinkel updated this revision to Diff 73345.
hfinkel added a comment.
Rebased
https://reviews.llvm.org/D22189
Files:
lib/CodeGen/CGDeclCXX.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGOpenMPRuntime.cpp
lib/CodeGen/CGStmtOpenMP.cpp
lib/CodeGen/CodeGenFunction.h
Index: lib/CodeGen/CodeGen
vsk added a comment.
In https://reviews.llvm.org/D25199#559797, @kcc wrote:
> >> will just crash without further reporting
>
> I agree, and we can address that by having special logic in ubsan's segv
> handler.
> This does not have to be in this patch.
@kcc Is it safe to add a handler for
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
A couple of comment nits, then this LGTM. Thanks!
> SemaDecl.cpp:8659-8664
> + // void func();
> + // template class C1 { friend void func() { } };
> + // template class
pcc added a reviewer: rsmith.
pcc added a comment.
It seems to me that this sanitizer would break the semantics of otherwise
well-defined programs. For example:
int *x = nullptr;
delete x;
if (x != nullptr) {
// normally unreachable
}
It may be that a null comparison would be enough
STL_MSFT added a comment.
stack_allocator.h is inconsistent about saying plain size_t or std::size_t.
stack_allocator.h should include type_traits for integral_constant.
stack_allocator.h refers to stack_allocator as both a class and a struct. It
should be consistent (not just because MSVC comp
arphaman created this revision.
arphaman added reviewers: rsmith, rjmccall.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.
This patch fixes a crash that happens because of an assertion failure in C mode
only. The assertion failure happens becau
vsk added a comment.
In https://reviews.llvm.org/D25199#559849, @pcc wrote:
> It seems to me that this sanitizer would break the semantics of otherwise
> well-defined programs. For example:
>
> int *x = nullptr;
> delete x;
> if (x != nullptr) {
> // normally unreachable
> }
>
>
> It
GorNishanov updated this revision to Diff 73350.
GorNishanov added a comment.
Herald added a subscriber: modocache.
1. https://reviews.llvm.org/owners/package/2/ test removed
2. Update LanguageExtensions.rst with suggested edits
3. coro-builtins.c test now passes useful arguments and pattern match
Can't reproduce on my Windows.
On Mon, Oct 3, 2016 at 1:31 PM Artem Dergachev wrote:
> I also made a quick blind guess at r283141 (
> http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20161003/172390.html
> )
>
> Thanks a lot for trying to help, sorry for causing prob
rsmith added inline comments.
> SemaOverload.cpp:11750
> ExprResult
> Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
> BinaryOperatorKind Opc,
We should never be calling this function outside of C++ mode.
It looks like the bug is in `Sema::BuildBinOp` -- in C,
spatel marked 2 inline comments as done.
spatel added a comment.
Thanks, Eric. I actually drafted this with the name "recip-estimates", but
thought there might be value in reusing the programmer-visible flag name. I'm
good with "reciprocal-estimates" too.
https://reviews.llvm.org/D24815
___
spatel added inline comments.
> echristo wrote in CGCall.cpp:1735
> Would be nice to get these pulled into a single fast-math string that's set
> and then used all over for sure. :)
I'm probably not imagining some use case, but I was hoping that we can just
delete the 4 (fast/inf/nan/nsz) that
rsmith added inline comments.
> SemaChecking.cpp:11053-11056
> + if ((RequiredAlignment > AlignRecord) ||
> + ((Context.toCharUnitsFromBits(
> +Context.getFieldOffset(cast(MD))) %
> +RequiredAlignment) != 0)) {
This doesn't seem to correctly handle the c
spatel updated this revision to Diff 73364.
spatel added a comment.
Patch updated as suggested by Eric:
1. The attribute is named "reciprocal-estimates".
2. Remove unnecessary -disable-llvm-optzns flag from test file.
Quick fixes, but this will not go in until the LLVM side
(https://reviews.llv
Author: gornishanov
Date: Mon Oct 3 17:44:48 2016
New Revision: 283155
URL: http://llvm.org/viewvc/llvm-project?rev=283155&view=rev
Log:
[coroutines] Adding builtins for coroutine intrinsics and backendutil support.
Summary:
With this commit simple coroutines can be created in plain C using coro
elsteveogrande added a comment.
Ping :) Hoping someone on cfe-commits might be able to take a quick look...
Repository:
rL LLVM
https://reviews.llvm.org/D25153
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/
joerg added a comment.
The following is from my comment on the original commit.
I'm trying this patch now. Two instances here show false positives. Some
others are misuse/overuse of __packed, it is time consuming to check all
of them.
- sbin/route
https://nxr.netbsd.org/xref/src/sbin/route/rout
GorNishanov closed this revision.
GorNishanov added a comment.
Closed by commit r283155 - [coroutines] Adding builtins for coroutine
intrinsics and backendutil support.
For some reason phabricator did not close it automatically.
https://reviews.llvm.org/D24373
__
rnk added inline comments.
> Targets.cpp:2319
> + { #ID, TYPE, ATTRS, HEADER, LANGS, FEATURE },
> +#include "clang/Basic/BuiltinsX86.def"
> +
I'd rather not duplicate this readonly data. I had this clever idea that we'd
do something like:
const Builtin::Info BuiltinInfo[] = {
...
#inclu
rnk added inline comments.
> agutowski wrote in BuiltinsX86_32.def:1
> Will we ever need that file? It seems to nicely complement the 64-bit one,
> but if it's going to be empty forever maybe it's better to remove it.
I guess we should remove it. Take a look at isX86_64Builtin in
Sema/SemaChec
rsmith added inline comments.
> PrintPreprocessedOutput.cpp:320-321
> + * if we're emitting a '#'-style directive (escape both CR and LF). Paths
> are quoted in some
> + * instances, so escape quotes. Escaping is with a backslash (and
> backslashes themselves have
> + * to be escaped); for CR
elsteveogrande added a comment.
Also the other issues are old comments, now fixed with the latest updates to
this. This does now take the value of the include token: `include` or
`include_once` or `import` etc.; and send that to output.
Thanks very much @rsmith ! (Sorry to have been a pest.)
echristo added inline comments.
> spatel wrote in CGCall.cpp:1735
> I'm probably not imagining some use case, but I was hoping that we can just
> delete the 4 (fast/inf/nan/nsz) that are already covered by instruction-level
> FMF. An auto-upgrade might be needed within LLVM...and/or a big pile
echristo added a comment.
Yes, none of this should be added or expanded in TargetOptions.
Thanks!
-eric
https://reviews.llvm.org/D24909
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
Author: marshall
Date: Mon Oct 3 18:39:52 2016
New Revision: 283161
URL: http://llvm.org/viewvc/llvm-project?rev=283161&view=rev
Log:
Add tests to make sure that is_constructible is false. We already
checked 'unqualified void'. This was brought up by LWG#2738
Modified:
libcxx/trunk/test/st
Author: marshall
Date: Mon Oct 3 18:40:48 2016
New Revision: 283162
URL: http://llvm.org/viewvc/llvm-project?rev=283162&view=rev
Log:
Change a couple of 'template http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string?rev=283162&r1=283161&r2=283162&view=diff
=
Author: marshall
Date: Mon Oct 3 18:42:31 2016
New Revision: 283163
URL: http://llvm.org/viewvc/llvm-project?rev=283163&view=rev
Log:
Mark a couple more Issaquah issues as done: 2578 and 2738
Modified:
libcxx/trunk/www/upcoming_meeting.html
Modified: libcxx/trunk/www/upcoming_meeting.html
U
kcc added a comment.
> Maybe we could call this `-fpoison-dangling-ptrs` and force users to be more
> explicit about opting into this behavior change. That would remove some of
> the constraints usually placed on new sanitizer checks (e.g support for
> executing after the error triggers, supp
dberris added a comment.
In https://reviews.llvm.org/D24799#55, @rSerge wrote:
> @dberris , could you deliver this patch to mainline? Or do we need approval
> from more reviewers?
Yes, I can do this -- sorry it was the long weekend for me down under. I'll get
to this today. :)
https://
agutowski updated this revision to Diff 73375.
agutowski added a comment.
remove BuiltinsX86_32.def; reduce redundancy in x86 builtins info
https://reviews.llvm.org/D24598
Files:
include/clang/Basic/BuiltinsX86_64.def
include/clang/Basic/TargetBuiltins.h
lib/Basic/Targets.cpp
lib/CodeGe
mehdi_amini added inline comments.
> echristo wrote in CGCall.cpp:1735
> Would be nice to get these pulled into a single fast-math string that's set
> and then used all over for sure. :)
I agree with getting on a path to remove these function attributes that have an
equivalent on per-instructi
Eugene.Zelenko added a comment.
Looks like patch was not committed.
https://reviews.llvm.org/D3771
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vsk added a comment.
In https://reviews.llvm.org/D25199#560023, @kcc wrote:
> > Maybe we could call this `-fpoison-dangling-ptrs` and force users to be
> > more explicit about opting into this behavior change. That would remove
> > some of the constraints usually placed on new sanitizer checks
spatel added inline comments.
> mehdi_amini wrote in CGCall.cpp:1735
> I agree with getting on a path to remove these function attributes that have
> an equivalent on per-instruction flag.
>
> I wonder what is the status of these flags in SelectionDAG though? We still
> have a variant of the f
GorNishanov updated this revision to Diff 73378.
GorNishanov marked an inline comment as done.
GorNishanov added a comment.
Implemented review feedback. Landing is imminent.
https://reviews.llvm.org/D25068
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sem
rnk created this revision.
rnk added a reviewer: rsmith.
rnk added a subscriber: cfe-commits.
Other compilers accept invalid code here that we reject, and we need a
better error message to try to convince users that the code is really
incorrect. Consider:
class Foo {
typedef MyIterHelper it
On Sat, Oct 1, 2016 at 9:34 AM, Joerg Sonnenberger via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> On Fri, Sep 30, 2016 at 03:41:53PM -0700, Richard Trieu via cfe-commits
> wrote:
> > Currently, this warning is on by default. As you said, the results you
> > found look intentional in many
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.
Committed in https://reviews.llvm.org/rL205810.
https://reviews.llvm.org/D3065
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
Author: gornishanov
Date: Mon Oct 3 19:31:16 2016
New Revision: 283170
URL: http://llvm.org/viewvc/llvm-project?rev=283170&view=rev
Log:
[coroutines] Switch to using std::experimental namespace per P0057R5
Summary:
Look for coroutine_traits and friends in std::experimental namespace.
Patch (most
This warning group (-Wshift-op-parentheses) is already a subgroup of
-Wparentheses. The suggestion of moving to a sub group was to create a new
subgroup, either as a child of -Wshift-op-parentheses or -Wparentheses, so
that this new warning can be turned on and off independently and allow it
to be
elsteveogrande added inline comments.
> elsteveogrande wrote in PrintPreprocessedOutput.cpp:320-321
> right you are! I was thinking of `//` when I was doing that escaping logic.
>
> I should either escape `*/`, or even better, just use `//` for comments, is
> that ok? (I feel like `//` is mor
mehdi_amini added inline comments.
> spatel wrote in CGCall.cpp:1735
> Good point - I think we have to convert all codegen tests that have these
> function-level attributes to IR FMF and make sure that the output doesn't
> change. Definitely not part of this patch, but hopefully something that
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.
Committed in https://reviews.llvm.org/rL200239.
https://reviews.llvm.org/D2600
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.
Committed in https://reviews.llvm.org/rL194085.
https://reviews.llvm.org/D2088
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.
Committed in https://reviews.llvm.org/rL194123.
https://reviews.llvm.org/D2076
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.
Committed in https://reviews.llvm.org/rL194124.
https://reviews.llvm.org/D2075
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.
Committed in https://reviews.llvm.org/rL194119.
https://reviews.llvm.org/D2072
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
Eugene.Zelenko closed this revision.
Eugene.Zelenko added a comment.
Committed in https://reviews.llvm.org/rL192361.
https://reviews.llvm.org/D1885
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinf
Author: vitalybuka
Date: Mon Oct 3 21:19:12 2016
New Revision: 283179
URL: http://llvm.org/viewvc/llvm-project?rev=283179&view=rev
Log:
Revert "[analyzer] A blind attempt to fix a buildbot" as it does not help.
This reverts commit r283141.
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Co
Author: vitalybuka
Date: Mon Oct 3 21:19:17 2016
New Revision: 283180
URL: http://llvm.org/viewvc/llvm-project?rev=283180&view=rev
Log:
Revert "[analyzer] Extend bug reports with extra notes" to fix Windows bot.
This reverts commit r283092.
Modified:
cfe/trunk/include/clang/StaticAnalyzer/C
rtrieu updated this revision to Diff 73388.
rtrieu added a comment.
Add a more detailed error message to let users know where the two records
differ. This replaces the generic error which only stated that two definitions
are different without any details.
https://reviews.llvm.org/D21675
File
rtrieu marked 2 inline comments as done.
rtrieu added inline comments.
> dblaikie wrote in DeclBase.cpp:1810-1812
> Inconsistent {} on single line block (in VisitEnumConstantDecl above {} are
> not used on a single line block) - usually drop the {} on single line blocks.
>
> (several other inst
Author: vitalybuka
Date: Mon Oct 3 21:36:58 2016
New Revision: 283181
URL: http://llvm.org/viewvc/llvm-project?rev=283181&view=rev
Log:
Revert "[analyzer] Add extra notes to ObjCDeallocChecker" as its depends on
reverted r283092
This reverts commit r283093.
Modified:
cfe/trunk/lib/StaticAn
Author: vitalybuka
Date: Mon Oct 3 21:40:35 2016
New Revision: 283182
URL: http://llvm.org/viewvc/llvm-project?rev=283182&view=rev
Log:
Revert "[analyzer] Improve CloneChecker diagnostics" as its depends on reverted
r283092
This reverts commit r283094.
Removed:
cfe/trunk/test/Analysis/copy
vitalybuka added a comment.
reverted with r283182
https://reviews.llvm.org/D24916
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vitalybuka added a comment.
Reverted with r283181 as it depends r283092
https://reviews.llvm.org/D24915
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
vitalybuka added a comment.
reverted with r283182 as it depends on r283092
https://reviews.llvm.org/D24916
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
GorNishanov closed this revision.
GorNishanov added a comment.
Closed by: r283170 - [coroutines] Switch to using std::experimental namespace
per P0057R5
https://reviews.llvm.org/D25068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http:/
aprantl added a comment.
Not knowing the specifics of the ObjC class layout: Does this work correctly
with private ivars (i.e.: Does this need an extra check that there are no extra
ivars in a later @implementation)?
https://reviews.llvm.org/D7
__
rsmith added a comment.
There are a bunch of cases here that do this:
if (auto t = getThing())
ID.addThing(t);
if (auto t = getOtherThing())
ID.addThing(t);
That will result in hash collisions between objects with thing and objects with
otherthing (for instance, `struct A { int n :
I also made a quick blind guess at r283141
(http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20161003/172390.html)
Thanks a lot for trying to help, sorry for causing problems.
03/10/2016 23:26, Vitaly Buka пишет:
I will look into this and get back to you.
On Mon,
elsteveogrande updated this revision to Diff 73394.
elsteveogrande updated the summary for this revision.
elsteveogrande added a comment.
Fixed escaping function; simplified that function as well as the diagnostic
output.
https://reviews.llvm.org/D25153
Files:
include/clang/Driver/Options.td
majnemer added inline comments.
> PrintPreprocessedOutput.cpp:321
> + */
> +std::string sanitizePath(const StringRef& path) {
> + std::string str(path.size() * 2, '\0');
Don't pass `StringRef` by const reference, just pass it by value.
> PrintPreprocessedOutput.cpp:321-325
> +std::string sanit
elsteveogrande updated this revision to Diff 73398.
elsteveogrande added a comment.
Thanks for the feedback @majnemer! Fixed these. I _think_ I used `StringRef`
right; I assume it's like `std::string` in that it manages its own memory (i.e.
I didn't create references to deallocated strings, et
mgorny abandoned this revision.
mgorny added a comment.
This is no longer necessary since updated patch to
https://reviews.llvm.org/D23755 handles all the projects using the functions.
https://reviews.llvm.org/D23832
___
cfe-commits mailing list
cf
hfinkel created this revision.
hfinkel added reviewers: anemet, rsmith, rjmccall.
hfinkel added a subscriber: cfe-commits.
Herald added a subscriber: mcrosier.
The backend now has the capability to save information from optimizations, the
same information that can be used to generate optimization
hfinkel added a dependency: D25224: Don't filter diagnostics written as YAML to
the output file.
hfinkel added a comment.
Note: This depends on https://reviews.llvm.org/D25224.
https://reviews.llvm.org/D25225
___
cfe-commits mailing list
cfe-commit
101 - 180 of 180 matches
Mail list logo
