[PATCH] D39903: [libclang] Allow pretty printing declarations
jbcoe added inline comments. Comment at: include/clang-c/Index.h:4118 + */ +CINDEX_LINKAGE unsigned clang_PrintingPolicy_getIndentation(CXPrintingPolicy); + Could one use an enum to get/set different properties of the policy? ``` clang_PrintingPolicy_get(CXPrintingPolicy Policy, unsigned Property); clang_PrintingPolicy_set(CXPrintingPolicy Policy, unsigned Property, unsigned Value); ``` I've seen other C-API's (for Linear and Quadratic programming) follow a similar approach quite extensibly. It would significantly reduce the size of the API. Repository: rC Clang https://reviews.llvm.org/D39903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41792: [AArch64] Add ARMv8.2-A FP16 scalar intrinsics
SjoerdMeijer added inline comments.
Comment at: clang/include/clang/Basic/arm_fp16.td:58
+class IInst : Inst {}
+
+// ARMv8.2-A FP16 intrinsics.
az wrote:
> SjoerdMeijer wrote:
> > There's a little bit of duplication here: the definitions above are the
> > same as in arm_neon.td. Would it be easy to share this, with e.g. an
> > include?
> The duplication is small compared to the overall infrastructure/data
> structure needed to automatically generate the intrinsics. There are 3 ways
> to do this: 1) copy only the needed data structure in arm_fp16.td (this is
> what was done in original review) 2) put all data structure in a newly
> created file and include it in arm_neon.td and arm_fp16.td (done here). 3)
> put only the duplication in a new file and include it. I did not go for this
> one given that we create a new file for the only purpose of avoiding a small
> duplication but I am fine of going with 3 too. Note that some of the
> duplicated structure in the original arm_fp16.td was a stripped down version
> of the copied one.
Given that the duplication is tiny, I don't have strong opinions to be honest.
Would be nice to share these definitions if that's easy to do, otherwise we can
perfectly live with this I think.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:4102
NEONMAP1(vuqadds_s32, aarch64_neon_suqadd, Add1ArgType),
+ // FP16 scalar intrinisics go here.
+ NEONMAP1(vabdh_f16, aarch64_sisd_fabd, Add1ArgType),
az wrote:
> SjoerdMeijer wrote:
> > Looks like a few intrinsic descriptions are missing here. For example, the
> > first 2-operand intrinsic vaddh_f16 is missing, but there are also more. Is
> > this intentional, or might they have slipped through the cracks (or am I
> > missing something)?
> I agree that this is confusing. For the intrinsics listed in this table, code
> generation happens in a generic way based on the info in the table. The ones
> not listed in this table are addressed in a more specific way below in a the
> function called EmitAArch64BuiltinExpr. While I do not like how few things
> were implemented in generating the intrinsics, I am in general following the
> approach taken for arm_neon instead of introducing a new approach.
Ah, I see, I somehow missed that. Fair enough.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:6511
"vgetq_lane");
+ case NEON::BI__builtin_neon_vaddh_f16:
+ Ops.push_back(EmitScalarExpr(E->getArg(1)));
nit: indentation seems off by 1 for these case statements.
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:6531
+ Value *F = CGM.getIntrinsic(Intrinsic::fma, HalfTy);
+Value *Zero = llvm::ConstantFP::getZeroValueForNegation(HalfTy);
+ Value* Sub = Builder.CreateFSub(Zero, EmitScalarExpr(E->getArg(1)),
"vsubh");
nit: indentation seems off by 1
Comment at: clang/utils/TableGen/NeonEmitter.cpp:2464
+" * Permission is hereby granted, free of charge, to any person "
+"obtaining "
+"a copy\n"
more nits: I see this is copied from above, but I think this and the next line
can be on the same line, just increasing readability a tiny bit.
Comment at: clang/utils/TableGen/NeonEmitter.cpp:2468
+"\"Software\"),"
+" to deal\n"
+" * in the Software without restriction, including without limitation "
same here
Comment at: clang/utils/TableGen/NeonEmitter.cpp:2470
+" * in the Software without restriction, including without limitation "
+"the "
+"rights\n"
and same here
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:250
def int_aarch64_neon_umax : AdvSIMD_2VectorArg_Intrinsic;
- def int_aarch64_neon_fmax : AdvSIMD_2VectorArg_Intrinsic;
+ def int_aarch64_neon_fmax : AdvSIMD_2FloatArg_Intrinsic;
def int_aarch64_neon_fmaxnmp : AdvSIMD_2VectorArg_Intrinsic;
There's a scalar and vector variant of FMAX and thus I am wondering if we don't
need two definitions here: one using AdvSIMD_2FloatArg_Intrinsic and the other
AdvSIMD_2VectorArg_Intrinsic?
Comment at: llvm/include/llvm/IR/IntrinsicsAArch64.td:262
def int_aarch64_neon_umin : AdvSIMD_2VectorArg_Intrinsic;
- def int_aarch64_neon_fmin : AdvSIMD_2VectorArg_Intrinsic;
+ def int_aarch64_neon_fmin : AdvSIMD_2FloatArg_Intrinsic;
def int_aarch64_neon_fminnmp : AdvSIMD_2VectorArg_Intrinsic;
Same here for FMIN
https://reviews.llvm.org/D41792
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)
baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, dcoughlin.
Herald added subscribers: a.sidorin, szepet.
This patch is a "light" version of https://reviews.llvm.org/D35109:
Since the range-based constraint manager (default) is weak in handling
comparisons where symbols are on both sides it is wise to rearrange them to
have symbols only on the left side. Thus e.g. `A + n >= B + m` becomes `A - B
>= m - n` which enables the constraint manager to store a range `m - n ..
MAX_VALUE` for the symbolic expression `A - B`. This can be used later to check
whether e.g. `A + k == B + l` can be true, which is also rearranged to `A - B
== l - k` so the constraint manager can check whether `l - k` is in the range
(thus greater than or equal to `m - n`).
The restriction in this version is the the rearrangement happens only if the
concrete integers are within the range `[min/4 .. max/4]` where `min` and `max`
are the minimal and maximal values of their type.
The rearrangement is not enabled by default. It has to be enabled by using
`-analyzer-config aggressive-relational-comparison-simplification=true`.
Co-author of this patch is Artem Dergachev (NoQ).
https://reviews.llvm.org/D41938
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
test/Analysis/svalbuilder-rearrange-comparisons.c
Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- /dev/null
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -0,0 +1,928 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_dump(int x);
+void clang_analyzer_eval(int x);
+void clang_analyzer_printState();
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+int g();
+int f() {
+ int x = g();
+ // Assert that no overflows occur in this test file.
+ // Assuming that concrete integers are also within that range.
+ if (x > ((int)INT_MAX / 4))
+exit(1);
+ if (x < -(((int)INT_MAX) / 4))
+exit(1);
+ return x;
+}
+
+void compare_different_symbol_equal() {
+ int x = f(), y = f();
+ clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+ clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+ clang_analyzer_dump(x == y);
+ // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 0}}
+}
+
+void compare_different_symbol_plus_left_int_equal() {
+ int x = f()+1, y = f();
+ clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+ clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+ clang_analyzer_dump(x == y);
+ // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_minus_left_int_equal() {
+ int x = f()-1, y = f();
+ clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+ clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+ clang_analyzer_dump(x == y);
+ // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 1}}
+}
+
+void compare_different_symbol_plus_right_int_equal() {
+ int x = f(), y = f()+2;
+ clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+ clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 2}}
+ clang_analyzer_dump(x == y);
+ // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 2}}
+}
+
+void compare_different_symbol_minus_right_int_equal() {
+ int x = f(), y = f()-2;
+ clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+ clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 2}}
+ clang_analyzer_dump(x == y);
+ // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 2}}
+}
+
+void compare_different_symbol_plus_left_plus_right_int_equal() {
+ int x = f()+2, y = f()+1;
+ clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+ clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+ clang_analyzer_dump(x == y);
+ // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_plus_left_minus_right_int_equal() {
+ int x = f()+2, y = f()-1;
+ clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+ clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+ clang_analyzer_dump(x == y);
+ // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_plus_right_int_equal() {
+ int x = f()-2, y = f()+1;
+ clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+ clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+ clang_analyzer_dump(x == y);
+ // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_minus_right_int_equal() {
+ int x = f()-2, y = f()-1;
+ clang_analyzer_dump(
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
baloghadamsoftware added a comment. In https://reviews.llvm.org/D35110#972430, @NoQ wrote: > In https://reviews.llvm.org/D35110#969782, @baloghadamsoftware wrote: > > > Strange, but modifying the tests from `m n` to `m - n > > 0` does not help. The statement `if (m - n 0)` does not store a > > range for `m - n` in the constraint manager. With the other patch which > > automatically changes `m n` to `m - n 0` the range is > > stored automatically. > > > I guess we can easily assume how a `SymIntExpr` relates to a number by > storing a range on the opaque left-hand-side symbol, no matter how > complicated it is, but we cannot assume how a symbol relates to a symbol > (there's no obvious range to store). That's just how `assumeSym` currently > works. Actually it happens because `m - n` evaluates to `Unknown`. The code part responsible for this is the beginning of `SValBuilder::makeSymExprValNN()`, which prevents `m - n`-like symbolic expression unless one of `m` or `n` is `Tainted`. Anna added this part 5-6 years ago because some kind of bug, but it seems that it still exists. If I try to remove it then one test executes for days (with loop max count 63 or 64) and two tests fail with an assert. https://reviews.llvm.org/D35110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38639: [clangd] #include statements support for Open definition
ilya-biryukov added inline comments.
Comment at: clangd/ClangdUnit.h:51
+using IncludeReferenceMap = std::unordered_map;
+
We use `unordered_map` as a `vector>` here. (i.e. we never look up
values by key, because we don't only the range, merely a point in the range)
Replace map with `vector>` and remove `RangeHash` that we don't need
anymore.
Comment at: clangd/XRefs.cpp:185
+
+ if ((unsigned)R.start.line ==
+ SourceMgr.getSpellingLineNumber(
malaperle wrote:
> ilya-biryukov wrote:
> > malaperle wrote:
> > > ilya-biryukov wrote:
> > > > why do we need to convert to unsigned? To slience compiler warnings?
> > > Yes, "line" from the protocol is signed, whereas
> > > getSpellingColumn/lineNumber returns unsigned. I'll extract another var
> > > for the line number and cast both to int instead to have less casts and
> > > make the condition smaller.
> > Can we maybe convert to `clangd::Position` using the helper methods first
> > and do the comparison of two `clangd::Position`s?
> > Comparing between `clangd::Position` and clang's line/column numbers is a
> > common source of off-by-one errors in clangd.
> offsetToPosition (if that's what you mean) uses a StringRef of the code,
> which is not handy in this case. I'll add another one "sourceLocToPosition"
> to convert a SourceLocation to a Position. WDYT? It can be used in a few
> other places too.
Looks good, thanks!
Comment at: unittests/clangd/ClangdTests.cpp:781
+
+ Position P = Position{1, 11};
+
We moved `findDefinition` tests to `XRefsTests.cpp` and added a useful
`Annotation` helper to make writing these tests simpler.
Maybe we could use it for this test as well?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38639
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit
ncw created this revision. ncw added reviewers: sunfish, dschuff. Herald added subscribers: cfe-commits, aheejin, jgravelle-google, sbc100, jfb. See: https://bugs.llvm.org/show_bug.cgi?id=35582 Since Wasm has to use 32-to-16 bit instructions to narrow values down, 32-bit should be a tad faster for operations needing 16 or more bits. Repository: rC Clang https://reviews.llvm.org/D41941 Files: lib/Basic/Targets/WebAssembly.h test/Preprocessor/init.c Index: test/Preprocessor/init.c === --- test/Preprocessor/init.c +++ test/Preprocessor/init.c @@ -9166,10 +9166,10 @@ // WEBASSEMBLY32-NEXT:#define __INTPTR_MAX__ 2147483647 // WEBASSEMBLY32-NEXT:#define __INTPTR_TYPE__ int // WEBASSEMBLY32-NEXT:#define __INTPTR_WIDTH__ 32 -// WEBASSEMBLY32-NEXT:#define __INT_FAST16_FMTd__ "hd" -// WEBASSEMBLY32-NEXT:#define __INT_FAST16_FMTi__ "hi" -// WEBASSEMBLY32-NEXT:#define __INT_FAST16_MAX__ 32767 -// WEBASSEMBLY32-NEXT:#define __INT_FAST16_TYPE__ short +// WEBASSEMBLY32-NEXT:#define __INT_FAST16_FMTd__ "d" +// WEBASSEMBLY32-NEXT:#define __INT_FAST16_FMTi__ "i" +// WEBASSEMBLY32-NEXT:#define __INT_FAST16_MAX__ 2147483647 +// WEBASSEMBLY32-NEXT:#define __INT_FAST16_TYPE__ int // WEBASSEMBLY32-NEXT:#define __INT_FAST32_FMTd__ "d" // WEBASSEMBLY32-NEXT:#define __INT_FAST32_FMTi__ "i" // WEBASSEMBLY32-NEXT:#define __INT_FAST32_MAX__ 2147483647 @@ -9182,10 +9182,10 @@ // WEBASSEMBLY32-NEXT:#define __INT_FAST8_FMTi__ "hhi" // WEBASSEMBLY32-NEXT:#define __INT_FAST8_MAX__ 127 // WEBASSEMBLY32-NEXT:#define __INT_FAST8_TYPE__ signed char -// WEBASSEMBLY32-NEXT:#define __INT_LEAST16_FMTd__ "hd" -// WEBASSEMBLY32-NEXT:#define __INT_LEAST16_FMTi__ "hi" -// WEBASSEMBLY32-NEXT:#define __INT_LEAST16_MAX__ 32767 -// WEBASSEMBLY32-NEXT:#define __INT_LEAST16_TYPE__ short +// WEBASSEMBLY32-NEXT:#define __INT_LEAST16_FMTd__ "d" +// WEBASSEMBLY32-NEXT:#define __INT_LEAST16_FMTi__ "i" +// WEBASSEMBLY32-NEXT:#define __INT_LEAST16_MAX__ 2147483647 +// WEBASSEMBLY32-NEXT:#define __INT_LEAST16_TYPE__ int // WEBASSEMBLY32-NEXT:#define __INT_LEAST32_FMTd__ "d" // WEBASSEMBLY32-NEXT:#define __INT_LEAST32_FMTi__ "i" // WEBASSEMBLY32-NEXT:#define __INT_LEAST32_MAX__ 2147483647 @@ -9312,12 +9312,12 @@ // WEBASSEMBLY32-NEXT:#define __UINTPTR_MAX__ 4294967295U // WEBASSEMBLY32-NEXT:#define __UINTPTR_TYPE__ unsigned int // WEBASSEMBLY32-NEXT:#define __UINTPTR_WIDTH__ 32 -// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_FMTX__ "hX" -// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_FMTo__ "ho" -// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_FMTu__ "hu" -// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_FMTx__ "hx" -// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_MAX__ 65535 -// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_TYPE__ unsigned short +// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_FMTX__ "X" +// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_FMTo__ "o" +// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_FMTu__ "u" +// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_FMTx__ "x" +// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_MAX__ 4294967295U +// WEBASSEMBLY32-NEXT:#define __UINT_FAST16_TYPE__ unsigned int // WEBASSEMBLY32-NEXT:#define __UINT_FAST32_FMTX__ "X" // WEBASSEMBLY32-NEXT:#define __UINT_FAST32_FMTo__ "o" // WEBASSEMBLY32-NEXT:#define __UINT_FAST32_FMTu__ "u" @@ -9336,12 +9336,12 @@ // WEBASSEMBLY32-NEXT:#define __UINT_FAST8_FMTx__ "hhx" // WEBASSEMBLY32-NEXT:#define __UINT_FAST8_MAX__ 255 // WEBASSEMBLY32-NEXT:#define __UINT_FAST8_TYPE__ unsigned char -// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_FMTX__ "hX" -// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_FMTo__ "ho" -// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_FMTu__ "hu" -// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_FMTx__ "hx" -// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_MAX__ 65535 -// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_TYPE__ unsigned short +// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_FMTX__ "X" +// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_FMTo__ "o" +// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_FMTu__ "u" +// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_FMTx__ "x" +// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_MAX__ 4294967295U +// WEBASSEMBLY32-NEXT:#define __UINT_LEAST16_TYPE__ unsigned int // WEBASSEMBLY32-NEXT:#define __UINT_LEAST32_FMTX__ "X" // WEBASSEMBLY32-NEXT:#define __UINT_LEAST32_FMTo__ "o" // WEBASSEMBLY32-NEXT:#define __UINT_LEAST32_FMTu__ "u" @@ -9498,10 +9498,10 @@ // WEBASSEMBLY64-NEXT:#define __INTPTR_MAX__ 9223372036854775807L // WEBASSEMBLY64-NEXT:#define __INTPTR_TYPE__ long int // WEBASSEMBLY64-NEXT:#define __INTPTR_WIDTH__ 64 -// WEBASSEMBLY64-NEXT:#define __INT_FAST16_FMTd__ "hd" -// WEBASSEMBLY64-NEXT:#define __INT_FAST16_FMTi__ "hi" -// WEBASSEMBLY64-NEXT:#define __INT_FAST16_MAX__ 32767 -// WEBASSEMBLY64-NEXT:#define __INT_FAST16_TYPE__ short +// WEBASSEMBLY64-NEXT:#define __INT_FAST16_FMTd__ "d" +// WEBASSEMBLY64-NEXT:#define __INT_FAST16_FMTi__ "i" +// WEBASSEMBLY64-NEXT:#define __INT_FAST16_MAX__ 2147
r322268 - [Lex] Use WritableMemoryBuffer in ScratchBuffer.cpp
Author: labath Date: Thu Jan 11 02:43:45 2018 New Revision: 322268 URL: http://llvm.org/viewvc/llvm-project?rev=322268&view=rev Log: [Lex] Use WritableMemoryBuffer in ScratchBuffer.cpp This avoids the need to const_cast the buffer contents to write to it. NFCI. Modified: cfe/trunk/lib/Lex/ScratchBuffer.cpp Modified: cfe/trunk/lib/Lex/ScratchBuffer.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ScratchBuffer.cpp?rev=322268&r1=322267&r2=322268&view=diff == --- cfe/trunk/lib/Lex/ScratchBuffer.cpp (original) +++ cfe/trunk/lib/Lex/ScratchBuffer.cpp Thu Jan 11 02:43:45 2018 @@ -74,11 +74,11 @@ void ScratchBuffer::AllocScratchBuffer(u // Get scratch buffer. Zero-initialize it so it can be dumped into a PCH file // deterministically. - std::unique_ptr OwnBuf = - llvm::MemoryBuffer::getNewMemBuffer(RequestLen, ""); - llvm::MemoryBuffer &Buf = *OwnBuf; + std::unique_ptr OwnBuf = + llvm::WritableMemoryBuffer::getNewMemBuffer(RequestLen, + ""); + CurBuffer = OwnBuf->getBufferStart(); FileID FID = SourceMgr.createFileID(std::move(OwnBuf)); BufferStartLoc = SourceMgr.getLocForStartOfFile(FID); - CurBuffer = const_cast(Buf.getBufferStart()); BytesUsed = 0; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41897: Fixing a crash in Sema.
jkorous-apple updated this revision to Diff 129284.
jkorous-apple added a comment.
Changes based on Aaron's feedback.
https://reviews.llvm.org/D41897
Files:
Sema/SemaDeclCXX.cpp
SemaCXX/base-class-ambiguity-check.cpp
Index: SemaCXX/base-class-ambiguity-check.cpp
===
--- /dev/null
+++ SemaCXX/base-class-ambiguity-check.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only %s
+
+template class Foo {
+ struct Base : T {};
+
+ // Up to commit 680b3a8619 (2018-01-10) this produced a crash in Sema.
+ struct Derived : Base, T {};
+};
Index: Sema/SemaDeclCXX.cpp
===
--- Sema/SemaDeclCXX.cpp
+++ Sema/SemaDeclCXX.cpp
@@ -2417,9 +2417,16 @@
// Attach the remaining base class specifiers to the derived class.
Class->setBases(Bases.data(), NumGoodBases);
+ // Check that the only base classes that are duplicate are virtual.
for (unsigned idx = 0; idx < NumGoodBases; ++idx) {
// Check whether this direct base is inaccessible due to ambiguity.
QualType BaseType = Bases[idx]->getType();
+
+// Skip all dependent types in templates being used as base specifiers.
+// Checks below assume that the base specifier is a CXXRecord.
+if (BaseType->isDependentType())
+ continue;
+
CanQualType CanonicalBase = Context.getCanonicalType(BaseType)
.getUnqualifiedType();
Index: SemaCXX/base-class-ambiguity-check.cpp
===
--- /dev/null
+++ SemaCXX/base-class-ambiguity-check.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fsyntax-only %s
+
+template class Foo {
+ struct Base : T {};
+
+ // Up to commit 680b3a8619 (2018-01-10) this produced a crash in Sema.
+ struct Derived : Base, T {};
+};
Index: Sema/SemaDeclCXX.cpp
===
--- Sema/SemaDeclCXX.cpp
+++ Sema/SemaDeclCXX.cpp
@@ -2417,9 +2417,16 @@
// Attach the remaining base class specifiers to the derived class.
Class->setBases(Bases.data(), NumGoodBases);
+ // Check that the only base classes that are duplicate are virtual.
for (unsigned idx = 0; idx < NumGoodBases; ++idx) {
// Check whether this direct base is inaccessible due to ambiguity.
QualType BaseType = Bases[idx]->getType();
+
+// Skip all dependent types in templates being used as base specifiers.
+// Checks below assume that the base specifier is a CXXRecord.
+if (BaseType->isDependentType())
+ continue;
+
CanQualType CanonicalBase = Context.getCanonicalType(BaseType)
.getUnqualifiedType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26
nsz added a comment. if clang wants to provide _Float128 then the f128 constant suffix (specified by TS18661-3) and __builtin_inff128, __builtin_nanf128, __builtin_nansf128, __builtin_huge_valf128 (gcc builtins required by math.h) need to be supported too. as this patch is committed clang is broken (cannot use glibc headers) on any target where long double is quad precision (aarch64, mips64, s390, sparc64, riscv64) even if glibc fixes the >=gcc-7 check to something that works for clang: either the _Float128 typedef conflicts with the definition by clang or the suffixes/builtins are missing. Repository: rC Clang https://reviews.llvm.org/D40673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26
nsz added a comment. also note that there is less than 3 weeks until glibc-2.27 is released, if the headers need a fix for clang then say so quickly i opened https://sourceware.org/bugzilla/show_bug.cgi?id=22700 but it needs attention from some clang developers, in particular there is no way to check if the compiler has _Float128 type defined but no working f128 suffix in the headers. Repository: rC Clang https://reviews.llvm.org/D40673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39903: [libclang] Allow pretty printing declarations
nik updated this revision to Diff 129426.
nik added a comment.
> Could one use an enum to get/set different properties of the policy?
>
> I've seen other C-API's (for Linear and Quadratic programming) follow a
> similar approach quite extensibly.
>
> It would significantly reduce the size of the API.
You are right and rethinking this, I see no problem with it. I thought of that
too (see my very first comment to this change), but then for some reason the
PrintingPolicy::Indentation (unsigned : 8) has driven me to this version. I
probably have only thought of strictly false/true values to set/get.
OK, adapted to enum version.
Repository:
rC Clang
https://reviews.llvm.org/D39903
Files:
include/clang-c/Index.h
test/Index/print-display-names.cpp
tools/c-index-test/c-index-test.c
tools/libclang/CIndex.cpp
tools/libclang/libclang.exports
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -178,6 +178,8 @@
clang_getCursorCompletionString
clang_getCursorDefinition
clang_getCursorDisplayName
+clang_getCursorPrintingPolicy
+clang_getCursorPrettyPrinted
clang_getCursorExtent
clang_getCursorExceptionSpecificationType
clang_getCursorKind
@@ -359,3 +361,6 @@
clang_EvalResult_getAsDouble
clang_EvalResult_getAsStr
clang_EvalResult_dispose
+clang_PrintingPolicy_get
+clang_PrintingPolicy_set
+clang_PrintingPolicy_dispose
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4706,6 +4706,191 @@
return cxstring::createSet(Manglings);
}
+CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) {
+ if (clang_Cursor_isNull(C))
+return 0;
+ return new PrintingPolicy(getCursorContext(C).getPrintingPolicy());
+}
+
+void clang_PrintingPolicy_dispose(CXPrintingPolicy Policy) {
+ if (Policy)
+delete static_cast(Policy);
+}
+
+unsigned clang_PrintingPolicy_get(CXPrintingPolicy Policy,
+ enum CXPrintingPolicyProperty Property) {
+ if (!Policy)
+return 0;
+
+ PrintingPolicy *P = static_cast(Policy);
+ switch (Property) {
+ case CXPrintingPolicy_Indentation:
+return P->Indentation;
+ case CXPrintingPolicy_SuppressSpecifiers:
+return P->SuppressSpecifiers;
+ case CXPrintingPolicy_SuppressTagKeyword:
+return P->SuppressTagKeyword;
+ case CXPrintingPolicy_IncludeTagDefinition:
+return P->IncludeTagDefinition;
+ case CXPrintingPolicy_SuppressScope:
+return P->SuppressScope;
+ case CXPrintingPolicy_SuppressUnwrittenScope:
+return P->SuppressUnwrittenScope;
+ case CXPrintingPolicy_SuppressInitializers:
+return P->SuppressInitializers;
+ case CXPrintingPolicy_ConstantArraySizeAsWritten:
+return P->ConstantArraySizeAsWritten;
+ case CXPrintingPolicy_AnonymousTagLocations:
+return P->AnonymousTagLocations;
+ case CXPrintingPolicy_SuppressStrongLifetime:
+return P->SuppressStrongLifetime;
+ case CXPrintingPolicy_SuppressLifetimeQualifiers:
+return P->SuppressLifetimeQualifiers;
+ case CXPrintingPolicy_SuppressTemplateArgsInCXXConstructors:
+return P->SuppressTemplateArgsInCXXConstructors;
+ case CXPrintingPolicy_Bool:
+return P->Bool;
+ case CXPrintingPolicy_Restrict:
+return P->Restrict;
+ case CXPrintingPolicy_Alignof:
+return P->Alignof;
+ case CXPrintingPolicy_UnderscoreAlignof:
+return P->UnderscoreAlignof;
+ case CXPrintingPolicy_UseVoidForZeroParams:
+return P->UseVoidForZeroParams;
+ case CXPrintingPolicy_TerseOutput:
+return P->TerseOutput;
+ case CXPrintingPolicy_PolishForDeclaration:
+return P->PolishForDeclaration;
+ case CXPrintingPolicy_Half:
+return P->Half;
+ case CXPrintingPolicy_MSWChar:
+return P->MSWChar;
+ case CXPrintingPolicy_IncludeNewlines:
+return P->IncludeNewlines;
+ case CXPrintingPolicy_MSVCFormatting:
+return P->MSVCFormatting;
+ case CXPrintingPolicy_ConstantsAsWritten:
+return P->ConstantsAsWritten;
+ case CXPrintingPolicy_SuppressImplicitBase:
+return P->SuppressImplicitBase;
+ case CXPrintingPolicy_FullyQualifiedName:
+return P->FullyQualifiedName;
+ }
+
+ return 0;
+}
+
+void clang_PrintingPolicy_set(CXPrintingPolicy Policy,
+ enum CXPrintingPolicyProperty Property,
+ unsigned Value) {
+ if (!Policy)
+return;
+
+ PrintingPolicy *P = static_cast(Policy);
+ switch (Property) {
+ case CXPrintingPolicy_Indentation:
+P->Indentation = Value;
+break;
+ case CXPrintingPolicy_SuppressSpecifiers:
+P->SuppressSpecifiers = Value;
+break;
+ case CXPrintingPolicy_SuppressTagKeyword:
+P->SuppressTagKeyword = Value;
+break;
+ case CXPrintingPolicy_IncludeTagDefinition:
+P->IncludeTagDefinition = Value;
+break;
+ case CXPrintingPolicy_SuppressScope:
+P->SuppressScope =
[PATCH] D39903: [libclang] Allow pretty printing declarations
nik updated this revision to Diff 129427.
nik added a comment.
Used macros as in a previous version to make it less verbose and error prone.
Repository:
rC Clang
https://reviews.llvm.org/D39903
Files:
include/clang-c/Index.h
test/Index/print-display-names.cpp
tools/c-index-test/c-index-test.c
tools/libclang/CIndex.cpp
tools/libclang/libclang.exports
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -178,6 +178,8 @@
clang_getCursorCompletionString
clang_getCursorDefinition
clang_getCursorDisplayName
+clang_getCursorPrintingPolicy
+clang_getCursorPrettyPrinted
clang_getCursorExtent
clang_getCursorExceptionSpecificationType
clang_getCursorKind
@@ -359,3 +361,6 @@
clang_EvalResult_getAsDouble
clang_EvalResult_getAsStr
clang_EvalResult_dispose
+clang_PrintingPolicy_get
+clang_PrintingPolicy_set
+clang_PrintingPolicy_dispose
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4706,6 +4706,125 @@
return cxstring::createSet(Manglings);
}
+CXPrintingPolicy clang_getCursorPrintingPolicy(CXCursor C) {
+ if (clang_Cursor_isNull(C))
+return 0;
+ return new PrintingPolicy(getCursorContext(C).getPrintingPolicy());
+}
+
+void clang_PrintingPolicy_dispose(CXPrintingPolicy Policy) {
+ if (Policy)
+delete static_cast(Policy);
+}
+
+#define HANDLE_CASE(P, PROPERTY_NAME) \
+ case CXPrintingPolicy_##PROPERTY_NAME: \
+return P->PROPERTY_NAME;
+
+unsigned clang_PrintingPolicy_get(CXPrintingPolicy Policy,
+ enum CXPrintingPolicyProperty Property) {
+ if (!Policy)
+return 0;
+
+ PrintingPolicy *P = static_cast(Policy);
+ switch (Property) {
+HANDLE_CASE(P, Indentation)
+HANDLE_CASE(P, SuppressSpecifiers)
+HANDLE_CASE(P, SuppressTagKeyword)
+HANDLE_CASE(P, IncludeTagDefinition)
+HANDLE_CASE(P, SuppressScope)
+HANDLE_CASE(P, SuppressUnwrittenScope)
+HANDLE_CASE(P, SuppressInitializers)
+HANDLE_CASE(P, ConstantArraySizeAsWritten)
+HANDLE_CASE(P, AnonymousTagLocations)
+HANDLE_CASE(P, SuppressStrongLifetime)
+HANDLE_CASE(P, SuppressLifetimeQualifiers)
+HANDLE_CASE(P, SuppressTemplateArgsInCXXConstructors)
+HANDLE_CASE(P, Bool)
+HANDLE_CASE(P, Restrict)
+HANDLE_CASE(P, Alignof)
+HANDLE_CASE(P, UnderscoreAlignof)
+HANDLE_CASE(P, UseVoidForZeroParams)
+HANDLE_CASE(P, TerseOutput)
+HANDLE_CASE(P, PolishForDeclaration)
+HANDLE_CASE(P, Half)
+HANDLE_CASE(P, MSWChar)
+HANDLE_CASE(P, IncludeNewlines)
+HANDLE_CASE(P, MSVCFormatting)
+HANDLE_CASE(P, ConstantsAsWritten)
+HANDLE_CASE(P, SuppressImplicitBase)
+HANDLE_CASE(P, FullyQualifiedName)
+ }
+
+ return 0;
+}
+
+#undef HANDLE_CASE
+#define HANDLE_CASE(P, PROPERTY_NAME, VALUE) \
+ case CXPrintingPolicy_##PROPERTY_NAME: \
+P->PROPERTY_NAME = VALUE; \
+break;
+
+void clang_PrintingPolicy_set(CXPrintingPolicy Policy,
+ enum CXPrintingPolicyProperty Property,
+ unsigned Value) {
+ if (!Policy)
+return;
+
+ PrintingPolicy *P = static_cast(Policy);
+ switch (Property) {
+HANDLE_CASE(P, Indentation, Value)
+HANDLE_CASE(P, SuppressSpecifiers, Value)
+HANDLE_CASE(P, SuppressTagKeyword, Value)
+HANDLE_CASE(P, IncludeTagDefinition, Value)
+HANDLE_CASE(P, SuppressScope, Value)
+HANDLE_CASE(P, SuppressUnwrittenScope, Value)
+HANDLE_CASE(P, SuppressInitializers, Value)
+HANDLE_CASE(P, ConstantArraySizeAsWritten, Value)
+HANDLE_CASE(P, AnonymousTagLocations, Value)
+HANDLE_CASE(P, SuppressStrongLifetime, Value)
+HANDLE_CASE(P, SuppressLifetimeQualifiers, Value)
+HANDLE_CASE(P, SuppressTemplateArgsInCXXConstructors, Value)
+HANDLE_CASE(P, Bool, Value)
+HANDLE_CASE(P, Restrict, Value)
+HANDLE_CASE(P, Alignof, Value)
+HANDLE_CASE(P, UnderscoreAlignof, Value)
+HANDLE_CASE(P, UseVoidForZeroParams, Value)
+HANDLE_CASE(P, TerseOutput, Value)
+HANDLE_CASE(P, PolishForDeclaration, Value)
+HANDLE_CASE(P, Half, Value)
+HANDLE_CASE(P, MSWChar, Value)
+HANDLE_CASE(P, IncludeNewlines, Value)
+HANDLE_CASE(P, MSVCFormatting, Value)
+HANDLE_CASE(P, ConstantsAsWritten, Value)
+HANDLE_CASE(P, SuppressImplicitBase, Value)
+HANDLE_CASE(P, FullyQualifiedName, Value)
+ }
+}
+
+#undef HANDLE_CASE
+
+CXString clang_getCursorPrettyPrinted(CXCursor C, CXPrintingPolicy cxPolicy) {
+ if (clang_Cursor_isNull(C))
+return cxstring::createEmpty();
+
+ if (clang_isDeclaration(C.kind)) {
+const Decl *D = getCursorDecl(C);
+if
[PATCH] D41815: [clang-tidy] implement check for goto
JonasToth added inline comments.
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:22
+ if (getLangOpts().CPlusPlus)
+Finder->addMatcher(gotoStmt().bind("goto"), this);
+}
aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > aaron.ballman wrote:
> > > > Are you planning to add the exception listed in the C++ Core Guideline?
> > > > It makes an explicit exception allowing you to jump forward out of a
> > > > loop construct.
> > > What should this check do with indirect goto statements (it's a GCC
> > > extension we support where you can jump to an expression)?
> > >
> > > Regardless, there should be a test for indirect gotos and jump forward
> > > out of loop constructs.
> > > Are you planning to add the exception listed in the C++ Core Guideline?
> > > It makes an explicit exception allowing you to jump forward out of a loop
> > > construct.
> >
> > I do plan for this. Because I dont have any experience with gotos I wanted
> > to do it in small steps.
> >
> > > What should this check do with indirect goto statements (it's a GCC
> > > extension we support where you can jump to an expression)?
> > Iam not aware of these :)
> >> What should this check do with indirect goto statements (it's a GCC
> >> extension we support where you can jump to an expression)?
> >
> > Iam not aware of these :)
>
> https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
> (and a good reference on why these are interesting:
> https://eli.thegreenplace.net/2012/07/12/computed-goto-for-efficient-dispatch-tables)
I would think that this is a special feature that will only be used if you know
what you are doing. So it should be allowed with configuration. I am not sure
about the default though. For now it is ignored.
HICPP has a rule on gotos as well, which states that only forward jumps are
allowed.
I think that these different approaches to `goto` should land here sometime as
different configurations.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41815: [clang-tidy] implement check for goto
JonasToth updated this revision to Diff 129433.
JonasToth added a comment.
I enhanced the check to do more:
- check that jumps will only be forward. AFAIK that is required in all
sensefull usecases of goto, is it?
- additionally check for gotos in nested loops. These are not diagnosed if the
jump is forward implementing the exception in the core guidelines.
With these modifications the check can be used to enforce the rule in the
CoreGuidelines and the goto part of `6.3.1 Ensure that the label(s) for a jump
statement or a switch condition appear later, in the same or an enclosing block`
for the HICPP module.
Some test cases for all combinations are missing, i can add those once you
agree that the functionality change is indeed ok.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41815
Files:
clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp
clang-tidy/cppcoreguidelines/AvoidGotoCheck.h
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
Index: test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-avoid-goto.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-goto %t
+
+void noop() {}
+
+int main() {
+ noop();
+ goto jump_to_me;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+ noop();
+
+jump_to_me:;
+
+jump_backwards:;
+ noop();
+ goto jump_backwards;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+ // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not jump backwards with 'goto'
+ // CHECK-MESSAGES: [[@LINE-5]]:1: note: label defined here
+
+ // Test the GNU extension https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
+some_label:;
+ void *dynamic_label = &&some_label;
+
+ // FIXME: Not detected
+ goto *dynamic_label;
+ // CHECK MESSAGES: [[@LINE-1]]:3: warning: avoid using 'goto' for flow control
+}
+
+void forward_jump_out_nested_loop() {
+ for (int i = 0; i < 10; ++i) {
+noop();
+for (int j = 0; j < 10; ++j) {
+ noop();
+ if (i + j > 10)
+goto early_exit1;
+}
+noop();
+ }
+
+ for (int i = 0; i < 10; ++i) {
+noop();
+while (true) {
+ noop();
+ if (i > 5)
+goto early_exit1;
+}
+noop();
+ }
+ // Jumping further results in error, because the variable declaration would
+ // be skipped.
+early_exit1:;
+
+ int i = 0;
+ while (true) {
+noop();
+while (true) {
+ noop();
+ if (i > 5)
+goto early_exit2;
+ i++;
+}
+noop();
+ }
+
+ while (true) {
+noop();
+for (int j = 0; j < 10; ++j) {
+ noop();
+ if (j > 5)
+goto early_exit2;
+}
+noop();
+ }
+
+early_exit2:;
+}
+
+void jump_out_backwards() {
+
+before_the_loop:
+ noop();
+
+ for (int i = 0; i < 10; ++i) {
+for (int j = 0; j < 10; ++j) {
+ if (i * j > 80)
+goto before_the_loop;
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: do not jump backwards with 'goto'
+ // CHECK-MESSAGES: [[@LINE-8]]:1: note: label defined here
+}
+ }
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -52,6 +52,7 @@
cert-msc30-c (redirects to cert-msc50-cpp)
cert-msc50-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init)
+ cppcoreguidelines-avoid-goto
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator)
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-no-malloc
Index: docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-avoid-goto.rst
@@ -0,0 +1,11 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-goto
+
+cppcoreguidelines-avoid-goto
+
+
+The usage of ``goto`` has been discouraged for a long time and is diagnosed
+with this check.
+
+This check implements `ES.76 `_
+from the CppCoreGuidelines. For more information on why to avoid programming
+with ``goto`` you can read the famous paper `A Case against the GO TO Statement. `_.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,12 @@
Improvements to clang-tidy
--
+- New `cppcoreguidelines-avoid-goto
+
[clang-tools-extra] r322274 - [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces
Author: alexfh
Date: Thu Jan 11 05:00:28 2018
New Revision: 322274
URL: http://llvm.org/viewvc/llvm-project?rev=322274&view=rev
Log:
[clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested
namespaces
Summary:
Fixes bug 34701
When we encounter a namespace find the location of the left bracket.
Then if the text between the name and the left bracket contains a ':'
then it's a C++17 nested namespace.
Reviewers: #clang-tools-extra, alexfh, aaron.ballman
Reviewed By: aaron.ballman
Subscribers: curdeius, cfe-commits, krasimir, JonasToth, JDevlieghere, xazax.hun
Tags: #clang-tools-extra
Patch by Alexandru Octavian Buțiu!
Differential Revision: https://reviews.llvm.org/D38284
Added:
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h
Modified:
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=322274&r1=322273&r2=322274&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
Thu Jan 11 05:00:28 2018
@@ -23,7 +23,7 @@ NamespaceCommentCheck::NamespaceCommentC
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
- "namespace( +([a-zA-Z0-9_]+))?\\.? *(\\*/)?$",
+ "namespace( +([a-zA-Z0-9_:]+))?\\.? *(\\*/)?$",
llvm::Regex::IgnoreCase),
ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {}
@@ -56,6 +56,15 @@ static std::string getNamespaceComment(c
return Fix;
}
+static std::string getNamespaceComment(const std::string &NameSpaceName,
+ bool InsertLineBreak) {
+ std::string Fix = "// namespace ";
+ Fix.append(NameSpaceName);
+ if (InsertLineBreak)
+Fix.append("\n");
+ return Fix;
+}
+
void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
const auto *ND = Result.Nodes.getNodeAs("namespace");
const SourceManager &Sources = *Result.SourceManager;
@@ -74,11 +83,44 @@ void NamespaceCommentCheck::check(const
SourceLocation AfterRBrace = ND->getRBraceLoc().getLocWithOffset(1);
SourceLocation Loc = AfterRBrace;
Token Tok;
+ SourceLocation LBracketLocation = ND->getLocation();
+ SourceLocation NestedNamespaceBegin = LBracketLocation;
+
+ // Currently for nested namepsace (n1::n2::...) the AST matcher will match
foo
+ // then bar instead of a single match. So if we got a nested namespace we
have
+ // to skip the next ones.
+ for (const auto &EndOfNameLocation : Ends) {
+if (Sources.isBeforeInTranslationUnit(NestedNamespaceBegin,
+ EndOfNameLocation))
+ return;
+ }
+
+ // Ignore macros
+ if (!ND->getLocation().isMacroID()) {
+while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) ||
+ !Tok.is(tok::l_brace)) {
+ LBracketLocation = LBracketLocation.getLocWithOffset(1);
+}
+ }
+
+ auto TextRange =
+ Lexer::getAsCharRange(SourceRange(NestedNamespaceBegin,
LBracketLocation),
+Sources, getLangOpts());
+ StringRef NestedNamespaceName =
+ Lexer::getSourceText(TextRange, Sources, getLangOpts()).rtrim();
+ bool IsNested = NestedNamespaceName.contains(':');
+
+ if (IsNested)
+Ends.push_back(LBracketLocation);
+ else
+NestedNamespaceName = ND->getName();
+
// Skip whitespace until we find the next token.
while (Lexer::getRawToken(Loc, Tok, Sources, getLangOpts()) ||
Tok.is(tok::semi)) {
Loc = Loc.getLocWithOffset(1);
}
+
if (!locationsInSameFile(Sources, ND->getRBraceLoc(), Loc))
return;
@@ -98,10 +140,14 @@ void NamespaceCommentCheck::check(const
StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] : "";
StringRef Anonymous = Groups.size() > 3 ? Groups[3] : "";
- // Check if the namespace in the comment is the same.
- if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) ||
- (ND->getNameAsString() == NamespaceNameInComment &&
- Anonymous.empty())) {
+ if (IsNested && NestedNamespaceName == NamespaceNameInComment) {
+// C++17 nested namespace.
+return;
+ } else if ((ND->isAnonymousNamespace() &&
+ NamespaceNameInComment.empty()) ||
+ (ND->getNameAsString() == Names
[PATCH] D38284: [clang-tidy] Fix google-readability-namespace-comments handling of C++17 nested namespaces
This revision was automatically updated to reflect the committed changes.
Closed by commit rL322274: [clang-tidy] Fix
google-readability-namespace-comments handling of C++17 nested… (authored by
alexfh, committed by ).
Herald added subscribers: llvm-commits, klimek.
Changed prior to commit:
https://reviews.llvm.org/D38284?vs=121152&id=129434#toc
Repository:
rL LLVM
https://reviews.llvm.org/D38284
Files:
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h
clang-tools-extra/trunk/test/clang-tidy/google-readability-nested-namespace-comments.cpp
Index: clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.h
@@ -34,6 +34,7 @@
llvm::Regex NamespaceCommentPattern;
const unsigned ShortNamespaceLines;
const unsigned SpacesBeforeComments;
+ llvm::SmallVector Ends;
};
} // namespace readability
Index: clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
@@ -23,7 +23,7 @@
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
NamespaceCommentPattern("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
- "namespace( +([a-zA-Z0-9_]+))?\\.? *(\\*/)?$",
+ "namespace( +([a-zA-Z0-9_:]+))?\\.? *(\\*/)?$",
llvm::Regex::IgnoreCase),
ShortNamespaceLines(Options.get("ShortNamespaceLines", 1u)),
SpacesBeforeComments(Options.get("SpacesBeforeComments", 1u)) {}
@@ -56,6 +56,15 @@
return Fix;
}
+static std::string getNamespaceComment(const std::string &NameSpaceName,
+ bool InsertLineBreak) {
+ std::string Fix = "// namespace ";
+ Fix.append(NameSpaceName);
+ if (InsertLineBreak)
+Fix.append("\n");
+ return Fix;
+}
+
void NamespaceCommentCheck::check(const MatchFinder::MatchResult &Result) {
const auto *ND = Result.Nodes.getNodeAs("namespace");
const SourceManager &Sources = *Result.SourceManager;
@@ -74,11 +83,44 @@
SourceLocation AfterRBrace = ND->getRBraceLoc().getLocWithOffset(1);
SourceLocation Loc = AfterRBrace;
Token Tok;
+ SourceLocation LBracketLocation = ND->getLocation();
+ SourceLocation NestedNamespaceBegin = LBracketLocation;
+
+ // Currently for nested namepsace (n1::n2::...) the AST matcher will match foo
+ // then bar instead of a single match. So if we got a nested namespace we have
+ // to skip the next ones.
+ for (const auto &EndOfNameLocation : Ends) {
+if (Sources.isBeforeInTranslationUnit(NestedNamespaceBegin,
+ EndOfNameLocation))
+ return;
+ }
+
+ // Ignore macros
+ if (!ND->getLocation().isMacroID()) {
+while (Lexer::getRawToken(LBracketLocation, Tok, Sources, getLangOpts()) ||
+ !Tok.is(tok::l_brace)) {
+ LBracketLocation = LBracketLocation.getLocWithOffset(1);
+}
+ }
+
+ auto TextRange =
+ Lexer::getAsCharRange(SourceRange(NestedNamespaceBegin, LBracketLocation),
+Sources, getLangOpts());
+ StringRef NestedNamespaceName =
+ Lexer::getSourceText(TextRange, Sources, getLangOpts()).rtrim();
+ bool IsNested = NestedNamespaceName.contains(':');
+
+ if (IsNested)
+Ends.push_back(LBracketLocation);
+ else
+NestedNamespaceName = ND->getName();
+
// Skip whitespace until we find the next token.
while (Lexer::getRawToken(Loc, Tok, Sources, getLangOpts()) ||
Tok.is(tok::semi)) {
Loc = Loc.getLocWithOffset(1);
}
+
if (!locationsInSameFile(Sources, ND->getRBraceLoc(), Loc))
return;
@@ -98,10 +140,14 @@
StringRef NamespaceNameInComment = Groups.size() > 5 ? Groups[5] : "";
StringRef Anonymous = Groups.size() > 3 ? Groups[3] : "";
- // Check if the namespace in the comment is the same.
- if ((ND->isAnonymousNamespace() && NamespaceNameInComment.empty()) ||
- (ND->getNameAsString() == NamespaceNameInComment &&
- Anonymous.empty())) {
+ if (IsNested && NestedNamespaceName == NamespaceNameInComment) {
+// C++17 nested namespace.
+return;
+ } else if ((ND->isAnonymousNamespace() &&
+ NamespaceNameInComment.empty()) ||
+ (ND->getNameAsString() == NamespaceNameInComment &&
+ Anonymous.empty())) {
+// Check if the namespace in the comment is the same.
// FIXME: Maybe we need a strict mode, where we always fix name
[PATCH] D41815: [clang-tidy] implement check for goto
lebedev.ri added a comment. In https://reviews.llvm.org/D41815#973260, @JonasToth wrote: > - check that jumps will only be forward. AFAIK that is required in all > sensefull usecases of goto, is it? You could implement loops/recursion with goto, something like: const int end = 10; int i = 0; assert(i < end); begin: i++ if(i < end) goto begin; // end But it really should be done with normal `for()`, or `while()`, so i think it would make sense to diagnose those. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41800: [analyzer] Use a custom program point for the check::NewAllocator callback.
a.sidorin accepted this revision. a.sidorin added inline comments. Comment at: test/Analysis/NewDelete-path-notes.cpp:44 // CHECK-NEXT: -// CHECK-NEXT:line6 +// CHECK-NEXT:line7 // CHECK-NEXT:col3 Not even a minor concern for this patch, but I think that placing `//RUN` and `//CHECK` after the code being tested could save us from massive changes of line numbers. https://reviews.llvm.org/D41800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
NoQ added a comment.
Had a fresh look on the C++ part, it is super clean now, i'm very impressed :)
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:373-374
- return RuntimeDefinition();
+ auto Engine = static_cast(
+ getState()->getStateManager().getOwningEngine());
+
*Humbly suggests to refactor whatever we need here into `SubEngine`'s virtual
method(s).
`getAnalysisManager()` is already there, so i guess we only need to expose
`getCrossTranslationUnitContext()`.
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:385
+ FD, Engine->getAnalysisManager().options.getCTUDir(),
+ "externalFnMap.txt");
+
I think `CallEvent` is the last place where i'd prefer to hardcode this
filename. Maybe hardcode it in `CrossTranslationUnitContext` or get from
`AnalyzerOptions`? (the former, i guess, because i doubt anybody would ever
want to change it).
Comment at: lib/StaticAnalyzer/Core/PathDiagnostic.cpp:418-423
SourceLocation XDL = XD->getLocation();
SourceLocation YDL = YD->getLocation();
if (XDL != YDL) {
const SourceManager &SM = XL.getManager();
- return SM.isBeforeInTranslationUnit(XDL, YDL);
+ return compareCrossTUSourceLocs(FullSourceLoc(XDL, SM),
+ FullSourceLoc(YDL, SM));
It seems to me that `XDL` and `YDL` are exactly the same as `XL` and `YL` we've
seen at the beginning of the function.
...we still have only one `SourceManager`, right?
https://reviews.llvm.org/D30691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39963: [RISCV] Add initial RISC-V target and driver support
This revision was automatically updated to reflect the committed changes.
Closed by commit rC322276: [RISCV] Add the RISCV target and compiler driver
(authored by asb, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D39963?vs=126771&id=129436#toc
Repository:
rC Clang
https://reviews.llvm.org/D39963
Files:
lib/Basic/CMakeLists.txt
lib/Basic/Targets.cpp
lib/Basic/Targets/RISCV.cpp
lib/Basic/Targets/RISCV.h
lib/Driver/CMakeLists.txt
lib/Driver/ToolChains/Arch/RISCV.cpp
lib/Driver/ToolChains/Arch/RISCV.h
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/Clang.h
lib/Driver/ToolChains/Gnu.cpp
lib/Driver/ToolChains/Linux.cpp
test/Driver/Inputs/multilib_riscv_linux_sdk/bin/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/include/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/ld
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32d/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64d/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32d/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64/.keep
test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64d/.keep
test/Driver/frame-pointer.c
test/Driver/riscv-abi.c
test/Driver/riscv-features.c
test/Driver/riscv32-toolchain.c
test/Driver/riscv64-toolchain.c
test/Preprocessor/init.c
Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -10003,3 +10003,398 @@
// ARM-DARWIN-BAREMETAL-64: #define __PTRDIFF_TYPE__ long int
// ARM-DARWIN-BAREMETAL-64: #define __SIZE_TYPE__ long unsigned int
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=riscv32 < /dev/null \
+// RUN: | FileCheck -match-full-lines -check-prefix=RISCV32 %s
+// RISCV32: #define _ILP32 1
+// RISCV32: #define __ATOMIC_ACQUIRE 2
+// RISCV32: #define __ATOMIC_ACQ_REL 4
+// RISCV32: #define __ATOMIC_CONSUME 1
+// RISCV32: #define __ATOMIC_RELAXED 0
+// RISCV32: #define __ATOMIC_RELEASE 3
+// RISCV32: #define __ATOMIC_SEQ_CST 5
+// RISCV32: #define __BIGGEST_ALIGNMENT__ 16
+// RISCV32: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+// RISCV32: #define __CHAR16_TYPE__ unsigned short
+// RISCV32: #define __CHAR32_TYPE__ unsigned int
+// RISCV32: #define __CHAR_BIT__ 8
+// RISCV32: #define __DBL_DECIMAL_DIG__ 17
+// RISCV32: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324
+// RISCV32: #define __DBL_DIG__ 15
+// RISCV32: #define __DBL_EPSILON__ 2.2204460492503131e-16
+// RISCV32: #define __DBL_HAS_DENORM__ 1
+// RISCV32: #define __DBL_HAS_INFINITY__ 1
+// RISCV32: #define __DBL_HAS_QUIET_NAN__ 1
+// RISCV32: #define __DBL_MANT_DIG__ 53
+// RISCV32: #define __DBL_MAX_10_EXP__ 308
+// RISCV32: #define __DBL_MAX_EXP__ 1024
+// RISCV32: #define __DBL_MAX__ 1.7976931348623157e+308
+// RISCV32: #define __DBL_MIN_10_EXP__ (-307)
+// RISCV32: #define __DBL_MIN_EXP__ (-1021)
+// RISCV32: #define __DBL_MIN__ 2.2250738585072014e-308
+// RISCV32: #define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__
+// RISCV32: #define __ELF__ 1
+// RISCV32: #define __FINITE_MATH_ONLY__ 0
+// RISCV32: #define __FLT_DECIMAL_DIG__ 9
+// RISCV32: #define __FLT_DENORM_MIN__ 1.40129846e-45F
+// RISCV32: #define __FLT_DIG__ 6
+// RISCV32: #define __FLT_EPSILON__ 1.19209290e-7F
+// RISCV32: #define __FLT_EVAL_METHOD__ 0
+// RISCV32: #define __FLT_HAS_DENORM__ 1
+// RISCV32: #define __FLT_HAS_INFINITY__ 1
+// RISCV32: #define __FLT_HAS_QUIET_NAN__ 1
+// RISCV32: #define __FLT_MANT_DIG__ 24
+// RISCV32: #define __FLT_MAX_10_EXP__ 38
+// RISCV32: #define __FLT_MAX_EXP__ 128
+// RISCV32: #define __FLT_MAX__ 3.40282347e+38F
+// RISCV32: #define __FLT_MIN_10_EXP__ (-37)
+// RISCV32: #define __FLT_MIN_EXP__ (-125)
+// RISCV32: #define __FLT_MIN__ 1.17549435e-38F
+// RISCV32: #define __FLT_RADIX__ 2
+// RISCV32: #define __GCC_ATOMIC_BOOL_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_CHAR_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_INT_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_LLONG_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_LONG_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_POINTER_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_SHORT_LOCK_FREE 1
+// RISCV32: #define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
+// RISCV32: #define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 1
+// RISCV32: #define __GNUC_MINOR__ {{.*}}
+// RISCV32: #define __GNUC_PATCHLEVEL__ {{.*}}
+// RISCV32: #define __GNUC_STDC_INLINE__ 1
+// RISCV32: #define __GNUC__ {{.*}}
+// RISCV32: #define __GXX_ABI_VERSION {{.*}}
+// R
[PATCH] D41946: [clangd] Add support for different file URI schemas.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, ilya-biryukov, mgorny, klimek.
DO NOT SUBMIT: We should replace the existing URI struct in Protocol.h
with the new URI and rename FileURI to URI. But before doing that, I'd like to
early feedback on names and APIs.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41946
Files:
clangd/CMakeLists.txt
clangd/URI.cpp
clangd/URI.h
unittests/clangd/CMakeLists.txt
unittests/clangd/URITests.cpp
Index: unittests/clangd/URITests.cpp
===
--- /dev/null
+++ unittests/clangd/URITests.cpp
@@ -0,0 +1,75 @@
+//===-- URITests.cpp -*- C++ -*---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "URI.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(URITest, URIStrings) {
+ EXPECT_EQ(FileURI::fromURI("abc:///x/y/z").toString(), "abc:///x/y/z");
+ EXPECT_EQ(FileURI::fromURI("abc:///x/y/z"), FileURI("abc", "/x/y/z"));
+ EXPECT_EQ(FileURI::fromURI("abc://x/y/z"), FileURI("abc", "x/y/z"));
+
+ EXPECT_TRUE(FileURI::fromURI("abc:///x/y/z").IsValid());
+ EXPECT_FALSE(FileURI::fromURI("://x/y/z").IsValid());
+ EXPECT_FALSE(FileURI::fromURI("/x/y/z").IsValid());
+}
+
+TEST(URITest, FileSystemSchema) {
+ FileURI Uri{"file", "/a/b/c"};
+ FileSystemSchema S;
+ EXPECT_EQ(S.getAbsolutePath(/*CurrentFile*/ "", Uri), "/a/b/c");
+ EXPECT_EQ(S.uriFromAbsolutePath("/a/b/c"), Uri);
+}
+
+// Assume all files in the schema have a "test-root/" root directory, and the
+// schema path is the relative path to the root directory.
+// So the schema of "/some-dir/test-root/x/y/z" is "test://x/y/z".
+class TestSchema : public URISchema {
+public:
+ static const char *Schema;
+
+ static const char *TestRoot;
+
+ std::string getAbsolutePath(llvm::StringRef CurrentFile,
+ const FileURI &Uri) const override {
+auto Pos = CurrentFile.find(TestRoot);
+assert(Pos != llvm::StringRef::npos);
+return CurrentFile.substr(0, Pos + llvm::StringRef(TestRoot).size()).str() +
+ Uri.Path;
+ }
+
+ FileURI uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override {
+auto Pos = AbsolutePath.find(TestRoot);
+assert(Pos != llvm::StringRef::npos);
+return {Schema,
+AbsolutePath.substr(Pos + Pos + llvm::StringRef(TestRoot).size())};
+ }
+};
+
+const char *TestSchema::Schema = "test";
+const char *TestSchema::TestRoot = "/test-root/";
+
+static URISchemaRegistry::Add X(TestSchema::Schema, "Test schema");
+
+TEST(URITest, ResolveURIs) {
+ EXPECT_EQ(resolve("", FileURI("file", "/a/b/c")), "/a/b/c");
+ EXPECT_EQ(resolve("/dir/test-root/x/y/z", FileURI("test", "a/b/c")),
+"/dir/test-root/a/b/c");
+ // Unsupported schema.
+ EXPECT_EQ(resolve("/x/y/z", FileURI("invalid", "a/b/c")), "");
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -18,6 +18,7 @@
FuzzyMatchTests.cpp
IndexTests.cpp
JSONExprTests.cpp
+ URITests.cpp
TestFS.cpp
TraceTests.cpp
SourceCodeTests.cpp
Index: clangd/URI.h
===
--- /dev/null
+++ clangd/URI.h
@@ -0,0 +1,95 @@
+//===--- URI.h - File URIs with schemas --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_PATHURI_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PATHURI_H
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Registry.h"
+
+namespace clang {
+namespace clangd {
+
+/// \brief An URI for a file path in a certain schema. This is invalid if either
+/// schema or path is empty.
+struct FileURI {
+ std::string Schema;
+ // This is interpreted according to the schema.
+ std::string Path;
+
+ FileURI() = default;
+
+ FileURI(std::string Schema, std::string Path)
+ : Schema(std::move(Schema)), Path(std::move(Path)) {}
+
+ bool IsValid() const { return !Schema.empty() && !Path.empty(); }
+
+ /// \brief Returns a URI string "://".
+ std::string toString() const;
+
+ /// \brief Parse a URI string "://". If this is an invalid URI,
+ /// this returns an empty URI with empty schema and path.
+ static FileURI fromURI(llvm::StringRef Uri);
+
+ frien
[PATCH] D41947: Provide default virtual filesystem argument in ClangTool constructor
vladimir.plyashkun created this revision.
vladimir.plyashkun added reviewers: alexfh, ilya-biryukov.
vladimir.plyashkun added a project: clang.
Herald added a subscriber: klimek.
This revision is part of another review.
Check this link for more details - https://reviews.llvm.org/D41535
Repository:
rC Clang
https://reviews.llvm.org/D41947
Files:
include/clang/Tooling/Tooling.h
lib/Tooling/Tooling.cpp
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -328,10 +328,11 @@
ClangTool::ClangTool(const CompilationDatabase &Compilations,
ArrayRef SourcePaths,
- std::shared_ptr PCHContainerOps)
+ std::shared_ptr PCHContainerOps,
+ IntrusiveRefCntPtr BaseFS)
: Compilations(Compilations), SourcePaths(SourcePaths),
PCHContainerOps(std::move(PCHContainerOps)),
- OverlayFileSystem(new vfs::OverlayFileSystem(vfs::getRealFileSystem())),
+ OverlayFileSystem(new vfs::OverlayFileSystem(BaseFS)),
InMemoryFileSystem(new vfs::InMemoryFileSystem),
Files(new FileManager(FileSystemOptions(), OverlayFileSystem)),
DiagConsumer(nullptr) {
Index: include/clang/Tooling/Tooling.h
===
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -296,10 +296,13 @@
///not found in Compilations, it is skipped.
/// \param PCHContainerOps The PCHContainerOperations for loading and
creating
/// clang modules.
+ /// \param BaseFS Base virtual filesystem used for OverlayFileSystem creation
ClangTool(const CompilationDatabase &Compilations,
ArrayRef SourcePaths,
std::shared_ptr PCHContainerOps =
-std::make_shared());
+std::make_shared(),
+IntrusiveRefCntPtr BaseFS =
+vfs::getRealFileSystem());
~ClangTool();
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -328,10 +328,11 @@
ClangTool::ClangTool(const CompilationDatabase &Compilations,
ArrayRef SourcePaths,
- std::shared_ptr PCHContainerOps)
+ std::shared_ptr PCHContainerOps,
+ IntrusiveRefCntPtr BaseFS)
: Compilations(Compilations), SourcePaths(SourcePaths),
PCHContainerOps(std::move(PCHContainerOps)),
- OverlayFileSystem(new vfs::OverlayFileSystem(vfs::getRealFileSystem())),
+ OverlayFileSystem(new vfs::OverlayFileSystem(BaseFS)),
InMemoryFileSystem(new vfs::InMemoryFileSystem),
Files(new FileManager(FileSystemOptions(), OverlayFileSystem)),
DiagConsumer(nullptr) {
Index: include/clang/Tooling/Tooling.h
===
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -296,10 +296,13 @@
///not found in Compilations, it is skipped.
/// \param PCHContainerOps The PCHContainerOperations for loading and creating
/// clang modules.
+ /// \param BaseFS Base virtual filesystem used for OverlayFileSystem creation
ClangTool(const CompilationDatabase &Compilations,
ArrayRef SourcePaths,
std::shared_ptr PCHContainerOps =
-std::make_shared());
+std::make_shared(),
+IntrusiveRefCntPtr BaseFS =
+vfs::getRealFileSystem());
~ClangTool();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41535: Add -vfsoverlay option for Clang-Tidy
vladimir.plyashkun updated this revision to Diff 129440.
vladimir.plyashkun added a reviewer: ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41535
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/ClangTidyOptions.cpp
clang-tidy/ClangTidyOptions.h
clang-tidy/tool/ClangTidyMain.cpp
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -209,6 +209,12 @@
cl::init(false),
cl::cat(ClangTidyCategory));
+static cl::opt VfsOverlay("vfsoverlay", cl::desc(R"(
+Overlay the virtual filesystem described by file over the real file system.
+)"),
+ cl::value_desc("filename"),
+ cl::cat(ClangTidyCategory));
+
namespace clang {
namespace tidy {
@@ -295,6 +301,7 @@
DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
DefaultOptions.FormatStyle = FormatStyle;
DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
+ DefaultOptions.VfsOverlay = VfsOverlay;
// USERNAME is used on Windows.
if (!DefaultOptions.User)
DefaultOptions.User = llvm::sys::Process::GetEnv("USERNAME");
@@ -312,6 +319,8 @@
OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
if (FormatStyle.getNumOccurrences() > 0)
OverrideOptions.FormatStyle = FormatStyle;
+ if (VfsOverlay.getNumOccurrences() > 0)
+OverrideOptions.VfsOverlay = VfsOverlay;
if (!Config.empty()) {
if (llvm::ErrorOr ParsedConfig =
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -108,6 +108,10 @@
/// \brief Add extra compilation arguments to the start of the list.
llvm::Optional ExtraArgsBefore;
+
+ /// \brief Overlay the virtual filesystem described by file
+ ///over the real file system
+ llvm::Optional VfsOverlay;
};
/// \brief Abstract interface for retrieving various ClangTidy options.
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -92,6 +92,7 @@
IO.mapOptional("CheckOptions", NOpts->Options);
IO.mapOptional("ExtraArgs", Options.ExtraArgs);
IO.mapOptional("ExtraArgsBefore", Options.ExtraArgsBefore);
+IO.mapOptional("VfsOverlay", Options.VfsOverlay);
}
};
@@ -152,6 +153,7 @@
overrideValue(Result.User, Other.User);
mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
mergeVectors(Result.ExtraArgsBefore, Other.ExtraArgsBefore);
+ overrideValue(Result.VfsOverlay, Other.VfsOverlay);
for (const auto &KeyValue : Other.CheckOptions)
Result.CheckOptions[KeyValue.first] = KeyValue.second;
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -166,6 +166,9 @@
/// \brief Returns all collected errors.
ArrayRef getErrors() const { return Errors; }
+ /// \brief Returns diagnostics engine
+ DiagnosticsEngine &getDiagnosticsEngine() const { return *DiagEngine; }
+
/// \brief Clears collected errors.
void clearErrors() { Errors.clear(); }
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -87,17 +87,46 @@
ClangTidyContext &Context;
};
+void pushVfsOverlayFromFile(StringRef OverlayFile,
+DiagnosticsEngine &Diags,
+vfs::OverlayFileSystem &OverlayFS) {
+ if (OverlayFile.empty())
+return;
+
+ llvm::ErrorOr> Buffer =
+ OverlayFS.getBufferForFile(OverlayFile);
+ if (!Buffer) {
+Diags.Report(diag::err_missing_vfs_overlay_file) << OverlayFile;
+return;
+ }
+
+ IntrusiveRefCntPtr FS = vfs::getVFSFromYAML(
+ std::move(Buffer.get()), /*DiagHandler*/ nullptr, OverlayFile);
+ if (!FS.get()) {
+Diags.Report(diag::err_invalid_vfs_overlay) << OverlayFile;
+return;
+ }
+ OverlayFS.pushOverlay(FS);
+}
+
class ErrorReporter {
public:
ErrorReporter(ClangTidyContext &Context, bool ApplyFixes)
- : Files(FileSystemOptions()), DiagOpts(new DiagnosticOptions()),
+ : OverlayFS(new vfs::OverlayFileSystem(vfs::getRealFileSystem())),
+Files(FileSystemOptions(), OverlayFS),
+DiagOpts(new DiagnosticOptions()),
DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)),
Diags(IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts,
DiagPrinter),
SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes),
TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) {
[PATCH] D41535: Add -vfsoverlay option for Clang-Tidy
vladimir.plyashkun added a comment. Second part here (provided default virtual filesystem argument to ClangTool constructor): https://reviews.llvm.org/D41947 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41535 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
baloghadamsoftware updated this revision to Diff 129441.
baloghadamsoftware added a comment.
Updated to be based upon https://reviews.llvm.org/D41938.
https://reviews.llvm.org/D35110
Files:
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
test/Analysis/constraint_manager_negate_difference.c
test/Analysis/ptr-arith.c
Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -265,49 +265,26 @@
clang_analyzer_eval((rhs - lhs) > 0); // expected-warning{{TRUE}}
}
-//---
-// False positives
-//---
-
void zero_implies_reversed_equal(int *lhs, int *rhs) {
clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
if ((rhs - lhs) == 0) {
-#ifdef ANALYZER_CM_Z3
clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
return;
}
clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-#ifdef ANALYZER_CM_Z3
clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
-#else
- clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-#endif
}
void canonical_equal(int *lhs, int *rhs) {
clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
if (lhs == rhs) {
-#ifdef ANALYZER_CM_Z3
clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
return;
}
clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
-
-#ifdef ANALYZER_CM_Z3
clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
-#else
- clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
}
void compare_element_region_and_base(int *p) {
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- /dev/null
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -0,0 +1,64 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_eval(int);
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+void assert_in_range(int x) {
+ // Assert that no overflows occur in this test file.
+ // Assuming that concrete integers are also within that range.
+ if (x > ((int)INT_MAX / 4))
+exit(1);
+ if (x < -(((int)INT_MAX) / 4))
+exit(1);
+}
+
+void assert_in_range_2(int m, int n) {
+ assert_in_range(m);
+ assert_in_range(n);
+}
+
+void equal(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m != n)
+return;
+ clang_analyzer_eval(n == m); // expected-warning{{TRUE}}
+}
+
+void non_equal(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m == n)
+return;
+ clang_analyzer_eval(n != m); // expected-warning{{TRUE}}
+}
+
+void less_or_equal(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m < n)
+return;
+ clang_analyzer_eval(n <= m); // expected-warning{{TRUE}}
+}
+
+void less(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m <= n)
+return;
+ clang_analyzer_eval(n < m); // expected-warning{{TRUE}}
+}
+
+void greater_or_equal(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m > n)
+return;
+ clang_analyzer_eval(n >= m); // expected-warning{{TRUE}}
+}
+
+void greater(int m, int n) {
+ assert_in_range_2(m, n);
+ if (m >= n)
+return;
+ clang_analyzer_eval(n > m); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -256,6 +256,29 @@
return newRanges;
}
+ // Turn all [A, B] ranges to [-B, -A]. Turn minimal signed value to maximal
+ // signed value and vice versa.
+ RangeSet Negate(BasicValueFactory &BV, Factory &F) const {
+PrimRangeSet newRanges = F.getEmptySet();
+
+for (iterator i = begin(), e = end(); i != e; ++i) {
+ const llvm::APSInt &from = i->From(), &to = i->To();
+ const llvm::APSInt &newFrom = (to.isMinSignedValue() ?
+ BV.getMaxValue(to) :
+ (to.isMaxSignedValue() ?
+ BV.getMinValue(to) :
+ BV.getValue(- to)));
+ const llvm::APSInt &newTo = (from.isMinSignedValue() ?
+ BV.getMaxValue(from) :
+
r322278 - [OpenCL] Reorder the CLK_sRGBx/sRGBA defines, NFC
Author: svenvh Date: Thu Jan 11 06:05:38 2018 New Revision: 322278 URL: http://llvm.org/viewvc/llvm-project?rev=322278&view=rev Log: [OpenCL] Reorder the CLK_sRGBx/sRGBA defines, NFC Swap them so that all channel order defines are ordered according to their values. Modified: cfe/trunk/lib/Headers/opencl-c.h Modified: cfe/trunk/lib/Headers/opencl-c.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/opencl-c.h?rev=322278&r1=322277&r2=322278&view=diff == --- cfe/trunk/lib/Headers/opencl-c.h (original) +++ cfe/trunk/lib/Headers/opencl-c.h Thu Jan 11 06:05:38 2018 @@ -15421,8 +15421,8 @@ int __ovld __cnfn get_image_channel_data #define CLK_DEPTH_STENCIL 0x10BE #if __OPENCL_C_VERSION__ >= CL_VERSION_2_0 #define CLK_sRGB 0x10BF -#define CLK_sRGBA 0x10C1 #define CLK_sRGBx 0x10C0 +#define CLK_sRGBA 0x10C1 #define CLK_sBGRA 0x10C2 #define CLK_ABGR 0x10C3 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks
baloghadamsoftware updated this revision to Diff 129445.
baloghadamsoftware added a comment.
Updated to be based upon https://reviews.llvm.org/D41938 and
https://reviews.llvm.org/D35110.
https://reviews.llvm.org/D32642
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
test/Analysis/Inputs/system-header-simulator-cxx.h
test/Analysis/constraint_manager_negate_difference.c
test/Analysis/diagnostics/explicit-suppression.cpp
test/Analysis/iterator-range.cpp
Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config c++-container-inlining=false %s -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
#include "Inputs/system-header-simulator-cxx.h"
@@ -13,7 +13,110 @@
}
}
+void simple_good_end_negated(const std::vector &v) {
+ auto i = v.end();
+ if (!(i == v.end())) {
+clang_analyzer_warnIfReached();
+*i; // no-warning
+ }
+}
+
void simple_bad_end(const std::vector &v) {
auto i = v.end();
*i; // expected-warning{{Iterator accessed outside of its range}}
}
+
+void simple_good_begin(const std::vector &v) {
+ auto i = v.begin();
+ if (i != v.begin()) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+ }
+}
+
+void simple_good_begin_negated(const std::vector &v) {
+ auto i = v.begin();
+ if (!(i == v.begin())) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+ }
+}
+
+void simple_bad_begin(const std::vector &v) {
+ auto i = v.begin();
+ *--i; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy(const std::vector &v) {
+ auto i1 = v.end();
+ auto i2 = i1;
+ *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void decrease(const std::vector &v) {
+ auto i = v.end();
+ --i;
+ *i; // no-warning
+}
+
+void copy_and_decrease1(const std::vector &v) {
+ auto i1 = v.end();
+ auto i2 = i1;
+ --i1;
+ *i1; // no-warning
+}
+
+void copy_and_decrease2(const std::vector &v) {
+ auto i1 = v.end();
+ auto i2 = i1;
+ --i1;
+ *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy_and_increase1(const std::vector &v) {
+ auto i1 = v.begin();
+ auto i2 = i1;
+ ++i1;
+ if (i1 == v.end())
+*i2; // no-warning
+}
+
+void copy_and_increase2(const std::vector &v) {
+ auto i1 = v.begin();
+ auto i2 = i1;
+ ++i1;
+ if (i2 == v.end())
+*i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy_and_increase3(const std::vector &v) {
+ auto i1 = v.begin();
+ auto i2 = i1;
+ ++i1;
+ if (v.end() == i2)
+*i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void tricky(std::vector &V, int e) {
+ const auto first = V.begin();
+ const auto comp1 = (first != V.end()), comp2 = (first == V.end());
+ if (comp1)
+*first;
+}
+
+void loop(std::vector &V, int e) {
+ auto start = V.begin();
+ while (true) {
+auto item = std::find(start, V.end(), e);
+if (item == V.end())
+ break;
+*item; // no-warning
+start = ++item; // no-warning
+ }
+}
+
+void bad_move(std::list &L1, std::list &L2) {
+ auto i0 = --L2.cend();
+ L1 = std::move(L2);
+ *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
void testCopyNull(C *I, C *E) {
std::copy(I, E, (C *)0);
#ifndef SUPPRESSED
- // expected-warning@../Inputs/system-header-simulator-cxx.h:490 {{Called C++ object pointer is null}}
+ // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
#endif
}
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- test/Analysis/constraint_manager_negate_difference.c
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=deb
[PATCH] D40677: [libcxx] Make std::basic_istream::get 0-terminate input array in case of error.
mclow.lists added a comment.
A couple of nits, but other than that, looks fine.
Comment at: libcxx/include/istream:970
}
+if (__n > 0)
+{
I'm not a big fan of "putting braces around single statement blocks", and (see
line 963) that doesn't match the rest of libcxx.
Comment at:
libcxx/test/std/input.output/iostream.format/input.streams/istream.unformatted/get_pointer_size_chart.pass.cpp:16
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
+
Does this need to XFAIL for clang < 6 as well?
https://reviews.llvm.org/D40677
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
dtzWill added a comment. Ping! Is this stalling on reviewer attention? If so, please submit to LLVM Weekly's review corner (assuming it works with current LLVM), see http://llvmweekly.org/reviewcorner for details. Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41816: [analyzer] Model and check unrepresentable left shifts
rnkovacs updated this revision to Diff 129448.
rnkovacs added a comment.
I extended the warning message to include more information. What do you think?
https://reviews.llvm.org/D41816
Files:
lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/BasicValueFactory.cpp
test/Analysis/bitwise-ops.c
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -51,3 +51,9 @@
}
return 0;
}
+
+int testUnrepresentableLeftShift(int a) {
+ if (a == 8)
+return a << 30; // expected-warning{{The result of the left shift is
undefined due to shifting '8' by '30', which is unrepresentable in the unsigned
version of the return type 'int'}}
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -224,7 +224,6 @@
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
if (V1.isSigned() && V1.isNegative())
return nullptr;
@@ -236,16 +235,17 @@
if (Amt >= V1.getBitWidth())
return nullptr;
+ if (V1.isSigned() && Amt > V1.countLeadingZeros())
+return nullptr;
+
return &getValue( V1.operator<<( (unsigned) Amt ));
}
case BO_Shr: {
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
-
if (V2.isSigned() && V2.isNegative())
return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -141,6 +141,19 @@
C.isNegative(B->getLHS())) {
OS << "The result of the left shift is undefined because the left "
"operand is negative";
+ } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl) {
+SValBuilder &SB = C.getSValBuilder();
+const llvm::APSInt *LHS =
+SB.getKnownValue(state, C.getSVal(B->getLHS()));
+const llvm::APSInt *RHS =
+SB.getKnownValue(state, C.getSVal(B->getRHS()));
+if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros()) {
+ OS << "The result of the left shift is undefined due to shifting \'"
+ << LHS->getSExtValue() << "\' by \'" << RHS->getZExtValue()
+ << "\', which is unrepresentable in the unsigned version of "
+ << "the return type \'" << B->getLHS()->getType().getAsString()
+ << "\'";
+}
} else {
OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -51,3 +51,9 @@
}
return 0;
}
+
+int testUnrepresentableLeftShift(int a) {
+ if (a == 8)
+return a << 30; // expected-warning{{The result of the left shift is undefined due to shifting '8' by '30', which is unrepresentable in the unsigned version of the return type 'int'}}
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -224,7 +224,6 @@
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
if (V1.isSigned() && V1.isNegative())
return nullptr;
@@ -236,16 +235,17 @@
if (Amt >= V1.getBitWidth())
return nullptr;
+ if (V1.isSigned() && Amt > V1.countLeadingZeros())
+return nullptr;
+
return &getValue( V1.operator<<( (unsigned) Amt ));
}
case BO_Shr: {
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
-
if (V2.isSigned() && V2.isNegative())
return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -141,6 +141,19 @@
C.isNegative(B->getLHS())) {
OS << "The result of the left shift is undefined because the left "
"operand is nega
[PATCH] D40677: [libcxx] Make std::basic_istream::get 0-terminate input array in case of error.
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. Please make the formatting match the rest of the file. Comment at: libcxx/test/std/input.output/iostream.format/input.streams/istream.unformatted/get_pointer_size_chart.pass.cpp:16 +// XFAIL: with_system_cxx_lib=macosx10.8 +// XFAIL: with_system_cxx_lib=macosx10.7 + mclow.lists wrote: > Does this need to XFAIL for clang < 6 as well? > I don't think that it does. Never mind this. https://reviews.llvm.org/D40677 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line
russellmcc added a comment. ping! Thanks for your consideration Repository: rC Clang https://reviews.llvm.org/D40988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
mclow.lists added inline comments. Comment at: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp:67 non_empty_dir, -file_in_bad_dir, +// file_in_bad_dir, // produces "St13exception_ptruncaught_exceptions not yet implemented" }; mclow.lists wrote: > TyanNN wrote: > > mclow.lists wrote: > > > TyanNN wrote: > > > > This error is quite strange: it is not inherited from std::exception, > > > > one can only really find our the error with std::current_exception, and > > > > it happens when you use the path in fs::remove and fs::remove_all. Any > > > > idea about this? @mclow.lists > > > I'm not seeing this error on my system (nor are any of the bots reporting > > > it AFAIK). > > > > > > That looks like "`uncaught_exceptions` is not yet implemented", and that > > > comes from the ABI library. > > > > > > What are you using for an ABI library? Is it libc++abi? > > > > > Yes, I use libc++abi from trunk that I've built in llvm tree with libc++. I > > don't have a system wide installation of libc++abi so that must be it. Does > > uncommenting the line really work on your system? For the tests report an > > uncaught exception and fail. > It's definitely not commented out on my system. > (or on the trunk) > I see the message "uncaught_exceptions not yet implemented" in src/support/runtime/exception_fallback.ipp - but only in an #ifdef block for `__EMSCRIPTEN__` https://reviews.llvm.org/D41830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
TyanNN updated this revision to Diff 129453.
TyanNN added a comment.
Fix tests
https://reviews.llvm.org/D41830
Files:
src/experimental/filesystem/operations.cpp
test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
Index: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
===
--- test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
+++ test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
@@ -64,16 +64,28 @@
permissions(bad_perms_file, perms::none);
const path testCases[] = {
-env.make_env_path("dne"),
file_in_bad_dir
};
const auto BadRet = static_cast(-1);
for (auto& p : testCases) {
std::error_code ec;
+
TEST_CHECK(fs::remove_all(p, ec) == BadRet);
TEST_CHECK(ec);
TEST_CHECK(checkThrow(p, ec));
}
+
+// PR#35780
+const path testCasesNonexistant[] = {
+"",
+env.make_env_path("dne")
+};
+for (auto &p : testCasesNonexistant) {
+std::error_code ec;
+
+TEST_CHECK(fs::remove_all(p) == 0);
+TEST_CHECK(!ec);
+}
}
TEST_CASE(basic_remove_all_test)
Index: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
===
--- test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
+++ test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
@@ -61,17 +61,29 @@
const path file_in_bad_dir = env.create_file(bad_perms_dir / "file", 42);
permissions(bad_perms_dir, perms::none);
const path testCases[] = {
-"",
-env.make_env_path("dne"),
non_empty_dir,
file_in_bad_dir,
};
for (auto& p : testCases) {
std::error_code ec;
+
TEST_CHECK(!fs::remove(p, ec));
TEST_CHECK(ec);
TEST_CHECK(checkThrow(p, ec));
}
+
+// PR#35780
+const path testCasesNonexistant[] = {
+"",
+env.make_env_path("dne")
+};
+
+for (auto& p : testCasesNonexistant) {
+std::error_code ec;
+
+TEST_CHECK(!fs::remove(p, ec));
+TEST_CHECK(!ec);
+}
}
TEST_CASE(basic_remove_test)
Index: src/experimental/filesystem/operations.cpp
===
--- src/experimental/filesystem/operations.cpp
+++ src/experimental/filesystem/operations.cpp
@@ -661,8 +661,10 @@
bool __remove(const path& p, std::error_code *ec) {
if (ec) ec->clear();
+
if (::remove(p.c_str()) == -1) {
-set_or_throw(ec, "remove", p);
+if (errno != ENOENT)
+set_or_throw(ec, "remove", p);
return false;
}
return true;
@@ -692,13 +694,18 @@
} // end namespace
std::uintmax_t __remove_all(const path& p, std::error_code *ec) {
+if (ec) ec->clear();
+
std::error_code mec;
auto count = remove_all_impl(p, mec);
if (mec) {
-set_or_throw(mec, ec, "remove_all", p);
-return static_cast(-1);
+if (mec == errc::no_such_file_or_directory) {
+return 0;
+} else {
+set_or_throw(mec, ec, "remove_all", p);
+return static_cast(-1);
+}
}
-if (ec) ec->clear();
return count;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
TyanNN added inline comments. Comment at: test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp:67 non_empty_dir, -file_in_bad_dir, +// file_in_bad_dir, // produces "St13exception_ptruncaught_exceptions not yet implemented" }; mclow.lists wrote: > mclow.lists wrote: > > TyanNN wrote: > > > mclow.lists wrote: > > > > TyanNN wrote: > > > > > This error is quite strange: it is not inherited from std::exception, > > > > > one can only really find our the error with std::current_exception, > > > > > and it happens when you use the path in fs::remove and > > > > > fs::remove_all. Any idea about this? @mclow.lists > > > > I'm not seeing this error on my system (nor are any of the bots > > > > reporting it AFAIK). > > > > > > > > That looks like "`uncaught_exceptions` is not yet implemented", and > > > > that comes from the ABI library. > > > > > > > > What are you using for an ABI library? Is it libc++abi? > > > > > > > Yes, I use libc++abi from trunk that I've built in llvm tree with libc++. > > > I don't have a system wide installation of libc++abi so that must be it. > > > Does uncommenting the line really work on your system? For the tests > > > report an uncaught exception and fail. > > It's definitely not commented out on my system. > > (or on the trunk) > > > I see the message "uncaught_exceptions not yet implemented" in > src/support/runtime/exception_fallback.ipp - but only in an #ifdef block for > `__EMSCRIPTEN__` Well, it somehow got fixed after moving tests for nonexistant files into a separate loop, so that's good. https://reviews.llvm.org/D41830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39903: [libclang] Allow pretty printing declarations
jbcoe added a comment. It might be worth adding some very simple get/set tests to ensure that properties are set as intended. Comment at: tools/libclang/CIndex.cpp:4720 + +#define HANDLE_CASE(P, PROPERTY_NAME) \ + case CXPrintingPolicy_##PROPERTY_NAME: \ I’m not convinced that the code replaced by the macro is sufficiently complicated to justify use of a macro. Comment at: tools/libclang/CIndex.cpp:4763 +#undef HANDLE_CASE +#define HANDLE_CASE(P, PROPERTY_NAME, VALUE) \ + case CXPrintingPolicy_##PROPERTY_NAME: \ I’m not convinced that the code replaced by the macro is sufficiently complicated to justify use of a macro. Repository: rC Clang https://reviews.llvm.org/D39903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. LGTM. Do you need me to commit this? https://reviews.llvm.org/D41830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
TyanNN added a comment. In https://reviews.llvm.org/D41830#973419, @mclow.lists wrote: > LGTM. > > Do you need me to commit this? Nope, i can do it myself https://reviews.llvm.org/D41830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
This revision was automatically updated to reflect the committed changes.
Closed by commit rL322293: Make std::experimental::filesystem::remove and
remove_all return false or 0 if… (authored by vaartis, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D41830?vs=129453&id=129457#toc
Repository:
rL LLVM
https://reviews.llvm.org/D41830
Files:
libcxx/trunk/src/experimental/filesystem/operations.cpp
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
Index: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
===
--- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
+++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
@@ -61,17 +61,29 @@
const path file_in_bad_dir = env.create_file(bad_perms_dir / "file", 42);
permissions(bad_perms_dir, perms::none);
const path testCases[] = {
-"",
-env.make_env_path("dne"),
non_empty_dir,
file_in_bad_dir,
};
for (auto& p : testCases) {
std::error_code ec;
+
TEST_CHECK(!fs::remove(p, ec));
TEST_CHECK(ec);
TEST_CHECK(checkThrow(p, ec));
}
+
+// PR#35780
+const path testCasesNonexistant[] = {
+"",
+env.make_env_path("dne")
+};
+
+for (auto& p : testCasesNonexistant) {
+std::error_code ec;
+
+TEST_CHECK(!fs::remove(p, ec));
+TEST_CHECK(!ec);
+}
}
TEST_CASE(basic_remove_test)
Index: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
===
--- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
+++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
@@ -64,16 +64,28 @@
permissions(bad_perms_file, perms::none);
const path testCases[] = {
-env.make_env_path("dne"),
file_in_bad_dir
};
const auto BadRet = static_cast(-1);
for (auto& p : testCases) {
std::error_code ec;
+
TEST_CHECK(fs::remove_all(p, ec) == BadRet);
TEST_CHECK(ec);
TEST_CHECK(checkThrow(p, ec));
}
+
+// PR#35780
+const path testCasesNonexistant[] = {
+"",
+env.make_env_path("dne")
+};
+for (auto &p : testCasesNonexistant) {
+std::error_code ec;
+
+TEST_CHECK(fs::remove_all(p) == 0);
+TEST_CHECK(!ec);
+}
}
TEST_CASE(basic_remove_all_test)
Index: libcxx/trunk/src/experimental/filesystem/operations.cpp
===
--- libcxx/trunk/src/experimental/filesystem/operations.cpp
+++ libcxx/trunk/src/experimental/filesystem/operations.cpp
@@ -661,8 +661,10 @@
bool __remove(const path& p, std::error_code *ec) {
if (ec) ec->clear();
+
if (::remove(p.c_str()) == -1) {
-set_or_throw(ec, "remove", p);
+if (errno != ENOENT)
+set_or_throw(ec, "remove", p);
return false;
}
return true;
@@ -692,13 +694,18 @@
} // end namespace
std::uintmax_t __remove_all(const path& p, std::error_code *ec) {
+if (ec) ec->clear();
+
std::error_code mec;
auto count = remove_all_impl(p, mec);
if (mec) {
-set_or_throw(mec, ec, "remove_all", p);
-return static_cast(-1);
+if (mec == errc::no_such_file_or_directory) {
+return 0;
+} else {
+set_or_throw(mec, ec, "remove_all", p);
+return static_cast(-1);
+}
}
-if (ec) ec->clear();
return count;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r322295 - Fix some too-big local arrays. Thanks to dcdillon for the patch. Reviewed as D28217
Author: marshall
Date: Thu Jan 11 09:16:52 2018
New Revision: 322295
URL: http://llvm.org/viewvc/llvm-project?rev=322295&view=rev
Log:
Fix some too-big local arrays. Thanks to dcdillon for the patch. Reviewed as
D28217
Modified:
libcxx/trunk/src/locale.cpp
Modified: libcxx/trunk/src/locale.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/locale.cpp?rev=322295&r1=322294&r2=322295&view=diff
==
--- libcxx/trunk/src/locale.cpp (original)
+++ libcxx/trunk/src/locale.cpp Thu Jan 11 09:16:52 2018
@@ -4659,7 +4659,7 @@ static
string*
init_am_pm()
{
-static string am_pm[24];
+static string am_pm[2];
am_pm[0] = "AM";
am_pm[1] = "PM";
return am_pm;
@@ -4669,7 +4669,7 @@ static
wstring*
init_wam_pm()
{
-static wstring am_pm[24];
+static wstring am_pm[2];
am_pm[0] = L"AM";
am_pm[1] = L"PM";
return am_pm;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28217: [libc++] Overallocation of am_pm array in locale.cpp
mclow.lists closed this revision. mclow.lists added a comment. Committed as r322295 https://reviews.llvm.org/D28217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit
dschuff added inline comments.
Comment at: lib/Basic/Targets/WebAssembly.h:108
IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.
I think we want least16_t to still be short, no? We do still support 16-bit
shorts, so my interpretation is that the smallest type with width of at least
16 should still be 16.
Repository:
rC Clang
https://reviews.llvm.org/D41941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41800: [analyzer] Use a custom program point for the check::NewAllocator callback.
NoQ added inline comments. Comment at: test/Analysis/NewDelete-path-notes.cpp:44 // CHECK-NEXT: -// CHECK-NEXT:line6 +// CHECK-NEXT:line7 // CHECK-NEXT:col3 a.sidorin wrote: > Not even a minor concern for this patch, but I think that placing `//RUN` and > `//CHECK` after the code being tested could save us from massive changes of > line numbers. Hmm, not sure if i understand, you mean //before// the code? (it would save us from line number changes in plists, but it'd make the tests harder to read because you'd have to scroll all the way down through the plist to find the actual code). https://reviews.llvm.org/D41800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit
ncw added inline comments.
Comment at: lib/Basic/Targets/WebAssembly.h:108
IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.
dschuff wrote:
> I think we want least16_t to still be short, no? We do still support 16-bit
> shorts, so my interpretation is that the smallest type with width of at least
> 16 should still be 16.
...is there a way to do that? I couldn't find any other archs that do it; it
seems like the stdint.h that Clang provides requires least16_t to match
fast16_t. I copied this from the AVR target, although maybe that doesn't
support 16-bit at all.
Repository:
rC Clang
https://reviews.llvm.org/D41941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41800: [analyzer] Use a custom program point for the check::NewAllocator callback.
a.sidorin added inline comments. Comment at: test/Analysis/NewDelete-path-notes.cpp:44 // CHECK-NEXT: -// CHECK-NEXT:line6 +// CHECK-NEXT:line7 // CHECK-NEXT:col3 NoQ wrote: > a.sidorin wrote: > > Not even a minor concern for this patch, but I think that placing `//RUN` > > and `//CHECK` after the code being tested could save us from massive > > changes of line numbers. > Hmm, not sure if i understand, you mean //before// the code? (it would save > us from line number changes in plists, but it'd make the tests harder to read > because you'd have to scroll all the way down through the plist to find the > actual code). I mean placing RUNs after the program code (but before `// CHECK`. Anyway, moving RUNs below will cause... line changes so it is not an important issue. https://reviews.llvm.org/D41800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit
ncw added inline comments.
Comment at: lib/Basic/Targets/WebAssembly.h:108
IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.
ncw wrote:
> dschuff wrote:
> > I think we want least16_t to still be short, no? We do still support 16-bit
> > shorts, so my interpretation is that the smallest type with width of at
> > least 16 should still be 16.
> ...is there a way to do that? I couldn't find any other archs that do it; it
> seems like the stdint.h that Clang provides requires least16_t to match
> fast16_t. I copied this from the AVR target, although maybe that doesn't
> support 16-bit at all.
Sorry, now I see that AVR uses int because it has 16-bit ints...
There isn't any existing Clang target that uses 32-bit for fast16_t, so maybe
it's currently not possible within Clang's framework (or at least, not without
also fiddling with least16_t). `lib/Frontend/InitPreprocessor.cpp` hardcodes
some logic with sets them to be the same.
I can abandon this review if that's not acceptable collateral damage (probably
not, on reflection) - or could tweak InitPreprocessor.cpp and stdint.h to be
more flexible (might need more review if you don't "own" those files?)
Repository:
rC Clang
https://reviews.llvm.org/D41941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy
leanil updated this revision to Diff 129465.
leanil added a comment.
Change result types to match the query return types.
https://reviews.llvm.org/D41384
Files:
lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
test/Analysis/security-syntax-checks.m
Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -146,6 +146,11 @@
strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as
it does not provide bounding of the memory buffer. Replace unbounded copy
functions with analogous functions that support length arguments such as
'strlcpy'. CWE-119}}
}
+void test_strcpy_safe() {
+ char x[5];
+ strcpy(x, "abcd");
+}
+
//===--===
// strcat()
//===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,22 @@
if (!checkCall_strCommon(CE, FD))
return;
+ unsigned long long ArraySize = 0;
+ unsigned StrLen;
+ bool StrLenFound = false;
+ const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+ if (const auto *DeclRef = dyn_cast(Target))
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr()))
+ ArraySize = Array->getSize().getLimitedValue();
+ if (const auto *String = dyn_cast(Source)) {
+StrLen = String->getLength();
+StrLenFound = true;
+ }
+ if (StrLenFound && ArraySize >= StrLen + 1)
+return;
+
// Issue a warning.
PathDiagnosticLocation CELoc =
PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -146,6 +146,11 @@
strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119}}
}
+void test_strcpy_safe() {
+ char x[5];
+ strcpy(x, "abcd");
+}
+
//===--===
// strcat()
//===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,22 @@
if (!checkCall_strCommon(CE, FD))
return;
+ unsigned long long ArraySize = 0;
+ unsigned StrLen;
+ bool StrLenFound = false;
+ const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+ if (const auto *DeclRef = dyn_cast(Target))
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr()))
+ ArraySize = Array->getSize().getLimitedValue();
+ if (const auto *String = dyn_cast(Source)) {
+StrLen = String->getLength();
+StrLenFound = true;
+ }
+ if (StrLenFound && ArraySize >= StrLen + 1)
+return;
+
// Issue a warning.
PathDiagnosticLocation CELoc =
PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41952: Clang test changes for 'no-overflow' flag for sdiv\udiv instructions
erichkeane created this revision. erichkeane added reviewers: magabari, zvi, craig.topper, DavidKreitzer, hsaito, nlopes, reames, MatzeB, eli.friedman, rengolin, hfinkel. Herald added subscribers: kbarton, nemanjai. See: https://reviews.llvm.org/D41944 These are the Clang CodeGen test changes required for the no-overflow flag. See RFC discussion here: https://groups.google.com/forum/#!msg/llvm-dev/eFtnCwpMMhs/eAHQj8rJCAAJ;context-place=searchin/llvm-dev/magabari%7Csort:date Repository: rC Clang https://reviews.llvm.org/D41952 Files: test/CodeGen/atomic_ops.c test/CodeGen/builtins-ppc-altivec.c test/CodeGen/builtins-ppc-vsx.c test/CodeGen/compound-type.c test/CodeGen/exceptions-seh.c test/CodeGen/ext-vector.c test/CodeGen/sanitize-trap.c test/CodeGen/ubsan-pass-object-size.c test/CodeGen/vla.c test/CodeGen/zvector.c test/CodeGenCXX/vla.cpp test/OpenMP/atomic_capture_codegen.cpp test/OpenMP/atomic_update_codegen.cpp test/OpenMP/declare_reduction_codegen.cpp test/OpenMP/for_lastprivate_codegen.cpp test/OpenMP/for_linear_codegen.cpp test/OpenMP/for_reduction_codegen.cpp test/OpenMP/for_reduction_codegen_UDR.cpp test/OpenMP/for_simd_codegen.cpp test/OpenMP/loops_explicit_clauses_codegen.cpp test/OpenMP/nvptx_teams_reduction_codegen.cpp test/OpenMP/parallel_firstprivate_codegen.cpp test/OpenMP/parallel_for_simd_codegen.cpp test/OpenMP/parallel_private_codegen.cpp test/OpenMP/parallel_reduction_codegen.cpp test/OpenMP/simd_codegen.cpp test/OpenMP/single_codegen.cpp test/OpenMP/taskgroup_task_reduction_codegen.cpp test/OpenMP/taskloop_reduction_codegen.cpp test/OpenMP/taskloop_simd_reduction_codegen.cpp test/OpenMP/teams_private_codegen.cpp Index: test/CodeGen/ext-vector.c === --- test/CodeGen/ext-vector.c +++ test/CodeGen/ext-vector.c @@ -134,7 +134,7 @@ // CHECK: add <4 x i32> // CHECK: sub <4 x i32> // CHECK: mul <4 x i32> - // CHECK: sdiv <4 x i32> + // CHECK: sdiv nof <4 x i32> // CHECK: srem <4 x i32> a = a + b; a = a - b; @@ -145,7 +145,7 @@ // CHECK: add <4 x i32> // CHECK: sub <4 x i32> // CHECK: mul <4 x i32> - // CHECK: sdiv <4 x i32> + // CHECK: sdiv nof <4 x i32> // CHECK: srem <4 x i32> a = a + c; a = a - c; @@ -156,7 +156,7 @@ // CHECK: add <4 x i32> // CHECK: sub <4 x i32> // CHECK: mul <4 x i32> - // CHECK: sdiv <4 x i32> + // CHECK: sdiv nof <4 x i32> // CHECK: srem <4 x i32> a += b; a -= b; @@ -167,7 +167,7 @@ // CHECK: add <4 x i32> // CHECK: sub <4 x i32> // CHECK: mul <4 x i32> - // CHECK: sdiv <4 x i32> + // CHECK: sdiv nof <4 x i32> // CHECK: srem <4 x i32> a += c; a -= c; @@ -254,12 +254,12 @@ uint4 b = *bp; int4 d; - // CHECK: udiv <4 x i32> + // CHECK: udiv nof <4 x i32> // CHECK: urem <4 x i32> a = a / b; a = a % b; - // CHECK: udiv <4 x i32> + // CHECK: udiv nof <4 x i32> // CHECK: urem <4 x i32> a = a / c; a = a % c; Index: test/CodeGen/builtins-ppc-altivec.c === --- test/CodeGen/builtins-ppc-altivec.c +++ test/CodeGen/builtins-ppc-altivec.c @@ -1244,28 +1244,28 @@ /* vec_div */ res_vsc = vec_div(vsc, vsc); -// CHECK: sdiv <16 x i8> -// CHECK-LE: sdiv <16 x i8> +// CHECK: sdiv nof <16 x i8> +// CHECK-LE: sdiv nof <16 x i8> res_vuc = vec_div(vuc, vuc); -// CHECK: udiv <16 x i8> -// CHECK-LE: udiv <16 x i8> +// CHECK: udiv nof <16 x i8> +// CHECK-LE: udiv nof <16 x i8> res_vs = vec_div(vs, vs); -// CHECK: sdiv <8 x i16> -// CHECK-LE: sdiv <8 x i16> +// CHECK: sdiv nof <8 x i16> +// CHECK-LE: sdiv nof <8 x i16> res_vus = vec_div(vus, vus); -// CHECK: udiv <8 x i16> -// CHECK-LE: udiv <8 x i16> +// CHECK: udiv nof <8 x i16> +// CHECK-LE: udiv nof <8 x i16> res_vi = vec_div(vi, vi); -// CHECK: sdiv <4 x i32> -// CHECK-LE: sdiv <4 x i32> +// CHECK: sdiv nof <4 x i32> +// CHECK-LE: sdiv nof <4 x i32> res_vui = vec_div(vui, vui); -// CHECK: udiv <4 x i32> -// CHECK-LE: udiv <4 x i32> +// CHECK: udiv nof <4 x i32> +// CHECK-LE: udiv nof <4 x i32> /* vec_dss */ vec_dss(0); Index: test/CodeGen/vla.c === --- test/CodeGen/vla.c +++ test/CodeGen/vla.c @@ -118,12 +118,12 @@ // CHECK-NEXT: [[T0:%.*]] = load [6 x i8]*, [6 x i8]** [[P]], align 4 // CHECK-NEXT: [[T1:%.*]] = load i32, i32* [[N]], align 4 - // CHECK-NEXT: [[T2:%.*]] = udiv i32 [[T1]], 2 + // CHECK-NEXT: [[T2:%.*]] = udiv nof i32 [[T1]], 2 // CHECK-NEXT: [[T3:%.*]] = mul nuw i32 [[DIM0]], [[DIM1]] // CHECK-NEXT: [[T4:%.*]] = mul nsw i32 [[T2]], [[T3]] // CHECK-NEXT: [[T5:%.*]] = getelementptr inbounds [6 x i8], [6 x i8]* [[T0]], i32 [[T4]] // CHECK-NEXT: [[T6:%.*]] = load i32, i32* [[N]], align 4 - // CHECK-NEXT: [[T7:%.*]] = udiv i32 [[T6]], 4 + // CHECK-NEXT: [[T7:%.*]] = udiv nof i32 [[T6]], 4 // C
[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit
dschuff added inline comments.
Comment at: lib/Basic/Targets/WebAssembly.h:108
IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.
ncw wrote:
> ncw wrote:
> > dschuff wrote:
> > > I think we want least16_t to still be short, no? We do still support
> > > 16-bit shorts, so my interpretation is that the smallest type with width
> > > of at least 16 should still be 16.
> > ...is there a way to do that? I couldn't find any other archs that do it;
> > it seems like the stdint.h that Clang provides requires least16_t to match
> > fast16_t. I copied this from the AVR target, although maybe that doesn't
> > support 16-bit at all.
> Sorry, now I see that AVR uses int because it has 16-bit ints...
>
> There isn't any existing Clang target that uses 32-bit for fast16_t, so maybe
> it's currently not possible within Clang's framework (or at least, not
> without also fiddling with least16_t). `lib/Frontend/InitPreprocessor.cpp`
> hardcodes some logic with sets them to be the same.
>
> I can abandon this review if that's not acceptable collateral damage
> (probably not, on reflection) - or could tweak InitPreprocessor.cpp and
> stdint.h to be more flexible (might need more review if you don't "own" those
> files?)
I think it's worth trying to fix InitPreprocessor.cpp/stdint.h to remove the
assumption that those types are the same. We'll need to get broader review, so
it will be slower than if we were just changing our own files, but that's OK.
If you're up for doing that I'd be happy to help you find reviewers, or
otherwise I can take a shot at it; now is a good time since we're taking a
closer look at our ABIs more generally anyway.
Repository:
rC Clang
https://reviews.llvm.org/D41941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit
ncw added inline comments.
Comment at: lib/Basic/Targets/WebAssembly.h:108
IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.
dschuff wrote:
> ncw wrote:
> > ncw wrote:
> > > dschuff wrote:
> > > > I think we want least16_t to still be short, no? We do still support
> > > > 16-bit shorts, so my interpretation is that the smallest type with
> > > > width of at least 16 should still be 16.
> > > ...is there a way to do that? I couldn't find any other archs that do it;
> > > it seems like the stdint.h that Clang provides requires least16_t to
> > > match fast16_t. I copied this from the AVR target, although maybe that
> > > doesn't support 16-bit at all.
> > Sorry, now I see that AVR uses int because it has 16-bit ints...
> >
> > There isn't any existing Clang target that uses 32-bit for fast16_t, so
> > maybe it's currently not possible within Clang's framework (or at least,
> > not without also fiddling with least16_t).
> > `lib/Frontend/InitPreprocessor.cpp` hardcodes some logic with sets them to
> > be the same.
> >
> > I can abandon this review if that's not acceptable collateral damage
> > (probably not, on reflection) - or could tweak InitPreprocessor.cpp and
> > stdint.h to be more flexible (might need more review if you don't "own"
> > those files?)
> I think it's worth trying to fix InitPreprocessor.cpp/stdint.h to remove the
> assumption that those types are the same. We'll need to get broader review,
> so it will be slower than if we were just changing our own files, but that's
> OK. If you're up for doing that I'd be happy to help you find reviewers, or
> otherwise I can take a shot at it; now is a good time since we're taking a
> closer look at our ABIs more generally anyway.
uhh... if you could take a look that would be great! I've only got a limited
Wasm "budget" from my employer, and have to get back to customer work for most
of the rest of the week.
Was there an issue I seem to remember as well, where Wasm32 was using "unsigned
int" for __SIZE_TYPE__ instead of "unsigned long".
Repository:
rC Clang
https://reviews.llvm.org/D41941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41316: [libcxx] Allow random_device to be built optionally
weimingz added a comment. any suggestions? Repository: rCXX libc++ https://reviews.llvm.org/D41316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.
apazos updated this revision to Diff 129476.
apazos added a comment.
Herald added a subscriber: niosHD.
Rebased.
https://reviews.llvm.org/D41271
Files:
lib/Driver/ToolChains/Gnu.cpp
test/Driver/riscv-gnutools.c
Index: test/Driver/riscv-gnutools.c
===
--- /dev/null
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +1,18 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64 %s
+
+// MABI-ILP32: "/usr/bin/as" "-mabi" "ilp32"
+// MABI-LP64: "/usr/bin/as" "-mabi" "lp64"
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as -march=rv32g %s
-### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as -march=rv64g %s
-### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64-MARCH-G %s
+
+// MABI-ILP32-MARCH-G: "/usr/bin/as" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-LP64-MARCH-G: "/usr/bin/as" "-mabi" "lp64" "-march" "rv64g"
+
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
break;
}
+ case llvm::Triple::riscv32:
+ case llvm::Triple::riscv64: {
+StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
+CmdArgs.push_back("-mabi");
+CmdArgs.push_back(ABIName.data());
+if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
+ StringRef MArch = A->getValue();
+ CmdArgs.push_back("-march");
+ CmdArgs.push_back(MArch.data());
+}
+break;
+ }
case llvm::Triple::sparc:
case llvm::Triple::sparcel: {
CmdArgs.push_back("-32");
Index: test/Driver/riscv-gnutools.c
===
--- /dev/null
+++ test/Driver/riscv-gnutools.c
@@ -0,0 +1,18 @@
+// Check gnutools are invoked with propagated values for -mabi and -march.
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32 %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64 %s
+
+// MABI-ILP32: "/usr/bin/as" "-mabi" "ilp32"
+// MABI-LP64: "/usr/bin/as" "-mabi" "lp64"
+
+// RUN: %clang -target riscv32-unknown-elf -fno-integrated-as -march=rv32g %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-ILP32-MARCH-G %s
+// RUN: %clang -target riscv64-unknown-elf -fno-integrated-as -march=rv64g %s -### \
+// RUN: 2>&1 | FileCheck -check-prefix=MABI-LP64-MARCH-G %s
+
+// MABI-ILP32-MARCH-G: "/usr/bin/as" "-mabi" "ilp32" "-march" "rv32g"
+// MABI-LP64-MARCH-G: "/usr/bin/as" "-mabi" "lp64" "-march" "rv64g"
+
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -629,6 +629,18 @@
ppc::getPPCAsmModeForCPU(getCPUName(Args, getToolChain().getTriple(;
break;
}
+ case llvm::Triple::riscv32:
+ case llvm::Triple::riscv64: {
+StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
+CmdArgs.push_back("-mabi");
+CmdArgs.push_back(ABIName.data());
+if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
+ StringRef MArch = A->getValue();
+ CmdArgs.push_back("-march");
+ CmdArgs.push_back(MArch.data());
+}
+break;
+ }
case llvm::Triple::sparc:
case llvm::Triple::sparcel: {
CmdArgs.push_back("-32");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41941: [WebAssembly] Change int_fast16_t to 32-bit
dschuff added inline comments.
Comment at: lib/Basic/Targets/WebAssembly.h:108
IntType getLeastIntTypeByWidth(unsigned BitWidth, bool IsSigned) const final
{
-// WebAssembly uses long long for int_least64_t and int_fast64_t.
-return BitWidth == 64
+// WebAssembly uses long long for int_least64_t and int_fast64_t, and int
+// for int_least16_t and int_fast16_t.
ncw wrote:
> dschuff wrote:
> > ncw wrote:
> > > ncw wrote:
> > > > dschuff wrote:
> > > > > I think we want least16_t to still be short, no? We do still support
> > > > > 16-bit shorts, so my interpretation is that the smallest type with
> > > > > width of at least 16 should still be 16.
> > > > ...is there a way to do that? I couldn't find any other archs that do
> > > > it; it seems like the stdint.h that Clang provides requires least16_t
> > > > to match fast16_t. I copied this from the AVR target, although maybe
> > > > that doesn't support 16-bit at all.
> > > Sorry, now I see that AVR uses int because it has 16-bit ints...
> > >
> > > There isn't any existing Clang target that uses 32-bit for fast16_t, so
> > > maybe it's currently not possible within Clang's framework (or at least,
> > > not without also fiddling with least16_t).
> > > `lib/Frontend/InitPreprocessor.cpp` hardcodes some logic with sets them
> > > to be the same.
> > >
> > > I can abandon this review if that's not acceptable collateral damage
> > > (probably not, on reflection) - or could tweak InitPreprocessor.cpp and
> > > stdint.h to be more flexible (might need more review if you don't "own"
> > > those files?)
> > I think it's worth trying to fix InitPreprocessor.cpp/stdint.h to remove
> > the assumption that those types are the same. We'll need to get broader
> > review, so it will be slower than if we were just changing our own files,
> > but that's OK. If you're up for doing that I'd be happy to help you find
> > reviewers, or otherwise I can take a shot at it; now is a good time since
> > we're taking a closer look at our ABIs more generally anyway.
> uhh... if you could take a look that would be great! I've only got a limited
> Wasm "budget" from my employer, and have to get back to customer work for
> most of the rest of the week.
>
> Was there an issue I seem to remember as well, where Wasm32 was using
> "unsigned int" for __SIZE_TYPE__ instead of "unsigned long".
Yes, @sunfish is switching the wasm and asm.js ABIs to both use unsigned long.
Repository:
rC Clang
https://reviews.llvm.org/D41941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41316: [libcxx] Allow random_device to be built optionally
jroelofs added a comment. > Should we go with current patch? You'll need a bunch of `UNSUPPORTED: has-no-random-device` or something like it, but other than that, I'm happy with the patch as-is. Repository: rCXX libc++ https://reviews.llvm.org/D41316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41271: [RISCV] Propagate -mabi and -march values to GNU assembler.
asb added a comment. I've just had a painful time landing https://reviews.llvm.org/D39963 due to failures on the Windows buildbots. I think you'll have problems on Windows with the tests in this patch, as Windows surely won't default to /usr/bin/as. Sadly I don't have access to a windows Clang build to confirm what path it will default to. https://reviews.llvm.org/D41271 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322276 - [RISCV] Add the RISCV target and compiler driver
Author: asb Date: Thu Jan 11 05:36:56 2018 New Revision: 322276 URL: http://llvm.org/viewvc/llvm-project?rev=322276&view=rev Log: [RISCV] Add the RISCV target and compiler driver As RV64 codegen has not yet been upstreamed into LLVM, we focus on RV32 driver support (RV64 to follow). Differential Revision: https://reviews.llvm.org/D39963 Added: cfe/trunk/lib/Basic/Targets/RISCV.cpp cfe/trunk/lib/Basic/Targets/RISCV.h cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.h cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/bin/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/bin/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/include/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/include/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64d/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/ld (with props) cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32d/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32d/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64d/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib64/lp64d/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32d/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32d/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64/.keep cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64d/ cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64d/.keep cfe/trunk/test/Driver/riscv-abi.c cfe/trunk/test/Driver/riscv-features.c cfe/trunk/test/Driver/riscv32-toolchain.c cfe/trunk/test/Driver/riscv64-toolchain.c Modified: cfe/trunk/lib/Basic/CMakeLists.txt cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/lib/Driver/CMakeLists.txt cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/lib/Driver/ToolChains/Clang.h cfe/trunk/lib/Driver/ToolChains/Gnu.cpp cfe/trunk/lib/Driver/ToolChains/Linux.cpp cfe/trunk/test/Driver/frame-pointer.c cfe/trunk/test/Preprocessor/init.c Modified: cfe/trunk/lib/Basic/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/CMakeLists.txt?rev=322276&r1=322275&r2=322276&view=diff == --- cfe/trunk/lib/Basic/CMakeLists.txt (original) +++ cfe/trunk/lib/Basic/CMakeLists.txt Thu Jan 11 05:36:56 2018 @@ -83,6 +83,7 @@ ad
r322277 - [Driver][RISCV] Fix r322276 by adding missing dummy crtbegin.o files to test input
Author: asb Date: Thu Jan 11 05:51:06 2018 New Revision: 322277 URL: http://llvm.org/viewvc/llvm-project?rev=322277&view=rev Log: [Driver][RISCV] Fix r322276 by adding missing dummy crtbegin.o files to test input The dummy crtbegin.o files were left out in r322276 (as they were ignored by svn add of test/Driver/Inputs/multilib_riscv_linux_sdk) and are necessary for the driver test to work. Added: cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/crtbegin.o cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32/crtbegin.o cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d/crtbegin.o cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64/crtbegin.o cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64d/crtbegin.o Added: cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/crtbegin.o URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/crtbegin.o?rev=322277&view=auto == (empty) Added: cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32/crtbegin.o URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32/crtbegin.o?rev=322277&view=auto == (empty) Added: cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d/crtbegin.o URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d/crtbegin.o?rev=322277&view=auto == (empty) Added: cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64/crtbegin.o URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64/crtbegin.o?rev=322277&view=auto == (empty) Added: cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64d/crtbegin.o URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib64/lp64d/crtbegin.o?rev=322277&view=auto == (empty) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322286 - [Driver][RISCV] Fix r322276 for Windows (path separator issue)
Author: asb
Date: Thu Jan 11 07:38:01 2018
New Revision: 322286
URL: http://llvm.org/viewvc/llvm-project?rev=322286&view=rev
Log:
[Driver][RISCV] Fix r322276 for Windows (path separator issue)
We were seeing test failures of riscv32-toolchain.c on windows due to the \
path separator being used for the linker. Add {{/|}} pattern (made
horrible due to escaping), just like introduced in r214931.
Modified:
cfe/trunk/test/Driver/riscv32-toolchain.c
Modified: cfe/trunk/test/Driver/riscv32-toolchain.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/riscv32-toolchain.c?rev=322286&r1=322285&r2=322286&view=diff
==
--- cfe/trunk/test/Driver/riscv32-toolchain.c (original)
+++ cfe/trunk/test/Driver/riscv32-toolchain.c Thu Jan 11 07:38:01 2018
@@ -10,7 +10,7 @@
// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
// RUN: | FileCheck -check-prefix=CC1-RV32-LINUX-ILP32 %s
-// CC1-RV32-LINUX-ILP32:
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin/ld"
+// CC1-RV32-LINUX-ILP32:
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}ld"
// CC1-RV32-LINUX-ILP32:
"--sysroot={{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot"
// CC1-RV32-LINUX-ILP32: "-m" "elf32lriscv"
// CC1-RV32-LINUX-ILP32: "-dynamic-linker" "/lib/ld-linux-riscv32-ilp32.so.1"
@@ -25,7 +25,7 @@
// RUN: --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
// RUN: | FileCheck -check-prefix=CC1-RV32-LINUX-ILP32D %s
-// CC1-RV32-LINUX-ILP32D:
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin/ld"
+// CC1-RV32-LINUX-ILP32D:
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}ld"
// CC1-RV32-LINUX-ILP32D:
"--sysroot={{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot"
// CC1-RV32-LINUX-ILP32D: "-m" "elf32lriscv"
// CC1-RV32-LINUX-ILP32D: "-dynamic-linker" "/lib/ld-linux-riscv32-ilp32d.so.1"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r322293 - Make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
Author: vaartis
Date: Thu Jan 11 09:04:29 2018
New Revision: 322293
URL: http://llvm.org/viewvc/llvm-project?rev=322293&view=rev
Log:
Make std::experimental::filesystem::remove and remove_all return false or 0 if
the file doesn't exist
Differential Revision: https://reviews.llvm.org/D41830
Modified:
libcxx/trunk/src/experimental/filesystem/operations.cpp
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
Modified: libcxx/trunk/src/experimental/filesystem/operations.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/operations.cpp?rev=322293&r1=322292&r2=322293&view=diff
==
--- libcxx/trunk/src/experimental/filesystem/operations.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/operations.cpp Thu Jan 11 09:04:29
2018
@@ -661,8 +661,10 @@ path __read_symlink(const path& p, std::
bool __remove(const path& p, std::error_code *ec) {
if (ec) ec->clear();
+
if (::remove(p.c_str()) == -1) {
-set_or_throw(ec, "remove", p);
+if (errno != ENOENT)
+set_or_throw(ec, "remove", p);
return false;
}
return true;
@@ -692,13 +694,18 @@ std::uintmax_t remove_all_impl(path cons
} // end namespace
std::uintmax_t __remove_all(const path& p, std::error_code *ec) {
+if (ec) ec->clear();
+
std::error_code mec;
auto count = remove_all_impl(p, mec);
if (mec) {
-set_or_throw(mec, ec, "remove_all", p);
-return static_cast(-1);
+if (mec == errc::no_such_file_or_directory) {
+return 0;
+} else {
+set_or_throw(mec, ec, "remove_all", p);
+return static_cast(-1);
+}
}
-if (ec) ec->clear();
return count;
}
Modified:
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp?rev=322293&r1=322292&r2=322293&view=diff
==
---
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
(original)
+++
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove/remove.pass.cpp
Thu Jan 11 09:04:29 2018
@@ -61,17 +61,29 @@ TEST_CASE(test_error_reporting)
const path file_in_bad_dir = env.create_file(bad_perms_dir / "file", 42);
permissions(bad_perms_dir, perms::none);
const path testCases[] = {
-"",
-env.make_env_path("dne"),
non_empty_dir,
file_in_bad_dir,
};
for (auto& p : testCases) {
std::error_code ec;
+
TEST_CHECK(!fs::remove(p, ec));
TEST_CHECK(ec);
TEST_CHECK(checkThrow(p, ec));
}
+
+// PR#35780
+const path testCasesNonexistant[] = {
+"",
+env.make_env_path("dne")
+};
+
+for (auto& p : testCasesNonexistant) {
+std::error_code ec;
+
+TEST_CHECK(!fs::remove(p, ec));
+TEST_CHECK(!ec);
+}
}
TEST_CASE(basic_remove_test)
Modified:
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp?rev=322293&r1=322292&r2=322293&view=diff
==
---
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
(original)
+++
libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp
Thu Jan 11 09:04:29 2018
@@ -64,16 +64,28 @@ TEST_CASE(test_error_reporting)
permissions(bad_perms_file, perms::none);
const path testCases[] = {
-env.make_env_path("dne"),
file_in_bad_dir
};
const auto BadRet = static_cast(-1);
for (auto& p : testCases) {
std::error_code ec;
+
TEST_CHECK(fs::remove_all(p, ec) == BadRet);
TEST_CHECK(ec);
TEST_CHECK(checkThrow(p, ec));
}
+
+// PR#35780
+const path testCasesNonexistant[] = {
+"",
+env.make_env_path("dne")
+};
+for (auto &p : testCasesNonexistant) {
+std::error_code ec;
+
+TEST_CHECK(fs::remove_all(p) == 0);
+TEST_CHECK(!ec);
+}
}
TEST_CASE(basic_remove_all_test)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322294 - [Driver][RISCV] Another Windows file separator fix for riscv32-toolchain.c test
Author: asb
Date: Thu Jan 11 09:06:32 2018
New Revision: 322294
URL: http://llvm.org/viewvc/llvm-project?rev=322294&view=rev
Log:
[Driver][RISCV] Another Windows file separator fix for riscv32-toolchain.c test
I've checking the failure log, this _should_ be the last one. Sorry for not
spotting this additional case first time round.
Modified:
cfe/trunk/test/Driver/riscv32-toolchain.c
Modified: cfe/trunk/test/Driver/riscv32-toolchain.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/riscv32-toolchain.c?rev=322294&r1=322293&r2=322294&view=diff
==
--- cfe/trunk/test/Driver/riscv32-toolchain.c (original)
+++ cfe/trunk/test/Driver/riscv32-toolchain.c Thu Jan 11 09:06:32 2018
@@ -14,7 +14,7 @@
// CC1-RV32-LINUX-ILP32:
"--sysroot={{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot"
// CC1-RV32-LINUX-ILP32: "-m" "elf32lriscv"
// CC1-RV32-LINUX-ILP32: "-dynamic-linker" "/lib/ld-linux-riscv32-ilp32.so.1"
-// CC1-RV32-LINUX-ILP32:
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32/crtbegin.o"
+// CC1-RV32-LINUX-ILP32:
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32{{/|}}crtbegin.o"
// CC1-RV32-LINUX-ILP32:
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32"
// CC1-RV32-LINUX-ILP32:
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32"
// CC1-RV32-LINUX-ILP32:
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32"
@@ -29,7 +29,7 @@
// CC1-RV32-LINUX-ILP32D:
"--sysroot={{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot"
// CC1-RV32-LINUX-ILP32D: "-m" "elf32lriscv"
// CC1-RV32-LINUX-ILP32D: "-dynamic-linker" "/lib/ld-linux-riscv32-ilp32d.so.1"
-// CC1-RV32-LINUX-ILP32D:
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d/crtbegin.o"
+// CC1-RV32-LINUX-ILP32D:
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d{{/|}}crtbegin.o"
// CC1-RV32-LINUX-ILP32D:
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32d"
// CC1-RV32-LINUX-ILP32D:
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32d"
// CC1-RV32-LINUX-ILP32D:
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32d"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41800: [analyzer] Use a custom program point for the check::NewAllocator callback.
NoQ added inline comments. Comment at: test/Analysis/NewDelete-path-notes.cpp:44 // CHECK-NEXT: -// CHECK-NEXT:line6 +// CHECK-NEXT:line7 // CHECK-NEXT:col3 a.sidorin wrote: > NoQ wrote: > > a.sidorin wrote: > > > Not even a minor concern for this patch, but I think that placing `//RUN` > > > and `//CHECK` after the code being tested could save us from massive > > > changes of line numbers. > > Hmm, not sure if i understand, you mean //before// the code? (it would save > > us from line number changes in plists, but it'd make the tests harder to > > read because you'd have to scroll all the way down through the plist to > > find the actual code). > I mean placing RUNs after the program code (but before `// CHECK`. Anyway, > moving RUNs below will cause... line changes so it is not an important issue. Hmm, yeah, right, maybe. So there would be no line number issues until we stick a test in between two other tests or modify existing tests. But i'm still too used to having RUNs above everything, i guess, to just quickly figure out what does the test do. Also it's not hard to regenerate line numbers (even if it looks scary). https://reviews.llvm.org/D41800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value
aaron.ballman added inline comments.
Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:210
+ // test that the check is disabled inside GNU statement expressions
+ ({ std::async(increment, 42); });
+ auto StmtExprRetval = ({ std::async(increment, 42); });
Not diagnosing this case is questionable -- the return value *is* unused and
that's a bad thing. However, this is likely to be an uncommon code pattern, so
I'd be fine if you added a FIXME to the AST matcher code that excludes GNU
expression statements to handle this case someday, and add a FIXME comment here
as well so we know what we would like the behavior to be (you could fix the
FIXMEs in a follow-up patch if you'd like).
https://reviews.llvm.org/D41655
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41546: [clang-tidy] Adding Fuchsia checker for statically constructed objects
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Aside from a minor formatting nit that I missed, LGTM!
Comment at: clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp:51
+const MatchFinder::MatchResult &Result) {
+ if (const auto *D = Result.Nodes.getNodeAs("decl")) {
+diag(D->getLocStart(), "static objects are disallowed; if possible, use a "
You can elide the braces here.
https://reviews.llvm.org/D41546
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41933: handle scoped_lockable objects being returned by value in C++17
aaron.ballman added a comment.
I think this approach is a reasonable approximation, but @delesley has the
final word.
Comment at: lib/Analysis/ThreadSafety.cpp:1994
+static CXXConstructorDecl *findConstructorForByValueReturn(CXXRecordDecl *RD) {
+ // Prefer a move constructor over a copy constructor. If there's more than
Can you sprinkle some const correctness around this declaration?
Comment at: lib/Analysis/ThreadSafety.cpp:2001
+ for (CXXConstructorDecl *Ctor : RD->ctors()) {
+if (Ctor->isDeleted() || Ctor->getAccess() != AS_public)
+ continue;
Should we care about access checking here (protected ctor within a reasonable
context, or friendship)?
Repository:
rC Clang
https://reviews.llvm.org/D41933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41897: Fixing a crash in Sema.
aaron.ballman added inline comments. Comment at: SemaCXX/base-class-ambiguity-check.cpp:1 +// RUN: %clang_cc1 -fsyntax-only %s + aaron.ballman wrote: > This run line isn't testing anything. Since you're trying to ensure this > doesn't crash, I would put `-verify` on the RUN line and `// > expected-no-diagnostics` on the line below. This comment hasn't been applied yet. https://reviews.llvm.org/D41897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41815: [clang-tidy] implement check for goto
aaron.ballman added a comment.
In https://reviews.llvm.org/D41815#973260, @JonasToth wrote:
> I enhanced the check to do more:
>
> - check that jumps will only be forward. AFAIK that is required in all
> sensefull usecases of goto, is it?
> - additionally check for gotos in nested loops. These are not diagnosed if
> the jump is forward implementing the exception in the core guidelines.
>
> With these modifications the check can be used to enforce the rule in the
> CoreGuidelines and the goto part of `6.3.1 Ensure that the label(s) for a
> jump statement or a switch condition appear later, in the same or an
> enclosing block` for the HICPP module.
Nice!
> Some test cases for all combinations are missing, i can add those once you
> agree that the functionality change is indeed ok.
I think this is the correct direction for the check.
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:22
+ if (getLangOpts().CPlusPlus)
+Finder->addMatcher(gotoStmt().bind("goto"), this);
+}
JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > aaron.ballman wrote:
> > > > > Are you planning to add the exception listed in the C++ Core
> > > > > Guideline? It makes an explicit exception allowing you to jump
> > > > > forward out of a loop construct.
> > > > What should this check do with indirect goto statements (it's a GCC
> > > > extension we support where you can jump to an expression)?
> > > >
> > > > Regardless, there should be a test for indirect gotos and jump forward
> > > > out of loop constructs.
> > > > Are you planning to add the exception listed in the C++ Core Guideline?
> > > > It makes an explicit exception allowing you to jump forward out of a
> > > > loop construct.
> > >
> > > I do plan for this. Because I dont have any experience with gotos I
> > > wanted to do it in small steps.
> > >
> > > > What should this check do with indirect goto statements (it's a GCC
> > > > extension we support where you can jump to an expression)?
> > > Iam not aware of these :)
> > >> What should this check do with indirect goto statements (it's a GCC
> > >> extension we support where you can jump to an expression)?
> > >
> > > Iam not aware of these :)
> >
> > https://gcc.gnu.org/onlinedocs/gcc/Labels-as-Values.html
> > (and a good reference on why these are interesting:
> > https://eli.thegreenplace.net/2012/07/12/computed-goto-for-efficient-dispatch-tables)
> I would think that this is a special feature that will only be used if you
> know what you are doing. So it should be allowed with configuration. I am not
> sure about the default though. For now it is ignored.
>
> HICPP has a rule on gotos as well, which states that only forward jumps are
> allowed.
>
> I think that these different approaches to `goto` should land here sometime
> as different configurations.
> I would think that this is a special feature that will only be used if you
> know what you are doing. So it should be allowed with configuration. I am not
> sure about the default though. For now it is ignored.
Ignoring it for now seems reasonable to me. You should add a TODO comment for
it so we don't lose track of it, though.
> HICPP has a rule on gotos as well, which states that only forward jumps are
> allowed.
That's essentially the same exception as the C++ Core Guidelines, which is nice.
> I think that these different approaches to goto should land here sometime as
> different configurations.
Agreed, if needed.
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:21
+AST_MATCHER(GotoStmt, isForwardJumping) {
+
+ return Node.getLocStart() < Node.getLabel()->getLocStart();
Can remove the spurious newline.
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:26
+void AvoidGotoCheck::registerMatchers(MatchFinder *Finder) {
+ // Check if the 'goto' is used for control flow other then jumping
+ // out of a nested loop.
s/then/than
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:41-53
+ if (const auto *BackGoto =
+ Result.Nodes.getNodeAs("backward-goto")) {
+diag(BackGoto->getGotoLoc(), "do not jump backwards with 'goto'")
+<< BackGoto->getSourceRange();
+diag(BackGoto->getLabel()->getLocStart(), "label defined here",
+ DiagnosticIDs::Note);
+ }
Is there a reason to have two separate diagnostics? It seems like these both
would be covered by "this use of 'goto' for flow control is prohibited".
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy
george.karpenkov added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:522
+ ArraySize = Array->getSize().getLimitedValue();
+ if (const auto *String = dyn_cast(Source)) {
+StrLen = String->getLength();
Nesting this if- inside the previous one would simplify the outer scope and let
you assign to `ArraySize` at declaration size.
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:526
+ }
+ if (StrLenFound && ArraySize >= StrLen + 1)
+return;
Why not put this if-expression into the one above where `StrLen` is found?
That would eliminate `StrLenFound` and remove the potential error surface of
uninitialized read from `StrLen` (the declaration for which should probably be
inside this block as well)
Comment at: test/Analysis/security-syntax-checks.m:151
+ char x[5];
+ strcpy(x, "abcd");
+}
I think it would make sense to add another test case where the warning is
expected, even though string length and array size are known at compile time.
https://reviews.llvm.org/D41384
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r322306 - Implement an _is_allocator type trait for use in deduction guides.
Author: marshall
Date: Thu Jan 11 11:36:22 2018
New Revision: 322306
URL: http://llvm.org/viewvc/llvm-project?rev=322306&view=rev
Log:
Implement an _is_allocator type trait for use in deduction guides.
Added:
libcxx/trunk/test/libcxx/memory/
libcxx/trunk/test/libcxx/memory/is_allocator.pass.cpp
Modified:
libcxx/trunk/include/memory
Modified: libcxx/trunk/include/memory
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/memory?rev=322306&r1=322305&r2=322306&view=diff
==
--- libcxx/trunk/include/memory (original)
+++ libcxx/trunk/include/memory Thu Jan 11 11:36:22 2018
@@ -5597,6 +5597,16 @@ struct __temp_value {
};
#endif
+#if _LIBCPP_STD_VER > 14
+template
+struct __is_allocator : false_type {};
+
+template
+struct __is_allocator<_Alloc,
+ void_t().allocate(size_t{}))>>
+ : true_type {};
+#endif
+
_LIBCPP_END_NAMESPACE_STD
_LIBCPP_POP_MACROS
Added: libcxx/trunk/test/libcxx/memory/is_allocator.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/memory/is_allocator.pass.cpp?rev=322306&view=auto
==
--- libcxx/trunk/test/libcxx/memory/is_allocator.pass.cpp (added)
+++ libcxx/trunk/test/libcxx/memory/is_allocator.pass.cpp Thu Jan 11 11:36:22
2018
@@ -0,0 +1,41 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+//
+
+//
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+// template
+// struct __is_allocator;
+
+// Is either true_type or false_type depending on if A is an allocator.
+
+#include
+#include
+
+#include "test_macros.h"
+#include "min_allocator.h"
+#include "test_allocator.h"
+
+template
+void test_allocators()
+{
+ static_assert( std::__is_allocator>::value, "" );
+ static_assert( std::__is_allocator>::value, "" );
+ static_assert( std::__is_allocator>::value, "" );
+}
+
+
+int main()
+{
+// test_allocators();
+ test_allocators();
+ test_allocators();
+ test_allocators();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations
lebedev.ri added a comment. In https://reviews.llvm.org/D41455#963752, @aaron.ballman wrote: > Aside from a documentation nit, this LGTM. However, you should wait to commit > until after you can safely regenerate the AST matcher documentation. The > issue there is that dump_ast_matchers.py was not updated after r318304. I > believe @dblaikie was looking into the Python script, but I'm not certain how > far he got with it. @dblaikie Hi! Did you get anywhere with the script update? As far i'm concerned, all new ASTMatchers are stalled because of this... Repository: rC Clang https://reviews.llvm.org/D41455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41815: [clang-tidy] implement check for goto
lebedev.ri added inline comments.
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:20
+
+AST_MATCHER(GotoStmt, isForwardJumping) {
+
It would be nice to have it in standard ASTMatchers, i believe it will be
useful for `else-after-return` check.
Though the ASTMatchers are stuck due to the doc-dumping script being 'broken'
(see D41455)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26
efriedma added a comment. > as this patch is committed clang is broken (cannot use glibc headers) This patch was supposed to *fix* compatibility with the glibc headers. If it doesn't, we should clearly revert it... but we need to figure out what we need to do to be compatible first. From what I can see on this thread, we *must* define _Float128 on x86-64 Linux, and we *must not* define _Float128 for any other glibc target. Is that correct? Or does it depend on the glibc version? (We have a bit of time before the 6.0 release, so we can adjust the behavior here to make it work. We probably don't want to try to add full _Float128 support on the branch, though.) Repository: rC Clang https://reviews.llvm.org/D40673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41958: Create a deduction guide for basic_string
mclow.lists created this revision.
mclow.lists added a reviewer: EricWF.
First of the C++17 deduction guides (I think).
Uses the new `__is_allocator` trait.
The failing test is not quite right yet, but the success bits all work.
https://reviews.llvm.org/D41958
Files:
include/string
test/std/strings/basic.string/string.cons/iter_alloc.fail.cpp
test/std/strings/basic.string/string.cons/iter_alloc.pass.cpp
Index: test/std/strings/basic.string/string.cons/iter_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/iter_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/iter_alloc.pass.cpp
@@ -13,6 +13,18 @@
// basic_string(InputIterator begin, InputIterator end,
// const Allocator& a = Allocator());
+// template::value_type>>
+// basic_string(InputIterator, InputIterator, Allocator = Allocator())
+//-> basic_string::value_type,
+// char_traits::value_type>,
+// Allocator>;
+//
+// The deduction guide shall not participate in overload resolution if InputIterator
+// is a type that does not qualify as an input iterator, or if Allocator is a type
+// that does not qualify as an allocator.
+
+
#include
#include
#include
@@ -116,4 +128,48 @@
test(input_iterator(s), input_iterator(s+50), A());
}
#endif
+
+// Test deduction guides
+#if TEST_STD_VER > 14
+{
+const char* s = "12345678901234";
+std::basic_string s1{s, s+10, std::allocator{}};
+using S = decltype(s1); // what type did we get?
+static_assert(std::is_same_v, "");
+static_assert(std::is_same_v>, "");
+static_assert(std::is_same_v>, "");
+assert(s1.size() == 10);
+assert(s1.compare(0, s1.size(), s, s1.size()) == 0);
+}
+{
+const wchar_t* s = L"12345678901234";
+std::basic_string s1{s, s+10, test_allocator{}};
+using S = decltype(s1); // what type did we get?
+static_assert(std::is_same_v, "");
+static_assert(std::is_same_v>, "");
+static_assert(std::is_same_v>, "");
+assert(s1.size() == 10);
+assert(s1.compare(0, s1.size(), s, s1.size()) == 0);
+}
+{
+const char16_t* s = u"12345678901234";
+std::basic_string s1{s, s+10, min_allocator{}};
+using S = decltype(s1); // what type did we get?
+static_assert(std::is_same_v, "");
+static_assert(std::is_same_v>, "");
+static_assert(std::is_same_v>, "");
+assert(s1.size() == 10);
+assert(s1.compare(0, s1.size(), s, s1.size()) == 0);
+}
+{
+const char32_t* s = U"12345678901234";
+std::basic_string s1{s, s+10, explicit_allocator{}};
+using S = decltype(s1); // what type did we get?
+static_assert(std::is_same_v, "");
+static_assert(std::is_same_v>, "");
+static_assert(std::is_same_v>, "");
+assert(s1.size() == 10);
+assert(s1.compare(0, s1.size(), s, s1.size()) == 0);
+}
+#endif
}
Index: test/std/strings/basic.string/string.cons/iter_alloc.fail.cpp
===
--- test/std/strings/basic.string/string.cons/iter_alloc.fail.cpp
+++ test/std/strings/basic.string/string.cons/iter_alloc.fail.cpp
@@ -0,0 +1,53 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+//
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+// template::value_type>>
+// basic_string(InputIterator, InputIterator, Allocator = Allocator())
+//-> basic_string::value_type,
+// char_traits::value_type>,
+// Allocator>;
+//
+// The deduction guide shall not participate in overload resolution if InputIterator
+// is a type that does not qualify as an input iterator, or if Allocator is a type
+// that does not qualify as an allocator.
+
+
+#include
+#include
+#include
+#include
+
+#include "test_macros.h"
+
+class NotAnItertor {};
+
+template
+class NotAnAllocator {};
+
+int main()
+{
+{ // Not an iterator at all
+std::basic_string s1{NotAnItertor{}, NotAnItertor{}, std::allocator{}}; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of 'basic_string'}}
+}
+{ // Not an input iterator
+const char16_t* s = u"12345678901234";
+std::basic_string s0;
+std::basic_string s1{std::back_insert_iterator(s0),
+ std::back_insert_iterator(s0),
+ std::allocator{}}; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of 'basic_string'}}
+}
+{ // Not an allocator
+const wchar_t* s = L"12345678901234";
+std::basic_string s1{s, s+10, NotAnAllocator{}}; // expected-error {{no viable constructor or ded
Re: [PATCH] D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations
I haven't as yet, no. Sorry - happy if someone else wants to have a go, or I'll take a closer look soon. - Dave On Thu, Jan 11, 2018 at 11:38 AM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri added a comment. > > In https://reviews.llvm.org/D41455#963752, @aaron.ballman wrote: > > > Aside from a documentation nit, this LGTM. However, you should wait to > commit until after you can safely regenerate the AST matcher documentation. > The issue there is that dump_ast_matchers.py was not updated after r318304. > I believe @dblaikie was looking into the Python script, but I'm not certain > how far he got with it. > > > @dblaikie Hi! Did you get anywhere with the script update? > As far i'm concerned, all new ASTMatchers are stalled because of this... > > > Repository: > rC Clang > > https://reviews.llvm.org/D41455 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
vsapsai added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2012-2015 +// Skip escaped characters. Escaped newlines will already be processed by +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); rsmith wrote: > You can just delete these four lines entirely. It will make Clang reject previously accepted `#include escape.h>` That's what we want, right? I agree with having no support for such file names, just want to confirm. For the reference, proposed change would match gcc 5.4.0 behaviour. gcc 6.1 and higher rejects such include too but in a different way. https://reviews.llvm.org/D41423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41933: handle scoped_lockable objects being returned by value in C++17
rsmith updated this revision to Diff 129496.
Repository:
rC Clang
https://reviews.llvm.org/D41933
Files:
lib/Analysis/ThreadSafety.cpp
test/SemaCXX/warn-thread-safety-analysis.cpp
Index: test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- test/SemaCXX/warn-thread-safety-analysis.cpp
+++ test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=1 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wthread-safety -Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions -DUSE_ASSERT_CAPABILITY=0 %s
// FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s
// FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s
@@ -5248,3 +5249,28 @@
C c;
void f() { c[A()]->g(); }
} // namespace PR34800
+
+namespace ReturnScopedLockable {
+ template class SCOPED_LOCKABLE ReadLockedPtr {
+ public:
+ReadLockedPtr(Object *ptr) SHARED_LOCK_FUNCTION((*this)->mutex);
+ReadLockedPtr(ReadLockedPtr &&) SHARED_LOCK_FUNCTION((*this)->mutex);
+~ReadLockedPtr() UNLOCK_FUNCTION();
+
+Object *operator->() const { return object; }
+
+ private:
+Object *object;
+ };
+
+ struct Object {
+int f() SHARED_LOCKS_REQUIRED(mutex);
+Mutex mutex;
+ };
+
+ ReadLockedPtr get();
+ int use() {
+auto ptr = get();
+return ptr->f();
+ }
+}
Index: lib/Analysis/ThreadSafety.cpp
===
--- lib/Analysis/ThreadSafety.cpp
+++ lib/Analysis/ThreadSafety.cpp
@@ -1689,7 +1689,7 @@
CapExprSet ScopedExclusiveReqs, ScopedSharedReqs;
StringRef CapDiagKind = "mutex";
- // Figure out if we're calling the constructor of scoped lockable class
+ // Figure out if we're constructing an object of scoped lockable class
bool isScopedVar = false;
if (VD) {
if (const CXXConstructorDecl *CD = dyn_cast(D)) {
@@ -1991,22 +1991,68 @@
// FIXME -- only handles constructors in DeclStmt below.
}
+static CXXConstructorDecl *
+findConstructorForByValueReturn(const CXXRecordDecl *RD) {
+ // Prefer a move constructor over a copy constructor. If there's more than
+ // one copy constructor or more than one move constructor, we arbitrarily
+ // pick the first declared such constructor rather than trying to guess which
+ // one is more appropriate.
+ CXXConstructorDecl *CopyCtor = nullptr;
+ for (CXXConstructorDecl *Ctor : RD->ctors()) {
+if (Ctor->isDeleted())
+ continue;
+if (Ctor->isMoveConstructor())
+ return Ctor;
+if (!CopyCtor && Ctor->isCopyConstructor())
+ CopyCtor = Ctor;
+ }
+ return CopyCtor;
+}
+
+static Expr *buildFakeCtorCall(CXXConstructorDecl *CD, ArrayRef Args,
+ SourceLocation Loc) {
+ ASTContext &Ctx = CD->getASTContext();
+ return CXXConstructExpr::Create(Ctx, Ctx.getRecordType(CD->getParent()), Loc,
+ CD, true, Args, false, false, false, false,
+ CXXConstructExpr::CK_Complete,
+ SourceRange(Loc, Loc));
+}
+
void BuildLockset::VisitDeclStmt(DeclStmt *S) {
// adjust the context
LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx);
for (auto *D : S->getDeclGroup()) {
if (VarDecl *VD = dyn_cast_or_null(D)) {
Expr *E = VD->getInit();
+ if (!E)
+continue;
+ E = E->IgnoreParens();
+
// handle constructors that involve temporaries
- if (ExprWithCleanups *EWC = dyn_cast_or_null(E))
+ if (auto *EWC = dyn_cast(E))
E = EWC->getSubExpr();
+ if (auto *BTE = dyn_cast(E))
+E = BTE->getSubExpr();
- if (CXXConstructExpr *CE = dyn_cast_or_null(E)) {
+ if (CXXConstructExpr *CE = dyn_cast(E)) {
NamedDecl *CtorD = dyn_cast_or_null(CE->getConstructor());
if (!CtorD || !CtorD->hasAttrs())
- return;
-handleCall(CE, CtorD, VD);
+ continue;
+handleCall(E, CtorD, VD);
+ } else if (isa(E) && E->isRValue()) {
+// If the object is initialized by a function call that returns a
+// scoped lockable by value, use the attributes on the copy or move
+// constructor to figure out what effect that should have on the
+// lockset.
+// FIXME: Is this really the best way to handle this situation?
+auto *RD = E->getType()->getAsCXXRecordDecl();
+if (!RD || !RD->hasAttr())
+ continue;
+CXXConstructorDecl *CtorD = findConstructorForByValueReturn(RD);
+if (!CtorD || !CtorD->hasAttrs())
+ continue;
[PATCH] D41933: handle scoped_lockable objects being returned by value in C++17
rsmith added inline comments.
Comment at: lib/Analysis/ThreadSafety.cpp:1994
+static CXXConstructorDecl *findConstructorForByValueReturn(CXXRecordDecl *RD) {
+ // Prefer a move constructor over a copy constructor. If there's more than
aaron.ballman wrote:
> Can you sprinkle some const correctness around this declaration?
I can't make the `CXXConstructorDecl` const without adding a `const_cast`
somewhere or making very large-scale changes to Clang :) The `CXXConstructExpr`
constructor needs a pointer to a non-const `CXXConstructorDecl`.
I can make the `CXXRecordDecl` const. I think that's actually a
const-correctness bug -- if `const` on an AST node means anything, it should
mean that the AST is potentially immutable, and you shouldn't be able to get
from a const AST node to a non-const one -- but it's probably closer to the
ideal than no const, so I'll add a const there.
Comment at: lib/Analysis/ThreadSafety.cpp:2001
+ for (CXXConstructorDecl *Ctor : RD->ctors()) {
+if (Ctor->isDeleted() || Ctor->getAccess() != AS_public)
+ continue;
aaron.ballman wrote:
> Should we care about access checking here (protected ctor within a reasonable
> context, or friendship)?
I'm not completely sure. My thinking was that if someone uses the "private copy
constructor with no definition" approach rather than the "deleted copy
constructor" approach, we should ignore that copy constructor. But this would
only really matter if they do that to their *move* constructor and they have an
accessible copy constructor, which seems like a sufficiently bizarre situation
to not have a special rule for it. (Well, it'd also matter if the private copy
ctor with no definition had thread safety attributes, which also seems like a
very strange case.)
Happy to go either way here. On balance I think I agree that it's more
consistent with the general language rules to ignore access here.
Repository:
rC Clang
https://reviews.llvm.org/D41933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance
juliehockett updated this revision to Diff 129498.
juliehockett marked 8 inline comments as done.
juliehockett added a comment.
1. Updating check and tests to address virtual inheritance
2. Rebasing from trunk
https://reviews.llvm.org/D40580
Files:
clang-tidy/fuchsia/CMakeLists.txt
clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
clang-tidy/fuchsia/MultipleInheritanceCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/fuchsia-multiple-inheritance.cpp
Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -0,0 +1,131 @@
+// RUN: %check_clang_tidy %s fuchsia-multiple-inheritance %t
+
+class Base_A {
+public:
+ virtual int foo() { return 0; }
+};
+
+class Base_B {
+public:
+ virtual int bar() { return 0; }
+};
+
+class Base_A_child : public Base_A {
+public:
+ virtual int baz() { return 0; }
+};
+
+class Interface_A {
+public:
+ virtual int foo() = 0;
+};
+
+class Interface_B {
+public:
+ virtual int bar() = 0;
+};
+
+class Interface_C {
+public:
+ virtual int blat() = 0;
+};
+
+class Interface_A_with_member {
+public:
+ virtual int foo() = 0;
+ int val = 0;
+};
+
+class Interface_with_A_Parent : public Base_A {
+public:
+ virtual int baz() = 0;
+};
+
+// Inherits from multiple concrete classes.
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child1 : public Base_A, Base_B {};
+class Bad_Child1 : public Base_A, Base_B {};
+
+// CHECK-MESSAGES: [[@LINE+1]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+class Bad_Child2 : public Base_A, Interface_A_with_member {
+ virtual int foo() override { return 0; }
+};
+
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+class Bad_Child3 : public Interface_with_A_Parent, Base_B {
+ virtual int baz() override { return 0; }
+};
+
+// Easy cases of single inheritance
+class Simple_Child1 : public Base_A {};
+class Simple_Child2 : public Interface_A {
+ virtual int foo() override { return 0; }
+};
+
+// Valid uses of multiple inheritance
+class Good_Child1 : public Interface_A, Interface_B {
+ virtual int foo() override { return 0; }
+ virtual int bar() override { return 0; }
+};
+
+class Good_Child2 : public Base_A, Interface_B {
+ virtual int bar() override { return 0; }
+};
+
+class Good_Child3 : public Base_A_child, Interface_C, Interface_B {
+ virtual int bar() override { return 0; }
+ virtual int blat() override { return 0; }
+};
+
+struct B1 { int x; };
+struct B2 { int x;};
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: struct D : B1, B2 {};
+struct D1 : B1, B2 {};
+
+struct Base1 { virtual void foo() = 0; };
+struct V1 : virtual Base1 {};
+struct V2 : virtual Base1 {};
+struct D2 : V1, V2 {};
+
+struct Base2 { virtual void foo(); };
+struct V3 : virtual Base2 {};
+struct V4 : virtual Base2 {};
+struct D3 : V3, V4 {};
+
+struct Base3 {};
+struct V5 : virtual Base3 { virtual void f(); };
+struct V6 : virtual Base3 { virtual void g(); };
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: struct D4 : V5, V6 {};
+struct D4 : V5, V6 {};
+
+struct Base4 {};
+struct V7 : virtual Base4 { virtual void f() = 0; };
+struct V8 : virtual Base4 { virtual void g() = 0; };
+struct D5 : V7, V8 {};
+
+struct Base5 { virtual void f() = 0; };
+struct V9 : virtual Base5 { virtual void f(); };
+struct V10 : virtual Base5 { virtual void g() = 0; };
+struct D6 : V9, V10 {};
+
+struct Base6 { virtual void f(); };
+struct Base7 { virtual void g(); };
+struct V15 : virtual Base6 { virtual void f() = 0; };
+struct V16 : virtual Base7 { virtual void g() = 0; };
+// CHECK-MESSAGES: [[@LINE+2]]:1: warning: inheriting mulitple classes which aren't pure virtual is discouraged [fuchsia-multiple-inheritance]
+// CHECK-NEXT: struct D9 : V15, V16 {};
+struct D9 : V15, V16 {};
+
+struct Static_Base { static void foo(); };
+struct V11 : virtual Static_Base {};
+struct V12 : virtual Static_Base {};
+struct D7 : V11, V12 {};
+
+struct Static_Base_2 {};
+struct V13 : virtual Static_Base_2 { static void f(); };
+struct V14 : virtual Static_Base_2 { static void g(); };
+struct D8 : V13, V14 {};
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/cla
[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance
rsmith added inline comments.
Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:60
+ // To be an interface, all base classes must be interfaces as well.
+ for (const auto &I : Node->bases()) {
+const auto *Ty = I.getType()->getAs();
aaron.ballman wrote:
> juliehockett wrote:
> > aaron.ballman wrote:
> > > What about virtual bases (`Node->vbases()`)? This would also be worth
> > > some test cases.
> > Added test cases for virtual, but aren't virtual bases also included in
> > `bases()`?
> No, they are separate in `CXXRecordDecl`.
That's not quite right. `bases()` contains all direct bases, regardless of
whether or not they're virtual. `vbases()` contains all virtual bases,
regardless of whether or not they're direct.
https://reviews.llvm.org/D40580
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41933: handle scoped_lockable objects being returned by value in C++17
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM, but you should wait for a bit to see if DeLesley has concerns.
Comment at: lib/Analysis/ThreadSafety.cpp:1994
+static CXXConstructorDecl *findConstructorForByValueReturn(CXXRecordDecl *RD) {
+ // Prefer a move constructor over a copy constructor. If there's more than
rsmith wrote:
> aaron.ballman wrote:
> > Can you sprinkle some const correctness around this declaration?
> I can't make the `CXXConstructorDecl` const without adding a `const_cast`
> somewhere or making very large-scale changes to Clang :) The
> `CXXConstructExpr` constructor needs a pointer to a non-const
> `CXXConstructorDecl`.
>
> I can make the `CXXRecordDecl` const. I think that's actually a
> const-correctness bug -- if `const` on an AST node means anything, it should
> mean that the AST is potentially immutable, and you shouldn't be able to get
> from a const AST node to a non-const one -- but it's probably closer to the
> ideal than no const, so I'll add a const there.
Thanks! I agree with you that `const` on an AST node should mean that the AST
is immutable, but I think getting to that point would be a pretty large change
to Clang in general, unfortunately.
Comment at: lib/Analysis/ThreadSafety.cpp:2001
+ for (CXXConstructorDecl *Ctor : RD->ctors()) {
+if (Ctor->isDeleted() || Ctor->getAccess() != AS_public)
+ continue;
rsmith wrote:
> aaron.ballman wrote:
> > Should we care about access checking here (protected ctor within a
> > reasonable context, or friendship)?
> I'm not completely sure. My thinking was that if someone uses the "private
> copy constructor with no definition" approach rather than the "deleted copy
> constructor" approach, we should ignore that copy constructor. But this would
> only really matter if they do that to their *move* constructor and they have
> an accessible copy constructor, which seems like a sufficiently bizarre
> situation to not have a special rule for it. (Well, it'd also matter if the
> private copy ctor with no definition had thread safety attributes, which also
> seems like a very strange case.)
>
> Happy to go either way here. On balance I think I agree that it's more
> consistent with the general language rules to ignore access here.
I think ignoring access is a good first approximation. We can refine later if
we find the analysis requires it.
Repository:
rC Clang
https://reviews.llvm.org/D41933
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance
aaron.ballman added inline comments.
Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:60
+ // To be an interface, all base classes must be interfaces as well.
+ for (const auto &I : Node->bases()) {
+const auto *Ty = I.getType()->getAs();
rsmith wrote:
> aaron.ballman wrote:
> > juliehockett wrote:
> > > aaron.ballman wrote:
> > > > What about virtual bases (`Node->vbases()`)? This would also be worth
> > > > some test cases.
> > > Added test cases for virtual, but aren't virtual bases also included in
> > > `bases()`?
> > No, they are separate in `CXXRecordDecl`.
> That's not quite right. `bases()` contains all direct bases, regardless of
> whether or not they're virtual. `vbases()` contains all virtual bases,
> regardless of whether or not they're direct.
Ah, I must have mis-remembered these APIs. Thanks, @rsmith and sorry for the
noise @juliehockett!
https://reviews.llvm.org/D40580
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r322310 - [clang-tidy] Adding Fuchsia checker for statically constructed objects
Author: juliehockett
Date: Thu Jan 11 13:17:43 2018
New Revision: 322310
URL: http://llvm.org/viewvc/llvm-project?rev=322310&view=rev
Log:
[clang-tidy] Adding Fuchsia checker for statically constructed objects
Adds a check to the Fuchsia module to warn if statically-stored objects
are created, unless constructed with `constexpr`.
See https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md for
reference.
Differential Revision: https://reviews.llvm.org/D41546
Added:
clang-tools-extra/trunk/clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp
clang-tools-extra/trunk/clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/fuchsia-statically-constructed-objects.rst
clang-tools-extra/trunk/test/clang-tidy/fuchsia-statically-constructed-objects.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt?rev=322310&r1=322309&r2=322310&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/CMakeLists.txt Thu Jan 11
13:17:43 2018
@@ -4,6 +4,7 @@ add_clang_library(clangTidyFuchsiaModule
DefaultArgumentsCheck.cpp
FuchsiaTidyModule.cpp
OverloadedOperatorCheck.cpp
+ StaticallyConstructedObjectsCheck.cpp
VirtualInheritanceCheck.cpp
LINK_LIBS
Modified: clang-tools-extra/trunk/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/fuchsia/FuchsiaTidyModule.cpp?rev=322310&r1=322309&r2=322310&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/fuchsia/FuchsiaTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/FuchsiaTidyModule.cpp Thu Jan 11
13:17:43 2018
@@ -12,6 +12,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "DefaultArgumentsCheck.h"
#include "OverloadedOperatorCheck.h"
+#include "StaticallyConstructedObjectsCheck.h"
#include "VirtualInheritanceCheck.h"
using namespace clang::ast_matchers;
@@ -28,6 +29,8 @@ public:
"fuchsia-default-arguments");
CheckFactories.registerCheck(
"fuchsia-overloaded-operator");
+CheckFactories.registerCheck(
+"fuchsia-statically-constructed-objects");
CheckFactories.registerCheck(
"fuchsia-virtual-inheritance");
}
Added:
clang-tools-extra/trunk/clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp?rev=322310&view=auto
==
---
clang-tools-extra/trunk/clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp
(added)
+++
clang-tools-extra/trunk/clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp
Thu Jan 11 13:17:43 2018
@@ -0,0 +1,58 @@
+//===--- StaticallyConstructedObjectsCheck.cpp -
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "StaticallyConstructedObjectsCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+AST_MATCHER(Expr, isConstantInitializer) {
+ return Node.isConstantInitializer(Finder->getASTContext(), false);
+}
+
+AST_MATCHER(VarDecl, isGlobalStatic) {
+ return Node.getStorageDuration() == SD_Static && !Node.isLocalVarDecl();
+}
+
+void StaticallyConstructedObjectsCheck::registerMatchers(MatchFinder *Finder) {
+ // Constructing global, non-trivial objects with static storage is
+ // disallowed, unless the object is statically initialized with a constexpr
+ // constructor or has no explicit constructor.
+
+ // Constexpr requires C++11 or later.
+ if (!getLangOpts().CPlusPlus11)
+return;
+
+ Finder->addMatcher(
+ varDecl(allOf(
+ // Match global, statically stored objects...
+ isGlobalStatic(),
+ // ... that have C++ constructors...
+ hasDescendant(cxxConstructExpr(unless(allOf(
+ // ... unless it is constexpr ...
+ hasDeclaration(cxxConstructorDecl(isConstexpr())),
+ // ... and is statically initialized.
+ isConstantInitializer()))
+ .bind("decl
[PATCH] D41546: [clang-tidy] Adding Fuchsia checker for statically constructed objects
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE322310: [clang-tidy] Adding Fuchsia checker for statically
constructed objects (authored by juliehockett, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41546?vs=129208&id=129505#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41546
Files:
clang-tidy/fuchsia/CMakeLists.txt
clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.cpp
clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/fuchsia-statically-constructed-objects.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/fuchsia-statically-constructed-objects.cpp
Index: test/clang-tidy/fuchsia-statically-constructed-objects.cpp
===
--- test/clang-tidy/fuchsia-statically-constructed-objects.cpp
+++ test/clang-tidy/fuchsia-statically-constructed-objects.cpp
@@ -0,0 +1,91 @@
+// RUN: %check_clang_tidy %s fuchsia-statically-constructed-objects %t
+
+// Trivial static is fine
+static int i;
+
+class ClassWithNoCtor {};
+
+class ClassWithCtor {
+public:
+ ClassWithCtor(int Val) : Val(Val) {}
+private:
+ int Val;
+};
+
+class ClassWithConstexpr {
+public:
+ ClassWithConstexpr(int Val1, int Val2) : Val(Val1) {}
+ constexpr ClassWithConstexpr(int Val) : Val(Val) {}
+
+private:
+ int Val;
+};
+
+ClassWithNoCtor A;
+ClassWithConstexpr C(0);
+ClassWithConstexpr E(0, 1);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: static objects are disallowed; if possible, use a constexpr constructor instead [fuchsia-statically-constructed-objects]
+// CHECK-MESSAGES-NEXT: ClassWithConstexpr E(0, 1);
+ClassWithCtor G(0);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: static objects are disallowed; if possible, use a constexpr constructor instead [fuchsia-statically-constructed-objects]
+// CHECK-MESSAGES-NEXT: ClassWithCtor G(0);
+
+static ClassWithNoCtor A2;
+static ClassWithConstexpr C2(0);
+static ClassWithConstexpr E2(0, 1);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: static objects are disallowed; if possible, use a constexpr constructor instead [fuchsia-statically-constructed-objects]
+// CHECK-MESSAGES-NEXT: static ClassWithConstexpr E2(0, 1);
+static ClassWithCtor G2(0);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: static objects are disallowed; if possible, use a constexpr constructor instead [fuchsia-statically-constructed-objects]
+// CHECK-MESSAGES-NEXT: static ClassWithCtor G2(0);
+
+struct StructWithConstexpr { constexpr StructWithConstexpr(int Val) {} };
+struct StructWithNoCtor {};
+struct StructWithCtor { StructWithCtor(); };
+
+StructWithNoCtor SNoCtor;
+StructWithConstexpr SConstexpr(0);
+StructWithCtor SCtor;
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: static objects are disallowed; if possible, use a constexpr constructor instead [fuchsia-statically-constructed-objects]
+// CHECK-MESSAGES-NEXT: StructWithCtor SCtor;
+
+static StructWithConstexpr SConstexpr2(0);
+static StructWithNoCtor SNoCtor2;
+static StructWithCtor SCtor2;
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: static objects are disallowed; if possible, use a constexpr constructor instead [fuchsia-statically-constructed-objects]
+// CHECK-MESSAGES-NEXT: static StructWithCtor SCtor2;
+
+extern StructWithCtor SCtor3;
+
+class ClassWithStaticMember {
+private:
+ static StructWithNoCtor S;
+};
+
+ClassWithStaticMember Z();
+
+class S {
+ int Val;
+public:
+ constexpr S(int i) : Val(100 / i) {}
+ int getVal() const { return Val; }
+};
+
+static S s1(1);
+static S s2(0);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: static objects are disallowed; if possible, use a constexpr constructor instead [fuchsia-statically-constructed-objects]
+// CHECK-MESSAGES-NEXT: static S s2(0);
+
+extern int get_i();
+static S s3(get_i());
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: static objects are disallowed; if possible, use a constexpr constructor instead [fuchsia-statically-constructed-objects]
+// CHECK-MESSAGES-NEXT: static S s3(get_i());
+
+void f() {
+ // Locally static is fine
+ static int i;
+ static ClassWithNoCtor A2;
+ static ClassWithConstexpr C2(0);
+ static ClassWithConstexpr E2(0, 1);
+ static ClassWithCtor G2(0);
+}
Index: clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.h
===
--- clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.h
+++ clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.h
@@ -0,0 +1,37 @@
+//===--- StaticallyConstructedObjectsCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_STATICALLY_CONSTRUCT
[PATCH] D41423: [Lex] Avoid out-of-bounds dereference in LexAngledStringLiteral.
rsmith accepted this revision. rsmith added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:2012-2015 +// Skip escaped characters. Escaped newlines will already be processed by +// getAndAdvanceChar. +if (C == '\\') + C = getAndAdvanceChar(CurPtr, Result); vsapsai wrote: > rsmith wrote: > > You can just delete these four lines entirely. > It will make Clang reject previously accepted `#include escape.h>` > That's what we want, right? I agree with having no support for such file > names, just want to confirm. For the reference, proposed change would match > gcc 5.4.0 behaviour. gcc 6.1 and higher rejects such include too but in a > different way. Hmm. I thought so, but on further inspection this area is more subtle than I'd anticipated. It's *permissible* to reject that. In C++, [lex.header]p2 says: "The appearance of [...] the character[...] `\` [...] in a //q-char-sequence// or an //h-char-sequence// is conditionally-supported with implementation-defined semantics". C11 6.4.7/3 has similar wording but makes the behavior undefined in that case. (Both GCC's behavior and Clang's behavior are permitted by these rules.) Clang's handling of the `#include "q-char-sequence"` form behaves the same as its handling of the `#include ` form today: the header name is *not* terminated by a `"` or `>` (respectively) that is preceded by an odd number of `\`s. Retaining this consistency of behavior seems like a good idea. However, the actual behavior when such a character is "escaped" does not seem reasonable: ``` #include <\>> // includes file named \>, not file named > #include "\"" // includes file named \", not file named " ``` ... and there is no general way to perform a double-quoted include of a file whose name contains a `"` (not preceded by `\`), or an angled include of a file whose name is `>` (not preceded by `>`). So it seems that permitting escaped ending characters without actually handling them as escape sequences is not particularly worthwhile. I think the best approach here is to follow GCC's lead: terminate a //header-name// (of either form) when the first `"` or `>` character is reached (depending on the form of header name), regardless of preceding `\`s. But... that's a more invasive change (you'll need to change the handling of double-quoted string literals too, for consistency). Let's keep that change and this one separate. https://reviews.llvm.org/D41423 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41963: [clang-tidy] Adding Fuchsia checker for thread local storage.
juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, alexfh, hokein, ilya-biryukov.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.
Adds a check to the Fuchsia module to warn if thread-local storage is used.
See https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md for reference.
https://reviews.llvm.org/D41963
Files:
clang-tidy/fuchsia/CMakeLists.txt
clang-tidy/fuchsia/FuchsiaTidyModule.cpp
clang-tidy/fuchsia/ThreadLocalCheck.cpp
clang-tidy/fuchsia/ThreadLocalCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/fuchsia-thread-local.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/fuchsia-thread-local.cpp
Index: test/clang-tidy/fuchsia-thread-local.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-thread-local.cpp
@@ -0,0 +1,13 @@
+// RUN: %check_clang_tidy %s fuchsia-thread-local %t
+
+int main() {
+ thread_local int foo;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: thread local storage is disallowed
+ // CHECK-NEXT: thread_local int foo;
+
+ extern thread_local int bar;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: thread local storage is disallowed
+ // CHECK-NEXT: extern thread_local int bar;
+ int baz;
+ return 0;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -71,6 +71,7 @@
fuchsia-default-arguments
fuchsia-overloaded-operator
fuchsia-statically-constructed-objects
+ fuchsia-thread-local
fuchsia-virtual-inheritance
google-build-explicit-make-pair
google-build-namespaces
Index: docs/clang-tidy/checks/fuchsia-thread-local.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-thread-local.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - fuchsia-thread-local
+
+fuchsia-thread-local
+
+
+Warns if thread-local storage is used.
+
+For example, using ``thread_local`` or ``extern thread_local`` is not allowed:
+
+.. code-block:: c++
+
+ thread_local int foo; // Warning
+ extern thread_local int bar;// Warning
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -64,6 +64,11 @@
object is statically initialized with a ``constexpr`` constructor or has no
explicit constructor.
+- New `fuchsia-thread-local
+ `_ check
+
+ Warns if thread-local storage is used.
+
Improvements to include-fixer
-
Index: clang-tidy/fuchsia/ThreadLocalCheck.h
===
--- /dev/null
+++ clang-tidy/fuchsia/ThreadLocalCheck.h
@@ -0,0 +1,35 @@
+//===--- ThreadLocalCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_THREAD_LOCAL_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_THREAD_LOCAL_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+/// Thread-local storage is disallowed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-thread-local.html
+class ThreadLocalCheck : public ClangTidyCheck {
+public:
+ ThreadLocalCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_THREAD_LOCAL_H
Index: clang-tidy/fuchsia/ThreadLocalCheck.cpp
===
--- /dev/null
+++ clang-tidy/fuchsia/ThreadLocalCheck.cpp
@@ -0,0 +1,32 @@
+//===--- ThreadLocalCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ThreadLocalCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy
RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26
> -Original Message- > From: Szabolcs Nagy via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Thursday, January 11, 2018 6:13 AM > To: Blower, Melanie ; Keane, Erich > ; sca...@apple.com; roger.ferreriba...@arm.com; > sjoerd.mei...@arm.com; jle...@google.com; > hubert.reinterpretc...@gmail.com > Cc: szabolcs.n...@arm.com; james.greenha...@arm.com; > eli.fried...@gmail.com; efrie...@codeaurora.org; rich...@metafoo.co.uk; > cfe-commits@lists.llvm.org; mlek...@skidmore.edu; jkor...@apple.com; > shen...@google.com; t...@google.com > Subject: [PATCH] D40673: Add _Float128 as alias to __float128 to enable > compilations on Fedora27/glibc2-26 > > nsz added a comment. > > if clang wants to provide _Float128 then the f128 constant suffix (specified > by > TS18661-3) and __builtin_inff128, __builtin_nanf128, __builtin_nansf128, > __builtin_huge_valf128 (gcc builtins required by math.h) need to be supported > too. [Blower, Melanie] You are right, there are f128 builtins missing and also we need the f128 literal suffix. There may be other builtins missing as well. Some of glibc sources refer to __FLT_EVAL_METHOD_TS_18661_3__ does clang support that? > > as this patch is committed clang is broken (cannot use glibc headers) on any > target where long double is quad precision (aarch64, mips64, s390, sparc64, > riscv64) even if glibc fixes the >=gcc-7 check to something that works for > clang: > either the _Float128 typedef conflicts with the definition by clang or the > suffixes/builtins are missing. > > > Repository: > rC Clang > > https://reviews.llvm.org/D40673 > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy
leanil updated this revision to Diff 129508.
leanil added a comment.
Nest condition checking. Add positive test.
https://reviews.llvm.org/D41384
Files:
lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
test/Analysis/security-syntax-checks.m
Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -146,6 +146,16 @@
strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as
it does not provide bounding of the memory buffer. Replace unbounded copy
functions with analogous functions that support length arguments such as
'strlcpy'. CWE-119}}
}
+void test_strcpy_2() {
+ char x[4];
+ strcpy(x, "abcd"); //expected-warning{{Call to function 'strcpy' is insecure
as it does not provide bounding of the memory buffer. Replace unbounded copy
functions with analogous functions that support length arguments such as
'strlcpy'. CWE-119}}
+}
+
+void test_strcpy_safe() {
+ char x[5];
+ strcpy(x, "abcd");
+}
+
//===--===
// strcat()
//===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,18 @@
if (!checkCall_strCommon(CE, FD))
return;
+ const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+ if (const auto *DeclRef = dyn_cast(Target))
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr())) {
+ unsigned long long ArraySize = Array->getSize().getLimitedValue();
+ if (const auto *String = dyn_cast(Source)) {
+if (ArraySize >= String->getLength() + 1)
+ return;
+ }
+}
+
// Issue a warning.
PathDiagnosticLocation CELoc =
PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -146,6 +146,16 @@
strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119}}
}
+void test_strcpy_2() {
+ char x[4];
+ strcpy(x, "abcd"); //expected-warning{{Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119}}
+}
+
+void test_strcpy_safe() {
+ char x[5];
+ strcpy(x, "abcd");
+}
+
//===--===
// strcat()
//===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,18 @@
if (!checkCall_strCommon(CE, FD))
return;
+ const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+ if (const auto *DeclRef = dyn_cast(Target))
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr())) {
+ unsigned long long ArraySize = Array->getSize().getLimitedValue();
+ if (const auto *String = dyn_cast(Source)) {
+if (ArraySize >= String->getLength() + 1)
+ return;
+ }
+}
+
// Issue a warning.
PathDiagnosticLocation CELoc =
PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26
> -Original Message- > From: Szabolcs Nagy via Phabricator [mailto:revi...@reviews.llvm.org] > Sent: Thursday, January 11, 2018 6:25 AM > To: Blower, Melanie ; Keane, Erich > ; sca...@apple.com; roger.ferreriba...@arm.com; > sjoerd.mei...@arm.com; jle...@google.com; > hubert.reinterpretc...@gmail.com > Cc: szabolcs.n...@arm.com; james.greenha...@arm.com; > eli.fried...@gmail.com; efrie...@codeaurora.org; rich...@metafoo.co.uk; > cfe-commits@lists.llvm.org; mlek...@skidmore.edu; jkor...@apple.com; > shen...@google.com; t...@google.com > Subject: [PATCH] D40673: Add _Float128 as alias to __float128 to enable > compilations on Fedora27/glibc2-26 > > nsz added a comment. > > also note that there is less than 3 weeks until glibc-2.27 is released, if the > headers need a fix for clang then say so quickly > > i opened https://sourceware.org/bugzilla/show_bug.cgi?id=22700 but it needs > attention from some clang developers, in particular there is no way to check > if > the compiler has _Float128 type defined but no working f128 suffix in the > headers. [Blower, Melanie] Thanks for opening this request. Perhaps we need to pull out this patch and only put it back in when all the necessary builtin's and f128 literal suffix is available. > > > Repository: > rC Clang > > https://reviews.llvm.org/D40673 > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39903: [libclang] Allow pretty printing declarations
jbcoe added inline comments. Comment at: tools/libclang/libclang.exports:365 +clang_PrintingPolicy_get +clang_PrintingPolicy_set +clang_PrintingPolicy_dispose clang_PrintingPolicy_setProperty and clang_PrintingPolicy_getProperty might be better names (I know the ones you have used were my earlier suggestions). Repository: rC Clang https://reviews.llvm.org/D39903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy
leanil marked 3 inline comments as done. leanil added inline comments. Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:526 + } + if (StrLenFound && ArraySize >= StrLen + 1) +return; george.karpenkov wrote: > Why not put this if-expression into the one above where `StrLen` is found? > That would eliminate `StrLenFound` and remove the potential error surface of > uninitialized read from `StrLen` (the declaration for which should probably > be inside this block as well) Good point. This makes `StrLen` itself redundant as well. https://reviews.llvm.org/D41384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy
NoQ accepted this revision.
NoQ added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr())) {
+ unsigned long long ArraySize = Array->getSize().getLimitedValue();
This can be simplified to `const auto *Array =
DeclRef->getType()->getAs()`.
`.getTypePtr()` is almost always redundant because of the fancy `operator->()`
on `QualType`.
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:518
+DeclRef->getDecl()->getType().getTypePtr())) {
+ unsigned long long ArraySize = Array->getSize().getLimitedValue();
+ if (const auto *String = dyn_cast(Source)) {
Hmm, actually, `ArraySize` is the number of elements in the array, not its byte
size, so you may want (if you like) to also suppress the warning when the array
is not a `char` array (even if it's a weird code anyway) by instead taking
`ASTContext.getTypeSize()` here instead.
Also i guess it's nice to use `uint64_t` because that's the return type of
`.getLimitedValue()` and that's what we usually use all over the place when we
have to deal with those values.
https://reviews.llvm.org/D41384
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy
NoQ added a comment. Do you have commit access or should someone else commit it for you? https://reviews.llvm.org/D41384 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41792: [AArch64] Add ARMv8.2-A FP16 scalar intrinsics
az updated this revision to Diff 129513. az marked 3 inline comments as done. https://reviews.llvm.org/D41792 Files: clang/include/clang/Basic/BuiltinsNEON.def clang/include/clang/Basic/CMakeLists.txt clang/include/clang/Basic/arm_fp16.td clang/include/clang/Basic/arm_neon.td clang/include/clang/Basic/arm_neon_incl.td clang/lib/Basic/Targets/AArch64.cpp clang/lib/CodeGen/CGBuiltin.cpp clang/lib/Headers/CMakeLists.txt clang/lib/Headers/module.modulemap clang/lib/Sema/SemaChecking.cpp clang/test/CodeGen/aarch64-v8.2a-fp16-intrinsics.c clang/utils/TableGen/NeonEmitter.cpp clang/utils/TableGen/TableGen.cpp clang/utils/TableGen/TableGenBackends.h llvm/include/llvm/IR/IntrinsicsAArch64.td Index: llvm/include/llvm/IR/IntrinsicsAArch64.td === --- llvm/include/llvm/IR/IntrinsicsAArch64.td +++ llvm/include/llvm/IR/IntrinsicsAArch64.td @@ -146,6 +146,9 @@ class AdvSIMD_CvtFPToFx_Intrinsic : Intrinsic<[llvm_anyint_ty], [llvm_anyfloat_ty, llvm_i32_ty], [IntrNoMem]>; + + class AdvSIMD_1Arg_Intrinsic +: Intrinsic<[llvm_any_ty], [LLVMMatchType<0>], [IntrNoMem]>; } // Arithmetic ops @@ -244,7 +247,7 @@ // Vector Max def int_aarch64_neon_smax : AdvSIMD_2VectorArg_Intrinsic; def int_aarch64_neon_umax : AdvSIMD_2VectorArg_Intrinsic; - def int_aarch64_neon_fmax : AdvSIMD_2VectorArg_Intrinsic; + def int_aarch64_neon_fmax : AdvSIMD_2FloatArg_Intrinsic; def int_aarch64_neon_fmaxnmp : AdvSIMD_2VectorArg_Intrinsic; // Vector Max Across Lanes @@ -256,7 +259,7 @@ // Vector Min def int_aarch64_neon_smin : AdvSIMD_2VectorArg_Intrinsic; def int_aarch64_neon_umin : AdvSIMD_2VectorArg_Intrinsic; - def int_aarch64_neon_fmin : AdvSIMD_2VectorArg_Intrinsic; + def int_aarch64_neon_fmin : AdvSIMD_2FloatArg_Intrinsic; def int_aarch64_neon_fminnmp : AdvSIMD_2VectorArg_Intrinsic; // Vector Min/Max Number @@ -354,7 +357,7 @@ def int_aarch64_neon_sqxtun : AdvSIMD_1VectorArg_Narrow_Intrinsic; // Vector Absolute Value - def int_aarch64_neon_abs : AdvSIMD_1IntArg_Intrinsic; + def int_aarch64_neon_abs : AdvSIMD_1Arg_Intrinsic; // Vector Saturating Absolute Value def int_aarch64_neon_sqabs : AdvSIMD_1IntArg_Intrinsic; Index: clang/utils/TableGen/TableGenBackends.h === --- clang/utils/TableGen/TableGenBackends.h +++ clang/utils/TableGen/TableGenBackends.h @@ -65,6 +65,7 @@ void EmitClangCommentCommandList(RecordKeeper &Records, raw_ostream &OS); void EmitNeon(RecordKeeper &Records, raw_ostream &OS); +void EmitFP16(RecordKeeper &Records, raw_ostream &OS); void EmitNeonSema(RecordKeeper &Records, raw_ostream &OS); void EmitNeonTest(RecordKeeper &Records, raw_ostream &OS); void EmitNeon2(RecordKeeper &Records, raw_ostream &OS); Index: clang/utils/TableGen/TableGen.cpp === --- clang/utils/TableGen/TableGen.cpp +++ clang/utils/TableGen/TableGen.cpp @@ -52,6 +52,7 @@ GenClangCommentCommandInfo, GenClangCommentCommandList, GenArmNeon, + GenArmFP16, GenArmNeonSema, GenArmNeonTest, GenAttrDocs, @@ -139,6 +140,7 @@ "Generate list of commands that are used in " "documentation comments"), clEnumValN(GenArmNeon, "gen-arm-neon", "Generate arm_neon.h for clang"), +clEnumValN(GenArmFP16, "gen-arm-fp16", "Generate arm_fp16.h for clang"), clEnumValN(GenArmNeonSema, "gen-arm-neon-sema", "Generate ARM NEON sema support for clang"), clEnumValN(GenArmNeonTest, "gen-arm-neon-test", @@ -250,6 +252,9 @@ case GenArmNeon: EmitNeon(Records, OS); break; + case GenArmFP16: +EmitFP16(Records, OS); +break; case GenArmNeonSema: EmitNeonSema(Records, OS); break; Index: clang/utils/TableGen/NeonEmitter.cpp === --- clang/utils/TableGen/NeonEmitter.cpp +++ clang/utils/TableGen/NeonEmitter.cpp @@ -552,7 +552,11 @@ // run - Emit arm_neon.h.inc void run(raw_ostream &o); + // runFP16 - Emit arm_fp16.h.inc + void runFP16(raw_ostream &o); + // runHeader - Emit all the __builtin prototypes used in arm_neon.h + // and arm_fp16.h void runHeader(raw_ostream &o); // runTests - Emit tests for all the Neon intrinsics. @@ -852,6 +856,35 @@ NumVectors = 0; Float = true; break; + case 'Y': +Bitwidth = ElementBitwidth = 16; +NumVectors = 0; +Float = true; +break; + case 'I': +Bitwidth = ElementBitwidth = 32; +NumVectors = 0; +Float = false; +Signed = true; +break; + case 'L': +Bitwidth = ElementBitwidth = 64; +NumVectors = 0; +Float = false; +Signed = true; +break; + case 'U': +Bitwidth = ElementBitwidth = 32; +NumVectors = 0; +Float = false; +Signed = false; +break; +
[PATCH] D41815: [clang-tidy] implement check for goto
JonasToth added a comment. In https://reviews.llvm.org/D41815#973265, @lebedev.ri wrote: > In https://reviews.llvm.org/D41815#973260, @JonasToth wrote: > > > - check that jumps will only be forward. AFAIK that is required in all > > sensefull usecases of goto, is it? > > > You could implement loops/recursion with goto, something like: > > const int end = 10; > int i = 0; > assert(i < end); > > begin: > > i++ > if(i < end) > goto begin; > > // end > > > But it really should be done with normal `for()`, or `while()`, so i think it > would make sense to diagnose those. That check is specifically targeting these code constellations to be bad practice. As you said, using the looping constructs is the way to go :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41815: [clang-tidy] implement check for goto
lebedev.ri added inline comments.
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:20
+
+AST_MATCHER(GotoStmt, isForwardJumping) {
+
JonasToth wrote:
> lebedev.ri wrote:
> > It would be nice to have it in standard ASTMatchers, i believe it will be
> > useful for `else-after-return` check.
> > Though the ASTMatchers are stuck due to the doc-dumping script being
> > 'broken' (see D41455)
> Yes. The GNU extension goto does not have ASTMatcher yet, so i will pack
> these together in a review. What do you think how long the ASTMatcher issue
> will be there? Maybe it could be done after that check lands?
> Maybe it could be done after that check lands?
Yes, absolutely. I did not meant that as a blocker here.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41815: [clang-tidy] implement check for goto
JonasToth added inline comments.
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:20
+
+AST_MATCHER(GotoStmt, isForwardJumping) {
+
lebedev.ri wrote:
> It would be nice to have it in standard ASTMatchers, i believe it will be
> useful for `else-after-return` check.
> Though the ASTMatchers are stuck due to the doc-dumping script being 'broken'
> (see D41455)
Yes. The GNU extension goto does not have ASTMatcher yet, so i will pack these
together in a review. What do you think how long the ASTMatcher issue will be
there? Maybe it could be done after that check lands?
Comment at: clang-tidy/cppcoreguidelines/AvoidGotoCheck.cpp:41-53
+ if (const auto *BackGoto =
+ Result.Nodes.getNodeAs("backward-goto")) {
+diag(BackGoto->getGotoLoc(), "do not jump backwards with 'goto'")
+<< BackGoto->getSourceRange();
+diag(BackGoto->getLabel()->getLocStart(), "label defined here",
+ DiagnosticIDs::Note);
+ }
aaron.ballman wrote:
> Is there a reason to have two separate diagnostics? It seems like these both
> would be covered by "this use of 'goto' for flow control is prohibited".
Yes, the new wording better. The note about the backward jump could be added in
the `label defined here` note. I think distignuishing between backward jumps
and forward jumps is still a good thing.
The forward jumps could come from a C code part where forward jumps are done
for resource cleaning. So having a strong `prohibited` and a suggesting `avoid`
diagnostic makes sense to me.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
EricWF added inline comments. Comment at: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.remove_all/remove_all.pass.cpp:86 + +TEST_CHECK(fs::remove_all(p) == 0); +TEST_CHECK(!ec); This test is incorrect. `ec` isn't passed to the function. Repository: rL LLVM https://reviews.llvm.org/D41830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41933: handle scoped_lockable objects being returned by value in C++17
delesley accepted this revision. delesley added a comment. LGTM. Thanks, Richard! Repository: rC Clang https://reviews.llvm.org/D41933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41933: handle scoped_lockable objects being returned by value in C++17
rsmith closed this revision. rsmith added a comment. Committed as r322316. Repository: rC Clang https://reviews.llvm.org/D41933 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322316 - Handle scoped_lockable objects being returned by value in C++17.
Author: rsmith
Date: Thu Jan 11 14:13:57 2018
New Revision: 322316
URL: http://llvm.org/viewvc/llvm-project?rev=322316&view=rev
Log:
Handle scoped_lockable objects being returned by value in C++17.
In C++17, guaranteed copy elision means that there isn't necessarily a
constructor call when a local variable is initialized by a function call that
returns a scoped_lockable by value. In order to model the effects of
initializing a local variable with a function call returning a scoped_lockable,
pretend that the move constructor was invoked within the caller at the point of
return.
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=322316&r1=322315&r2=322316&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Thu Jan 11 14:13:57 2018
@@ -1689,7 +1689,7 @@ void BuildLockset::handleCall(Expr *Exp,
CapExprSet ScopedExclusiveReqs, ScopedSharedReqs;
StringRef CapDiagKind = "mutex";
- // Figure out if we're calling the constructor of scoped lockable class
+ // Figure out if we're constructing an object of scoped lockable class
bool isScopedVar = false;
if (VD) {
if (const CXXConstructorDecl *CD = dyn_cast(D)) {
@@ -1991,6 +1991,33 @@ void BuildLockset::VisitCXXConstructExpr
// FIXME -- only handles constructors in DeclStmt below.
}
+static CXXConstructorDecl *
+findConstructorForByValueReturn(const CXXRecordDecl *RD) {
+ // Prefer a move constructor over a copy constructor. If there's more than
+ // one copy constructor or more than one move constructor, we arbitrarily
+ // pick the first declared such constructor rather than trying to guess which
+ // one is more appropriate.
+ CXXConstructorDecl *CopyCtor = nullptr;
+ for (CXXConstructorDecl *Ctor : RD->ctors()) {
+if (Ctor->isDeleted())
+ continue;
+if (Ctor->isMoveConstructor())
+ return Ctor;
+if (!CopyCtor && Ctor->isCopyConstructor())
+ CopyCtor = Ctor;
+ }
+ return CopyCtor;
+}
+
+static Expr *buildFakeCtorCall(CXXConstructorDecl *CD, ArrayRef Args,
+ SourceLocation Loc) {
+ ASTContext &Ctx = CD->getASTContext();
+ return CXXConstructExpr::Create(Ctx, Ctx.getRecordType(CD->getParent()), Loc,
+ CD, true, Args, false, false, false, false,
+ CXXConstructExpr::CK_Complete,
+ SourceRange(Loc, Loc));
+}
+
void BuildLockset::VisitDeclStmt(DeclStmt *S) {
// adjust the context
LVarCtx = Analyzer->LocalVarMap.getNextContext(CtxIndex, S, LVarCtx);
@@ -1998,15 +2025,34 @@ void BuildLockset::VisitDeclStmt(DeclStm
for (auto *D : S->getDeclGroup()) {
if (VarDecl *VD = dyn_cast_or_null(D)) {
Expr *E = VD->getInit();
+ if (!E)
+continue;
+ E = E->IgnoreParens();
+
// handle constructors that involve temporaries
- if (ExprWithCleanups *EWC = dyn_cast_or_null(E))
+ if (auto *EWC = dyn_cast(E))
E = EWC->getSubExpr();
+ if (auto *BTE = dyn_cast(E))
+E = BTE->getSubExpr();
- if (CXXConstructExpr *CE = dyn_cast_or_null(E)) {
+ if (CXXConstructExpr *CE = dyn_cast(E)) {
NamedDecl *CtorD = dyn_cast_or_null(CE->getConstructor());
if (!CtorD || !CtorD->hasAttrs())
- return;
-handleCall(CE, CtorD, VD);
+ continue;
+handleCall(E, CtorD, VD);
+ } else if (isa(E) && E->isRValue()) {
+// If the object is initialized by a function call that returns a
+// scoped lockable by value, use the attributes on the copy or move
+// constructor to figure out what effect that should have on the
+// lockset.
+// FIXME: Is this really the best way to handle this situation?
+auto *RD = E->getType()->getAsCXXRecordDecl();
+if (!RD || !RD->hasAttr())
+ continue;
+CXXConstructorDecl *CtorD = findConstructorForByValueReturn(RD);
+if (!CtorD || !CtorD->hasAttrs())
+ continue;
+handleCall(buildFakeCtorCall(CtorD, {E}, E->getLocStart()), CtorD, VD);
}
}
}
Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=322316&r1=322315&r2=322316&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Thu Jan 11 14:13:57
2018
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety
-Wthread-safety-beta -Wno-thread-safety-negative -fcxx-exceptions
-D
r322318 - Make internal/private GVs implicitly dso_local.
Author: rafael
Date: Thu Jan 11 14:15:12 2018
New Revision: 322318
URL: http://llvm.org/viewvc/llvm-project?rev=322318&view=rev
Log:
Make internal/private GVs implicitly dso_local.
While updating clang tests for having clang set dso_local I noticed
that:
- There are *a lot* of tests to update.
- Many of the updates are redundant.
They are redundant because a GV is "obviously dso_local". This patch
starts formalizing that a bit by requiring that internal and private
GVs be dso_local too. Since they all are, we don't have to print
dso_local to the textual representation, making it a bit more compact
and easier to read.
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-static-initializers.cpp
cfe/trunk/test/OpenMP/target_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_parallel_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_parallel_for_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_simd_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_teams_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_codegen_registration.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen_registration.cpp
Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=322318&r1=322317&r2=322318&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Thu Jan 11 14:15:12 2018
@@ -5912,6 +5912,7 @@ void CGOpenMPRuntime::emitTargetOutlined
if (CGM.getLangOpts().OpenMPIsDevice) {
OutlinedFnID = llvm::ConstantExpr::getBitCast(OutlinedFn, CGM.Int8PtrTy);
OutlinedFn->setLinkage(llvm::GlobalValue::ExternalLinkage);
+OutlinedFn->setDSOLocal(false);
} else
OutlinedFnID = new llvm::GlobalVariable(
CGM.getModule(), CGM.Int8Ty, /*isConstant=*/true,
Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-static-initializers.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-static-initializers.cpp?rev=322318&r1=322317&r2=322318&view=diff
==
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-static-initializers.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-static-initializers.cpp Thu Jan 11
14:15:12 2018
@@ -28,11 +28,11 @@ S s;
// See @llvm.global_ctors above.
__declspec(selectany) S selectany1;
__declspec(selectany) S selectany2;
-// CHECK: define linkonce_odr void @"\01??__Eselectany1@@YAXXZ"() {{.*}} comdat
+// CHECK: define linkonce_odr dso_local void @"\01??__Eselectany1@@YAXXZ"()
{{.*}} comdat
// CHECK-NOT: @"\01??_Bselectany1
// CHECK: call x86_thiscallcc %struct.S* @"\01??0S@@QAE@XZ"
// CHECK: ret void
-// CHECK: define linkonce_odr void @"\01??__Eselectany2@@YAXXZ"() {{.*}} comdat
+// CHECK: define linkonce_odr dso_local void @"\01??__Eselectany2@@YAXXZ"()
{{.*}} comdat
// CHECK-NOT: @"\01??_Bselectany2
// CHECK: call x86_thiscallcc %struct.S* @"\01??0S@@QAE@XZ"
// CHECK: ret void
@@ -231,7 +231,7 @@ void force_usage() {
DynamicDLLImportInitVSMangling::switch_test3();
}
-// CHECK: define linkonce_odr void @"\01??__Efoo@?$B@H@@2VA@@A@YAXXZ"() {{.*}}
comdat
+// CHECK: define linkonce_odr dso_local void
@"\01??__Efoo@?$B@H@@2VA@@A@YAXXZ"() {{.*}} comdat
// CHECK-NOT: and
// CHECK-NOT: ?_Bfoo@
// CHECK: call x86_thiscallcc %class.A* @"\01??0A@@QAE@XZ"
Modified: cfe/trunk/test/OpenMP/target_codegen_registration.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_codegen_registration.cpp?rev=322318&r1=322317&r2=322318&view=diff
==
--- cfe/trunk/test/OpenMP/target_codegen_registration.cpp (original)
+++ cfe/trunk/test/OpenMP/target_codegen_registration.cpp Thu Jan 11 14:15:12
2018
@@ -393,7 +393,7 @@ struct ST {
//CHECK: ret void
//CHECK: declare i32 @__tgt_unregister_lib([[DSCTY]]*)
-//CHECK: define linkonce hidden void @[[REGFN]](i8*)
+//CHECK: define linkonce dso_local hidden void @[[REGFN]](i8*)
//CHECK-SAME: comdat {
//CHECK: call i32 @__tgt_register_lib([[DSCTY]]* [[DESC]])
//CHECK: call i32 @__cxa_atexit(void (i8*)* @[[UNREGFN]], i8* bitcast
([[DSCTY]]* [[DESC]] to i8*),
Modified: cfe/trunk/test/OpenMP/target_parallel_codegen_registration.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_parallel_codegen_registration.cpp?rev=322318&r1=322317&r2=322318&view=diff
==
--- cfe/trunk/test/OpenMP/target_parallel_codegen_registration.cpp (original)
+++ cfe/trunk/test/OpenMP/target_parallel_codegen_registration.cpp Thu Jan 11
14:15:1
