[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri updated this revision to Diff 170045.
lebedev.ri marked 6 inline comments as done.
lebedev.ri added a comment.
- Apply minor wording nits.
- For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not **anything**
else (not even `llu`).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52670
Files:
clang-tidy/cert/CERTTidyModule.cpp
clang-tidy/cert/CMakeLists.txt
clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tidy/readability/UppercaseLiteralSuffixCheck.h
clang-tidy/utils/ASTUtils.cpp
clang-tidy/utils/ASTUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cert-dcl16-c.rst
docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
test/clang-tidy/readability-uppercase-literal-suffix.h
Index: test/clang-tidy/readability-uppercase-literal-suffix.h
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix.h
@@ -0,0 +1,16 @@
+template
+struct integral_constant {
+ static constexpr T value = v;
+ typedef T value_type;
+ typedef integral_constant type; // using injected-class-name
+ constexpr operator value_type() const noexcept { return value; }
+};
+
+using false_type = integral_constant;
+using true_type = integral_constant;
+
+template
+struct is_same : false_type {};
+
+template
+struct is_same : true_type {};
Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S
+
+#include "readability-uppercase-literal-suffix.h"
+
+void integer_suffix() {
+ static constexpr auto v0 = __LINE__; // synthetic
+ static_assert(v0 == 9 || v0 == 5, "");
+
+ static constexpr auto v1 = __cplusplus; // synthetic, long
+
+ static constexpr auto v2 = 1; // no literal
+ static_assert(is_same::value, "");
+ static_assert(v2 == 1, "");
+
+ // Unsigned
+
+ static constexpr auto v3 = 1u;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'u', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v3 = 1u;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}}
+ // CHECK-FIXES: static constexpr auto v3 = 1U;
+ static_assert(is_same::value, "");
+ static_assert(v3 == 1, "");
+
+ static constexpr auto v4 = 1U; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v4 == 1, "");
+
+ // Long
+
+ static constexpr auto v5 = 1l;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'l', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v5 = 1l;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}L{{$}}
+ // CHECK-FIXES: static constexpr auto v5 = 1L;
+ static_assert(is_same::value, "");
+ static_assert(v5 == 1, "");
+
+ static constexpr auto v6 = 1L; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v6 == 1, "");
+
+ // Long Long
+
+ static constexpr auto v7 = 1ll;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'll', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v7 = 1ll;
+ // CHECK-MESSAGES-NEXT: ^~~
+ // CHECK-MESSAGES-NEXT: {{^ *}}LL{{$}}
+ // CHECK-FIXES: static constexpr auto v7 = 1LL;
+ static_assert(is_same::value, "");
+ static_assert(v7 == 1, "");
+
+ static constexpr auto v8 = 1LL; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v8 == 1, "");
+
+ // Unsigned Long
+
+ static constexpr auto v9 = 1ul;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'ul', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1ul;
[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.
ioeric updated this revision to Diff 170046.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- address comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53374
Files:
clangd/AST.cpp
clangd/AST.h
clangd/Quality.cpp
clangd/Quality.h
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
unittests/clangd/QualityTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -83,6 +83,9 @@
IsIndexedForCodeCompletion;
}
MATCHER(Deprecated, "") { return arg.Flags & Symbol::Deprecated; }
+MATCHER(ImplementationDetail, "") {
+ return arg.Flags & Symbol::ImplementationDetail;
+}
MATCHER(RefRange, "") {
const Ref &Pos = testing::get<0>(arg);
const Range &Range = testing::get<1>(arg);
@@ -1037,6 +1040,21 @@
AllOf(QName("TestClangd"), Not(Deprecated();
}
+TEST_F(SymbolCollectorTest, ImplementationDetail) {
+ const std::string Header = R"(
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class Public {};
+ )";
+ runSymbolCollector(Header, /**/ "");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("X_Y_Decl"), ImplementationDetail()),
+ AllOf(QName("Public"), Not(ImplementationDetail();
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -45,17 +45,34 @@
[[deprecated]]
int _f() { return _X; }
+
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class MAC {};
)cpp");
+ Header.ExtraArgs = {"-DMAC=mac_name"};
+
auto Symbols = Header.headerSymbols();
auto AST = Header.build();
SymbolQualitySignals Quality;
Quality.merge(findSymbol(Symbols, "_X"));
EXPECT_FALSE(Quality.Deprecated);
+ EXPECT_FALSE(Quality.ImplementationDetail);
EXPECT_TRUE(Quality.ReservedName);
EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
+ Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
+ Quality.ImplementationDetail = false;
+ Quality.merge(
+ CodeCompletionResult(&findDecl(AST, "mac_name"), /*Priority=*/42));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
Symbol F = findSymbol(Symbols, "_f");
F.References = 24; // TestTU doesn't count references, so fake it.
Quality = {};
@@ -182,6 +199,10 @@
ReservedName.ReservedName = true;
EXPECT_LT(ReservedName.evaluate(), Default.evaluate());
+ SymbolQualitySignals ImplementationDetail;
+ ImplementationDetail.ImplementationDetail = true;
+ EXPECT_LT(ImplementationDetail.evaluate(), Default.evaluate());
+
SymbolQualitySignals WithReferences, ManyReferences;
WithReferences.References = 20;
ManyReferences.References = 1000;
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -536,6 +536,8 @@
if (isIndexedForCodeCompletion(ND, Ctx))
S.Flags |= Symbol::IndexedForCodeCompletion;
+ if (!isSpelledInSourceCode(&ND))
+S.Flags |= Symbol::ImplementationDetail;
S.SymInfo = index::getSymbolInfo(&ND);
std::string FileURI;
if (auto DeclLoc = getTokenLocation(findNameLoc(&ND), SM, Opts,
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -244,6 +244,8 @@
IndexedForCodeCompletion = 1 << 0,
/// Indicates if the symbol is deprecated.
Deprecated = 1 << 1,
+// Symbol is an implementation detail.
+ImplementationDetail = 1 << 2,
};
SymbolFlag Flags = SymbolFlag::None;
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -57,6 +57,7 @@
bool Deprecated = false;
bool ReservedName = false; // __foo, _Foo are usually implementation details.
// FIXME: make these findable once user types _.
+ bool ImplementationDetail = false;
unsigned References = 0;
enum SymbolCategory {
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -7,6 +7,7 @@
//
//===--===//
#include "Quality.h"
+#include "AST.h"
#include "FileDistance.h"
#include "URI.h"
#include
[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.
ioeric added inline comments. Comment at: clangd/Quality.cpp:182 if (auto *ID = SemaCCResult.Declaration->getIdentifier()) - ReservedName = ReservedName || isReserved(ID->getName()); + ReservedName = ReservedName || isReserved(ID->getName()) || + !SpelledInSourceCode(SemaCCResult.Declaration); sammccall wrote: > This doesn't match the current definition of `ReservedName`. > I'd suggest either: > - adding a new boolean (`ImplementationDetail`? maybe we'll add other > heuristics) and treating this as independent of reserved-ness > - renaming the current `ReservedName` flag to cover this expanded scope > (again, `ImplementationDetail` is a reasonable name) `ImplementationDetail` sounds great. Comment at: clangd/Quality.cpp:192 Category = categorize(IndexResult.SymInfo); ReservedName = ReservedName || isReserved(IndexResult.Name); } sammccall wrote: > The new `ReservedName` cases don't survive a round-trip through the index > (unlike the existing ones, which get recomputed from the name). > > I think you want to add a new flag bit to `Symbol`, set it in > `SymbolCollector`, and read it here. (IIRC new flags in the `Flags` field are > handled transparently by yaml and binary serialization). Yeah, I wasn't sure what the name to use here and wanted to get a a second opinion on adding the new flag. Thanks for the name! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53374 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344730 - [TI removal] Test predicate rather than casting to detect a terminator
Author: chandlerc
Date: Thu Oct 18 01:16:20 2018
New Revision: 344730
URL: http://llvm.org/viewvc/llvm-project?rev=344730&view=rev
Log:
[TI removal] Test predicate rather than casting to detect a terminator
and use the range based successor API.
Modified:
cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
Modified: cfe/trunk/lib/CodeGen/CGLoopInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGLoopInfo.cpp?rev=344730&r1=344729&r2=344730&view=diff
==
--- cfe/trunk/lib/CodeGen/CGLoopInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGLoopInfo.cpp Thu Oct 18 01:16:20 2018
@@ -12,6 +12,7 @@
#include "clang/AST/Attr.h"
#include "clang/Sema/LoopHint.h"
#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/CFG.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/InstrTypes.h"
#include "llvm/IR/Instructions.h"
@@ -335,10 +336,10 @@ void LoopInfoStack::InsertHelper(Instruc
if (!L.getLoopID())
return;
- if (TerminatorInst *TI = dyn_cast(I)) {
-for (unsigned i = 0, ie = TI->getNumSuccessors(); i < ie; ++i)
- if (TI->getSuccessor(i) == L.getHeader()) {
-TI->setMetadata(llvm::LLVMContext::MD_loop, L.getLoopID());
+ if (I->isTerminator()) {
+for (BasicBlock *Succ : successors(I))
+ if (Succ == L.getHeader()) {
+I->setMetadata(llvm::LLVMContext::MD_loop, L.getLoopID());
break;
}
return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52730: [analyzer] ConversionChecker: handle floating point
donat.nagy added a comment. Herald added a subscriber: dkrupp. polite ping Repository: rC Clang https://reviews.llvm.org/D52730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
curdeius added a comment.
In https://reviews.llvm.org/D53372#1267728, @lebedev.ri wrote:
> I think it would be good to add some more explanation as to *why* that `else`
> has to be kept.
Do you mean add a comment in the code or just an explanation for the review?
For the latter, e.g.:
// unrelated types:
struct type_a {};
struct type_b {};
auto f()
{
if constexpr(condition)
{
return type_a{};
}
else
{
return type_b{};
}
}
In this case removing else will just provoke a compilation error. There may be
some cases where you may remove else though.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53372
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
curdeius added a comment. Actually, I can make it an option for this check to skip or not constexpr ifs, WDYT? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53363: [clangd] Encode Line/Column as a 32-bits integer.
hokein updated this revision to Diff 170053.
hokein marked 6 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53363
Files:
clangd/FindSymbols.cpp
clangd/XRefs.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Serialization.cpp
clangd/index/SymbolCollector.cpp
clangd/index/YAMLSerialization.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -63,19 +63,19 @@
return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
}
MATCHER_P(DeclRange, Pos, "") {
- return std::tie(arg.CanonicalDeclaration.Start.Line,
- arg.CanonicalDeclaration.Start.Column,
- arg.CanonicalDeclaration.End.Line,
- arg.CanonicalDeclaration.End.Column) ==
- std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
- Pos.end.character);
+ return std::make_tuple(arg.CanonicalDeclaration.Start.line(),
+ arg.CanonicalDeclaration.Start.column(),
+ arg.CanonicalDeclaration.End.line(),
+ arg.CanonicalDeclaration.End.column()) ==
+ std::make_tuple(Pos.start.line, Pos.start.character, Pos.end.line,
+ Pos.end.character);
}
MATCHER_P(DefRange, Pos, "") {
- return std::tie(arg.Definition.Start.Line,
- arg.Definition.Start.Column, arg.Definition.End.Line,
- arg.Definition.End.Column) ==
- std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
- Pos.end.character);
+ return std::make_tuple(
+ arg.Definition.Start.line(), arg.Definition.Start.column(),
+ arg.Definition.End.line(), arg.Definition.End.column()) ==
+ std::make_tuple(Pos.start.line, Pos.start.character, Pos.end.line,
+ Pos.end.character);
}
MATCHER_P(RefCount, R, "") { return int(arg.References) == R; }
MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
@@ -86,10 +86,10 @@
MATCHER(RefRange, "") {
const Ref &Pos = testing::get<0>(arg);
const Range &Range = testing::get<1>(arg);
- return std::tie(Pos.Location.Start.Line, Pos.Location.Start.Column,
- Pos.Location.End.Line, Pos.Location.End.Column) ==
- std::tie(Range.start.line, Range.start.character, Range.end.line,
- Range.end.character);
+ return std::make_tuple(Pos.Location.Start.line(), Pos.Location.Start.column(),
+ Pos.Location.End.line(), Pos.Location.End.column()) ==
+ std::make_tuple(Range.start.line, Range.start.character,
+ Range.end.line, Range.end.character);
}
testing::Matcher &>
HaveRanges(const std::vector Ranges) {
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -31,13 +31,28 @@
MATCHER_P(Named, N, "") { return arg.Name == N; }
MATCHER_P(RefRange, Range, "") {
- return std::tie(arg.Location.Start.Line, arg.Location.Start.Column,
- arg.Location.End.Line, arg.Location.End.Column) ==
- std::tie(Range.start.line, Range.start.character, Range.end.line,
- Range.end.character);
+ return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),
+ arg.Location.End.line(), arg.Location.End.column()) ==
+ std::make_tuple(Range.start.line, Range.start.character,
+ Range.end.line, Range.end.character);
}
MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+TEST(SymbolLocation, Position) {
+ using Position = SymbolLocation::Position;
+ Position Pos;
+
+ Pos.setLine(1);
+ EXPECT_EQ(1u, Pos.line());
+ Pos.setColumn(2);
+ EXPECT_EQ(2u, Pos.column());
+
+ Pos.setLine(Position::MaxLine + 1); // overflow
+ EXPECT_EQ(Pos.line(), Position::MaxLine);
+ Pos.setColumn(Position::MaxColumn + 1); // overflow
+ EXPECT_EQ(Pos.column(), Position::MaxColumn);
+}
+
TEST(SymbolSlab, FindAndIterate) {
SymbolSlab::Builder B;
B.insert(symbol("Z"));
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -32,10 +32,10 @@
using testing::UnorderedElementsAre;
MATCHER_P(RefRange, Range, "") {
- return std::tie(arg.Location.Start.Line, arg.Location.Start.Column,
- arg.Location.End.Line, arg.Location.End.Column) ==
- std::tie(Range.start.line, Range.start.character, Range.end.line,
-
[PATCH] D52730: [analyzer] ConversionChecker: handle floating point
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164
+// double and possibly long double on some systems
+RepresentsUntilExp = 53; break;
+ case 32:
A link to the source of these number would be useful. How are these calculated.
Also, as far as I know the current C++ standard does not require anything
about how floating points are represented in an implementation. So it would be
great to somehow refer to the representation used by clang rather than
hardcoding these values. (Note that I am perfectly fine with warning for
implementation defined behavior as the original version also warn for such in
case of integers.)
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:172
+ default:
+// larger types, which can represent all integers below 2^64
+return false;
Comment should be full sentences with a full stop at the end.
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:183
+ unsigned CorrectedSrcWidth = AC.getIntWidth(SubType);
+ if (SubType->isSignedIntegerType()) {
+CorrectedSrcWidth--;
Do not add redundant braces when we only guard one line. It is perfectly ok
when the one statements spans over multiple lines (maybe due to comments).
Repository:
rC Clang
https://reviews.llvm.org/D52730
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53363: [clangd] Encode Line/Column as a 32-bits integer.
hokein added a comment.
> In https://reviews.llvm.org/D53363#1267721, @hokein wrote:
>
>> Yeah, I have a rough patch for it, using char* will save us ~50MB memory,
>> which will lead to ~300 MB memory usage in total.
>
>
> For just the StringRef in SymbolLocation::Position, or for all our strings?
> If the latter, that seems too low, as we should save strictly more than this
> patch (unless I'm missing something about alignment/padding)
This is just for the StringRef in `Position`, I think it matches our estimation.
More details coming:
- 6.4 million refs, 6.4m * 40 bytes = ~250MB
- 0.3 million symbols, 0.3m * 248 bytes = ~70MB
The original size of `Ref` (before any space optimization) is 40 bytes, we save
8 bytes per refs, 8*6.5 = ~50 MB; If we change all StringRef in `Symbol` to
`char*`, which will save 64 bytes (8*8) per sym, 64*0.3v = ~19 MB, which
doesn't gain as much as refs, as refs contributes more than 70% memory.
Comment at: clangd/index/Index.h:66
const SymbolLocation::Position &R) {
- return std::tie(L.Line, L.Column) == std::tie(R.Line, R.Column);
+ return std::make_tuple(L.line(), L.column()) ==
+ std::make_tuple(R.Line, R.Column);
sammccall wrote:
> Why only using the getters on one side?
Oops, I mis-thought the type of the right side LSP position, fixed.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53363
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53377: [clang-tidy] Ignore a case where the fix of make_unique check introduces side effect.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344733: [clang-tidy] Ignore a case where the fix of
make_unique check introduces side… (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53377
Files:
clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
@@ -403,18 +403,15 @@
// CHECK-FIXES: FFs = std::make_unique(Num2);
std::unique_ptr FI;
- FI.reset(new int[5]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(5);
- FI.reset(new int[5]());
+ FI.reset(new int[5]()); // default initialization.
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
// CHECK-FIXES: FI = std::make_unique(5);
+
+ // The check doesn't give warnings and fixes for cases where the original new
+ // expresion doesn't do any initialization.
+ FI.reset(new int[5]);
FI.reset(new int[Num]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num);
FI.reset(new int[Num2]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num2);
}
void aliases() {
Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -121,6 +121,15 @@
if (New->getNumPlacementArgs() != 0)
return;
+ // Be conservative for cases where we construct an array without any
+ // initalization.
+ // For example,
+ //P.reset(new int[5]) // check fix: P = make_unique(5)
+ //
+ // The fix of the check has side effect, it introduces default initialization
+ // which maybe unexpected and cause performance regression.
+ if (New->isArray() && !New->hasInitializer())
+return;
if (Construct)
checkConstruct(SM, Result.Context, Construct, Type, New);
else if (Reset)
Index: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
@@ -403,18 +403,15 @@
// CHECK-FIXES: FFs = std::make_unique(Num2);
std::unique_ptr FI;
- FI.reset(new int[5]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(5);
- FI.reset(new int[5]());
+ FI.reset(new int[5]()); // default initialization.
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
// CHECK-FIXES: FI = std::make_unique(5);
+
+ // The check doesn't give warnings and fixes for cases where the original new
+ // expresion doesn't do any initialization.
+ FI.reset(new int[5]);
FI.reset(new int[Num]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num);
FI.reset(new int[Num2]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num2);
}
void aliases() {
Index: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -121,6 +121,15 @@
if (New->getNumPlacementArgs() != 0)
return;
+ // Be conservative for cases where we construct an array without any
+ // initalization.
+ // For example,
+ //P.reset(new int[5]) // check fix: P = make_unique(5)
+ //
+ // The fix of the check has side effect, it introduces default initialization
+ // which maybe unexpected and cause performance regression.
+ if (New->isArray() && !New->hasInitializer())
+return;
if (Construct)
checkConstruct(SM, Result.Context, Construct, Type, New);
else if (Reset)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344733 - [clang-tidy] Ignore a case where the fix of make_unique check introduces side effect.
Author: hokein
Date: Thu Oct 18 02:13:34 2018
New Revision: 344733
URL: http://llvm.org/viewvc/llvm-project?rev=344733&view=rev
Log:
[clang-tidy] Ignore a case where the fix of make_unique check introduces side
effect.
Summary:
Previously, ptr.reset(new char[5]) will be replaced with `p =
make_unique(5)`, the fix has side effect -- doing
default initialization, it may cause performace regression (we are
bitten by this rececntly)
The check should be conservative for these cases.
Reviewers: alexfh
Subscribers: xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D53377
Modified:
clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
Modified: clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp?rev=344733&r1=344732&r2=344733&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/MakeSmartPtrCheck.cpp Thu Oct
18 02:13:34 2018
@@ -121,6 +121,15 @@ void MakeSmartPtrCheck::check(const Matc
if (New->getNumPlacementArgs() != 0)
return;
+ // Be conservative for cases where we construct an array without any
+ // initalization.
+ // For example,
+ //P.reset(new int[5]) // check fix: P = make_unique(5)
+ //
+ // The fix of the check has side effect, it introduces default initialization
+ // which maybe unexpected and cause performance regression.
+ if (New->isArray() && !New->hasInitializer())
+return;
if (Construct)
checkConstruct(SM, Result.Context, Construct, Type, New);
else if (Reset)
Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp?rev=344733&r1=344732&r2=344733&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-make-unique.cpp Thu Oct
18 02:13:34 2018
@@ -403,18 +403,15 @@ void initialization(int T, Base b) {
// CHECK-FIXES: FFs = std::make_unique(Num2);
std::unique_ptr FI;
- FI.reset(new int[5]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(5);
- FI.reset(new int[5]());
+ FI.reset(new int[5]()); // default initialization.
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
// CHECK-FIXES: FI = std::make_unique(5);
+
+ // The check doesn't give warnings and fixes for cases where the original new
+ // expresion doesn't do any initialization.
+ FI.reset(new int[5]);
FI.reset(new int[Num]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num);
FI.reset(new int[Num2]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num2);
}
void aliases() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53377: [clang-tidy] Ignore a case where the fix of make_unique check introduces side effect.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344733: [clang-tidy] Ignore a case where the fix of
make_unique check introduces side… (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53377?vs=170012&id=170054#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53377
Files:
clang-tidy/modernize/MakeSmartPtrCheck.cpp
test/clang-tidy/modernize-make-unique.cpp
Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -121,6 +121,15 @@
if (New->getNumPlacementArgs() != 0)
return;
+ // Be conservative for cases where we construct an array without any
+ // initalization.
+ // For example,
+ //P.reset(new int[5]) // check fix: P = make_unique(5)
+ //
+ // The fix of the check has side effect, it introduces default initialization
+ // which maybe unexpected and cause performance regression.
+ if (New->isArray() && !New->hasInitializer())
+return;
if (Construct)
checkConstruct(SM, Result.Context, Construct, Type, New);
else if (Reset)
Index: test/clang-tidy/modernize-make-unique.cpp
===
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -403,18 +403,15 @@
// CHECK-FIXES: FFs = std::make_unique(Num2);
std::unique_ptr FI;
- FI.reset(new int[5]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(5);
- FI.reset(new int[5]());
+ FI.reset(new int[5]()); // default initialization.
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
// CHECK-FIXES: FI = std::make_unique(5);
+
+ // The check doesn't give warnings and fixes for cases where the original new
+ // expresion doesn't do any initialization.
+ FI.reset(new int[5]);
FI.reset(new int[Num]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num);
FI.reset(new int[Num2]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num2);
}
void aliases() {
Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -121,6 +121,15 @@
if (New->getNumPlacementArgs() != 0)
return;
+ // Be conservative for cases where we construct an array without any
+ // initalization.
+ // For example,
+ //P.reset(new int[5]) // check fix: P = make_unique(5)
+ //
+ // The fix of the check has side effect, it introduces default initialization
+ // which maybe unexpected and cause performance regression.
+ if (New->isArray() && !New->hasInitializer())
+return;
if (Construct)
checkConstruct(SM, Result.Context, Construct, Type, New);
else if (Reset)
Index: test/clang-tidy/modernize-make-unique.cpp
===
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -403,18 +403,15 @@
// CHECK-FIXES: FFs = std::make_unique(Num2);
std::unique_ptr FI;
- FI.reset(new int[5]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(5);
- FI.reset(new int[5]());
+ FI.reset(new int[5]()); // default initialization.
// CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
// CHECK-FIXES: FI = std::make_unique(5);
+
+ // The check doesn't give warnings and fixes for cases where the original new
+ // expresion doesn't do any initialization.
+ FI.reset(new int[5]);
FI.reset(new int[Num]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num);
FI.reset(new int[Num2]);
- // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:
- // CHECK-FIXES: FI = std::make_unique(Num2);
}
void aliases() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
lebedev.ri added a comment. In https://reviews.llvm.org/D53372#1268207, @curdeius wrote: > In https://reviews.llvm.org/D53372#1267728, @lebedev.ri wrote: > > > I think it would be good to add some more explanation as to *why* that > > `else` has to be kept. > > > Do you mean add a comment in the code or just an explanation for the review? > In this case removing else will just provoke a compilation error. There may > be some cases where you may remove else though. I was indeed talking about the review, thank you. In https://reviews.llvm.org/D53372#1268208, @curdeius wrote: > Actually, I can make it an option for this check to skip or not constexpr > ifs, WDYT? I'm not sure, maybe this shouldn't be an option. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53389: [clangd] Clear the semantic of RefSlab::size.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
The RefSlab::size can easily cause confusions, it returns the number of
different symbols, rahter than the number of all references.
- rename size to numSymbols to indicate its semantic clearly.
- add numRefs() method and cache it, since calculating it everytime is
nontrivial.
- clear misused places.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53389
Files:
clangd/index/Background.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Serialization.cpp
Index: clangd/index/Serialization.cpp
===
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -496,18 +496,18 @@
}
}
- size_t SymSize = Symbols.size();
- size_t RefSize = Refs.size();
+ size_t NumSym = Symbols.size();
+ size_t NumRefs = Refs.numRefs();
+
trace::Span Tracer("BuildIndex");
auto Index =
UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
: MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2} bytes\n"
- " - number of symbos: {3}\n"
+ " - number of symbols: {3}\n"
" - number of refs: {4}\n",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
- Index->estimateMemoryUsage(),
- SymSize, RefSize);
+ Index->estimateMemoryUsage(), NumSym, NumRefs);
return Index;
}
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -390,7 +390,8 @@
const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
- size_t size() const { return Refs.size(); }
+ size_t numSymbols() const { return Refs.size(); }
+ size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }
size_t bytes() const {
@@ -414,11 +415,14 @@
};
private:
- RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena)
- : Arena(std::move(Arena)), Refs(std::move(Refs)) {}
+ RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena,
+ size_t NumRefs)
+ : Arena(std::move(Arena)), Refs(std::move(Refs)), NumRefs(NumRefs) {}
llvm::BumpPtrAllocator Arena;
std::vector Refs;
+ // Number of all references.
+ size_t NumRefs = 0;
};
struct FuzzyFindRequest {
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -152,17 +152,19 @@
// Reallocate refs on the arena to reduce waste and indirections when reading.
std::vector>> Result;
Result.reserve(Refs.size());
+ size_t NumRefs = 0;
for (auto &Sym : Refs) {
auto &SymRefs = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
+NumRefs += SymRefs.size();
auto *Array = Arena.Allocate(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef(Array, SymRefs.size()));
}
- return RefSlab(std::move(Result), std::move(Arena));
+ return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}
void SwapIndex::reset(std::unique_ptr Index) {
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -63,9 +63,9 @@
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
- " ref slab: {4} symbols, {5} bytes",
- FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
- Refs.bytes());
+ " ref slab: {4} symbols, {5} refs, {6} bytes",
+ FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.numSymbols(),
+ Refs.numRefs(), Refs.bytes());
return {std::move(Syms), std::move(Refs)};
}
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -174,9 +174,9 @@
Action->EndSourceFile();
log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
- Symbols.size(), Refs.size());
+ Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
- SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
+ SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique(std::move(Symbols)),
___
cfe-commits mailing list
[PATCH] D53389: [clangd] Clear the semantic of RefSlab::size.
sammccall added a comment. Adding numRefs() and fixing occurrences makes sense. However I think `size()` needs to stay - it's an important part of the "container" concept, and e.g. GTest relies on it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clangd/Quality.cpp:181
if (SemaCCResult.Declaration) {
+if (!isSpelledInSourceCode(SemaCCResult.Declaration))
+ ImplementationDetail = true;
We could consider putting this in a one-liner function in AST.h or so, in case
we want to add more heuristics - so SymbolCollector and Quality remain
consistent. Up to you.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53374
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53389: [clangd] Clear the semantic of RefSlab::size.
hokein updated this revision to Diff 170063.
hokein added a comment.
- reserve the size name, added a comment indicating its senmatic.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53389
Files:
clangd/index/Background.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Serialization.cpp
Index: clangd/index/Serialization.cpp
===
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -496,18 +496,18 @@
}
}
- size_t SymSize = Symbols.size();
- size_t RefSize = Refs.size();
+ size_t NumSym = Symbols.size();
+ size_t NumRefs = Refs.numRefs();
+
trace::Span Tracer("BuildIndex");
auto Index =
UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
: MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2} bytes\n"
- " - number of symbos: {3}\n"
+ " - number of symbols: {3}\n"
" - number of refs: {4}\n",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
- Index->estimateMemoryUsage(),
- SymSize, RefSize);
+ Index->estimateMemoryUsage(), NumSym, NumRefs);
return Index;
}
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -390,7 +390,9 @@
const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
+ /// Gets the number of symbols.
size_t size() const { return Refs.size(); }
+ size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }
size_t bytes() const {
@@ -414,11 +416,14 @@
};
private:
- RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena)
- : Arena(std::move(Arena)), Refs(std::move(Refs)) {}
+ RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena,
+ size_t NumRefs)
+ : Arena(std::move(Arena)), Refs(std::move(Refs)), NumRefs(NumRefs) {}
llvm::BumpPtrAllocator Arena;
std::vector Refs;
+ // Number of all references.
+ size_t NumRefs = 0;
};
struct FuzzyFindRequest {
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -152,17 +152,19 @@
// Reallocate refs on the arena to reduce waste and indirections when reading.
std::vector>> Result;
Result.reserve(Refs.size());
+ size_t NumRefs = 0;
for (auto &Sym : Refs) {
auto &SymRefs = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
+NumRefs += SymRefs.size();
auto *Array = Arena.Allocate(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef(Array, SymRefs.size()));
}
- return RefSlab(std::move(Result), std::move(Arena));
+ return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}
void SwapIndex::reset(std::unique_ptr Index) {
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -63,9 +63,9 @@
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
- " ref slab: {4} symbols, {5} bytes",
+ " ref slab: {4} symbols, {5} refs, {6} bytes",
FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
- Refs.bytes());
+ Refs.numRefs(), Refs.bytes());
return {std::move(Syms), std::move(Refs)};
}
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -174,9 +174,9 @@
Action->EndSourceFile();
log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
- Symbols.size(), Refs.size());
+ Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
- SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
+ SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique(std::move(Symbols)),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53389: [clangd] Clear the semantic of RefSlab::size.
hokein added a comment. In https://reviews.llvm.org/D53389#1268250, @sammccall wrote: > Adding numRefs() and fixing occurrences makes sense. > However I think `size()` needs to stay - it's an important part of the > "container" concept, and e.g. GTest relies on it. OK, and I added a comment for this method to avoid future confusion. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344735 - [clangd] Encode Line/Column as a 32-bits integer.
Author: hokein
Date: Thu Oct 18 03:43:50 2018
New Revision: 344735
URL: http://llvm.org/viewvc/llvm-project?rev=344735&view=rev
Log:
[clangd] Encode Line/Column as a 32-bits integer.
Summary:
This would buy us more memory. Using a 32-bits integer is enough for
most human-readable source code (up to 4M lines and 4K columns).
Previsouly, we used 8 bytes for a position, now 4 bytes, it would save
us 8 bytes for each Ref and each Symbol instance.
For LLVM-project binary index file, we save ~13% memory.
| Before | After |
| 412MB | 355MB |
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet,
cfe-commits
Differential Revision: https://reviews.llvm.org/D53363
Modified:
clang-tools-extra/trunk/clangd/FindSymbols.cpp
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/YAMLSerialization.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/FindSymbols.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.cpp?rev=344735&r1=344734&r2=344735&view=diff
==
--- clang-tools-extra/trunk/clangd/FindSymbols.cpp (original)
+++ clang-tools-extra/trunk/clangd/FindSymbols.cpp Thu Oct 18 03:43:50 2018
@@ -140,10 +140,10 @@ getWorkspaceSymbols(StringRef Query, int
Location L;
L.uri = URIForFile((*Path));
Position Start, End;
-Start.line = CD.Start.Line;
-Start.character = CD.Start.Column;
-End.line = CD.End.Line;
-End.character = CD.End.Column;
+Start.line = CD.Start.line();
+Start.character = CD.Start.column();
+End.line = CD.End.line();
+End.character = CD.End.column();
L.range = {Start, End};
SymbolKind SK = indexSymbolKindToSymbolKind(Sym.SymInfo.Kind);
std::string Scope = Sym.Scope;
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=344735&r1=344734&r2=344735&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Thu Oct 18 03:43:50 2018
@@ -57,10 +57,10 @@ llvm::Optional toLSPLocation(c
}
Location LSPLoc;
LSPLoc.uri = URIForFile(*Path);
- LSPLoc.range.start.line = Loc.Start.Line;
- LSPLoc.range.start.character = Loc.Start.Column;
- LSPLoc.range.end.line = Loc.End.Line;
- LSPLoc.range.end.character = Loc.End.Column;
+ LSPLoc.range.start.line = Loc.Start.line();
+ LSPLoc.range.start.character = Loc.Start.column();
+ LSPLoc.range.end.line = Loc.End.line();
+ LSPLoc.range.end.character = Loc.End.column();
return LSPLoc;
}
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=344735&r1=344734&r2=344735&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Thu Oct 18 03:43:50 2018
@@ -8,6 +8,7 @@
//===--===//
#include "Index.h"
+#include "Logger.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
@@ -18,11 +19,30 @@ namespace clang {
namespace clangd {
using namespace llvm;
+constexpr uint32_t SymbolLocation::Position::MaxLine;
+constexpr uint32_t SymbolLocation::Position::MaxColumn;
+void SymbolLocation::Position::setLine(uint32_t L) {
+ if (L > MaxLine) {
+log("Set an overflowed Line {0}", L);
+Line = MaxLine;
+return;
+ }
+ Line = L;
+}
+void SymbolLocation::Position::setColumn(uint32_t Col) {
+ if (Col > MaxColumn) {
+log("Set an overflowed Column {0}", Col);
+Column = MaxColumn;
+return;
+ }
+ Column = Col;
+}
+
raw_ostream &operator<<(raw_ostream &OS, const SymbolLocation &L) {
if (!L)
return OS << "(none)";
- return OS << L.FileURI << "[" << L.Start.Line << ":" << L.Start.Column << "-"
-<< L.End.Line << ":" << L.End.Column << ")";
+ return OS << L.FileURI << "[" << L.Start.line() << ":" << L.Start.column()
+<< "-" << L.End.line() << ":" << L.End.column() << ")";
}
SymbolID::SymbolID(StringRef USR)
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=344735&r1=344734&r2=344735&view=diff
=
[PATCH] D53363: [clangd] Encode Line/Column as a 32-bits integer.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344735: [clangd] Encode Line/Column as a 32-bits integer.
(authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53363?vs=170053&id=170064#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53363
Files:
clangd/FindSymbols.cpp
clangd/XRefs.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Serialization.cpp
clangd/index/SymbolCollector.cpp
clangd/index/YAMLSerialization.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -32,10 +32,10 @@
using testing::UnorderedElementsAre;
MATCHER_P(RefRange, Range, "") {
- return std::tie(arg.Location.Start.Line, arg.Location.Start.Column,
- arg.Location.End.Line, arg.Location.End.Column) ==
- std::tie(Range.start.line, Range.start.character, Range.end.line,
- Range.end.character);
+ return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),
+ arg.Location.End.line(), arg.Location.End.column()) ==
+ std::make_tuple(Range.start.line, Range.start.character,
+ Range.end.line, Range.end.character);
}
MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
MATCHER_P(DeclURI, U, "") { return arg.CanonicalDeclaration.FileURI == U; }
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -31,13 +31,28 @@
MATCHER_P(Named, N, "") { return arg.Name == N; }
MATCHER_P(RefRange, Range, "") {
- return std::tie(arg.Location.Start.Line, arg.Location.Start.Column,
- arg.Location.End.Line, arg.Location.End.Column) ==
- std::tie(Range.start.line, Range.start.character, Range.end.line,
- Range.end.character);
+ return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),
+ arg.Location.End.line(), arg.Location.End.column()) ==
+ std::make_tuple(Range.start.line, Range.start.character,
+ Range.end.line, Range.end.character);
}
MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+TEST(SymbolLocation, Position) {
+ using Position = SymbolLocation::Position;
+ Position Pos;
+
+ Pos.setLine(1);
+ EXPECT_EQ(1u, Pos.line());
+ Pos.setColumn(2);
+ EXPECT_EQ(2u, Pos.column());
+
+ Pos.setLine(Position::MaxLine + 1); // overflow
+ EXPECT_EQ(Pos.line(), Position::MaxLine);
+ Pos.setColumn(Position::MaxColumn + 1); // overflow
+ EXPECT_EQ(Pos.column(), Position::MaxColumn);
+}
+
TEST(SymbolSlab, FindAndIterate) {
SymbolSlab::Builder B;
B.insert(symbol("Z"));
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -63,19 +63,19 @@
return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
}
MATCHER_P(DeclRange, Pos, "") {
- return std::tie(arg.CanonicalDeclaration.Start.Line,
- arg.CanonicalDeclaration.Start.Column,
- arg.CanonicalDeclaration.End.Line,
- arg.CanonicalDeclaration.End.Column) ==
- std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
- Pos.end.character);
+ return std::make_tuple(arg.CanonicalDeclaration.Start.line(),
+ arg.CanonicalDeclaration.Start.column(),
+ arg.CanonicalDeclaration.End.line(),
+ arg.CanonicalDeclaration.End.column()) ==
+ std::make_tuple(Pos.start.line, Pos.start.character, Pos.end.line,
+ Pos.end.character);
}
MATCHER_P(DefRange, Pos, "") {
- return std::tie(arg.Definition.Start.Line,
- arg.Definition.Start.Column, arg.Definition.End.Line,
- arg.Definition.End.Column) ==
- std::tie(Pos.start.line, Pos.start.character, Pos.end.line,
- Pos.end.character);
+ return std::make_tuple(
+ arg.Definition.Start.line(), arg.Definition.Start.column(),
+ arg.Definition.End.line(), arg.Definition.End.column()) ==
+ std::make_tuple(Pos.start.line, Pos.start.character, Pos.end.line,
+ Pos.end.character);
}
MATCHER_P(RefCount, R, "") { return int(arg.References) == R; }
MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
@@ -86,10 +86,10 @@
MATCHER(RefRange, "") {
const Ref &Pos = testing::get<0>(arg);
const Range &Range = testing::get<1>(arg);
-
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
kristina added a comment. If the author doesn't mind I can just apply the style fix after patching and then rebuild and run all the relevant tests (or would you prefer to do that?). Seems easier than a new revision for an indentation change on one line. https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51633: [ASTImporter] Added error handling for AST import.
balazske added a comment. To the reviewers: Please accept this patch formally if you do not find any problems. This is an intermediate state of the code and there is more work that is dependent on this change. Repository: rC Clang https://reviews.llvm.org/D51633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
lebedev.ri updated this revision to Diff 170065.
lebedev.ri marked 6 inline comments as done.
lebedev.ri added a comment.
- Address nits
- Proper default for CPPCG
- Don't show record name. I indeed don't have a strong case here, it seems to
be more useful to dump that info, but maybe not? ¯\_(ツ)_/¯
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52771
Files:
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -0,0 +1,380 @@
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s cppcoreguidelines-non-private-member-variables-in-classes %t -- --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+
+////
+
+// Only data, do not warn
+
+struct S0 {
+ int S0_v0;
+
+public:
+ int S0_v1;
+
+protected:
+ int S0_v2;
+
+private:
+ int S0_v3;
+};
+
+class S1 {
+ int S1_v0;
+
+public:
+ int S1_v1;
+
+protected:
+ int S1_v2;
+
+private:
+ int S1_v3;
+};
+
+////
+
+// All functions are static, do not warn.
+
+struct S2 {
+ static void S2_m0();
+ int S2_v0;
+
+public:
+ static void S2_m1();
+ int S2_v1;
+
+protected:
+ static void S2_m3();
+ int S2_v2;
+
+private:
+ static void S2_m4();
+ int S2_v3;
+};
+
+class S3 {
+ static void S3_m0();
+ int S3_v0;
+
+public:
+ static void S3_m1();
+ int S3_v1;
+
+protected:
+ static void S3_m3();
+ int S3_v2;
+
+private:
+ static void S3_m4();
+ int S3_v3;
+};
+
+////
+
+// union != struct/class. do not diagnose.
+
+union U0 {
+ void U0_m0();
+ int U0_v0;
+
+public:
+ void U0_m1();
+ int U0_v1;
+
+protected:
+ void U0_m2();
+ int U0_v2;
+
+private:
+ void U0_m3();
+ int U0_v3;
+};
+
+////
+
+// Has non-static method with default visibility.
+
+struct S4 {
+ void S4_m0();
+
+ int S4_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v0' has public visibility
+public:
+ int S4_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v1' has public visibility
+protected:
+ int S4_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S4_v2' has protected visibility
+private:
+ int S4_v3;
+};
+
+class S5 {
+ void S5_m0();
+
+ int S5_v0;
+
+public:
+ int S5_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S5_v1' has public visibility
+protected:
+ int S5_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S5_v2' has protected visibility
+private:
+ int S5_v3;
+};
+
+////
+
+// Has non-static method wi
[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov
calixte added a comment. @vsk, gcc guys are ok for -fprofile-filter-files and -fprofile-exclude-files, are you ok with that ? Should these options prefixed by -Xclang or not ? Repository: rC Clang https://reviews.llvm.org/D52034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53387: [clangd] Lay JSONRPCDispatcher to rest.
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clangd/ClangdLSPServer.cpp:148
+ if (Trace)
+(*Trace)["Reply"] = *Result;
+ Server.reply(ID, json::Value(std::move(*Result)));
do we really want to stash all responses in trace? It sounds like it could get
pretty spammy (e.g. with completion results).
Comment at: clangd/ClangdLSPServer.cpp:184
+ mutable std::mutex RequestCancelersMutex;
+ llvm::StringMap> RequestCancelers;
+ unsigned NextRequestCookie = 0;
nit: while we are here, could you add some documentation for these two fields?
Comment at: clangd/ClangdLSPServer.cpp:409
+
+Reply(std::move(*Items));
+ },
We are not translating internal errors from ClangdServer to LSP errors anymore.
It seems fine and makes code cleaner. Just want to ask if this is intentional.
Comment at: clangd/ClangdLSPServer.h:34
+/// The server also supports $/cancelRequest (MessageHandler provides this).
+class ClangdLSPServer : private DiagnosticsConsumer {
public:
Just out of curiosity, why does `DiagnosticsConsumer` implemented in
ClangdLSPServer instead of ClangdServer?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53387
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.
sammccall created this revision.
sammccall added reviewers: hokein, arphaman.
Herald added subscribers: cfe-commits, kadircet, jkorous, MaskRay, ioeric,
ilya-biryukov.
CodeAction provides us with a standard way of representing fixes inline, so
use it, replacing our existing ad-hoc extension.
After this, it's easy to serialize diagnostics using the structured
toJSON/Protocol.h mechanism rather than assembling JSON ad-hoc.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53391
Files:
clangd/ClangdLSPServer.cpp
clangd/Diagnostics.cpp
clangd/Diagnostics.h
clangd/Protocol.cpp
clangd/Protocol.h
test/clangd/fixits-embed-in-diagnostic.test
unittests/clangd/ClangdUnitTests.cpp
Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -199,11 +199,13 @@
// Transform dianostics and check the results.
std::vector>> LSPDiags;
- toLSPDiags(D, [&](clangd::Diagnostic LSPDiag,
-llvm::ArrayRef Fixes) {
-LSPDiags.push_back({std::move(LSPDiag),
-std::vector(Fixes.begin(), Fixes.end())});
- });
+ toLSPDiags(
+ D, URIForFile("/path/to/foo/bar/main.cpp"), ClangdDiagnosticOptions(),
+ [&](clangd::Diagnostic LSPDiag, llvm::ArrayRef Fixes) {
+LSPDiags.push_back(
+{std::move(LSPDiag),
+ std::vector(Fixes.begin(), Fixes.end())});
+ });
EXPECT_THAT(
LSPDiags,
Index: test/clangd/fixits-embed-in-diagnostic.test
===
--- test/clangd/fixits-embed-in-diagnostic.test
+++ test/clangd/fixits-embed-in-diagnostic.test
@@ -1,12 +1,12 @@
# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"clangdFixSupport":true}}},"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"codeActionsInline":true}}},"trace":"off"}}
---
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}}
-# CHECK:"method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:"params": {
-# CHECK-NEXT: "diagnostics": [
+# CHECK: "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT: "params": {
+# CHECK-NEXT:"diagnostics": [
# CHECK-NEXT: {
-# CHECK-NEXT:"clangd_fixes": [
+# CHECK-NEXT:"codeActions": [
# CHECK-NEXT: {
# CHECK-NEXT:"edit": {
# CHECK-NEXT: "changes": {
@@ -27,6 +27,7 @@
# CHECK-NEXT:]
# CHECK-NEXT: }
# CHECK-NEXT:},
+# CHECK-NEXT:"kind": "quickfix",
# CHECK-NEXT:"title": "change 'union' to 'struct'"
# CHECK-NEXT: }
# CHECK-NEXT:],
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -323,9 +323,8 @@
/// workspace.symbol.symbolKind.valueSet
llvm::Optional WorkspaceSymbolKinds;
- /// Whether the client accepts diagnostics with fixes attached using the
- /// "clangd_fixes" extension.
- /// textDocument.publishDiagnostics.clangdFixSupport
+ /// Whether the client accepts diagnostics with codeActions attached inline.
+ /// textDocument.publishDiagnostics.codeActionsInline.
bool DiagnosticFixes = false;
/// Whether the client accepts diagnostics with category attached to it
@@ -536,6 +535,7 @@
};
bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &);
+struct CodeAction;
struct Diagnostic {
/// The range at which the message applies.
Range range;
@@ -560,7 +560,12 @@
/// An LSP extension that's used to send the name of the category over to the
/// client. The category typically describes the compilation stage during
/// which the issue was produced, e.g. "Semantic Issue" or "Parse Issue".
- std::string category;
+ llvm::Optional category;
+
+ /// Clangd extension: code actions related to this diagnostic.
+ /// Only with capability textDocument.publishDiagnostics.codeActionsInline.
+ /// (These actions can also be obtained using textDocument/codeAction).
+ llvm::Optional> codeActions;
};
llvm::json::Value toJSON(const Diagnostic &);
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -210,8 +210,8 @@
if (auto *Diagnostics = TextDocument->getObject("publishDiagnostics")) {
if (auto CategorySupport = Diagnostics->getBoolean("categorySupport"))
R.DiagnosticCategory = *CategorySupport;
- if (auto ClangdFixSupport = D
[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.
ioeric updated this revision to Diff 170068.
ioeric marked an inline comment as done.
ioeric added a comment.
- add isImplementationDetail helper
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53374
Files:
clangd/AST.cpp
clangd/AST.h
clangd/Quality.cpp
clangd/Quality.h
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
unittests/clangd/QualityTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -83,6 +83,9 @@
IsIndexedForCodeCompletion;
}
MATCHER(Deprecated, "") { return arg.Flags & Symbol::Deprecated; }
+MATCHER(ImplementationDetail, "") {
+ return arg.Flags & Symbol::ImplementationDetail;
+}
MATCHER(RefRange, "") {
const Ref &Pos = testing::get<0>(arg);
const Range &Range = testing::get<1>(arg);
@@ -1037,6 +1040,21 @@
AllOf(QName("TestClangd"), Not(Deprecated();
}
+TEST_F(SymbolCollectorTest, ImplementationDetail) {
+ const std::string Header = R"(
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class Public {};
+ )";
+ runSymbolCollector(Header, /**/ "");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("X_Y_Decl"), ImplementationDetail()),
+ AllOf(QName("Public"), Not(ImplementationDetail();
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -45,17 +45,34 @@
[[deprecated]]
int _f() { return _X; }
+
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class MAC {};
)cpp");
+ Header.ExtraArgs = {"-DMAC=mac_name"};
+
auto Symbols = Header.headerSymbols();
auto AST = Header.build();
SymbolQualitySignals Quality;
Quality.merge(findSymbol(Symbols, "_X"));
EXPECT_FALSE(Quality.Deprecated);
+ EXPECT_FALSE(Quality.ImplementationDetail);
EXPECT_TRUE(Quality.ReservedName);
EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
+ Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
+ Quality.ImplementationDetail = false;
+ Quality.merge(
+ CodeCompletionResult(&findDecl(AST, "mac_name"), /*Priority=*/42));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
Symbol F = findSymbol(Symbols, "_f");
F.References = 24; // TestTU doesn't count references, so fake it.
Quality = {};
@@ -182,6 +199,10 @@
ReservedName.ReservedName = true;
EXPECT_LT(ReservedName.evaluate(), Default.evaluate());
+ SymbolQualitySignals ImplementationDetail;
+ ImplementationDetail.ImplementationDetail = true;
+ EXPECT_LT(ImplementationDetail.evaluate(), Default.evaluate());
+
SymbolQualitySignals WithReferences, ManyReferences;
WithReferences.References = 20;
ManyReferences.References = 1000;
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -536,6 +536,8 @@
if (isIndexedForCodeCompletion(ND, Ctx))
S.Flags |= Symbol::IndexedForCodeCompletion;
+ if (isImplementationDetail(&ND))
+S.Flags |= Symbol::ImplementationDetail;
S.SymInfo = index::getSymbolInfo(&ND);
std::string FileURI;
if (auto DeclLoc = getTokenLocation(findNameLoc(&ND), SM, Opts,
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -244,6 +244,8 @@
IndexedForCodeCompletion = 1 << 0,
/// Indicates if the symbol is deprecated.
Deprecated = 1 << 1,
+// Symbol is an implementation detail.
+ImplementationDetail = 1 << 2,
};
SymbolFlag Flags = SymbolFlag::None;
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -57,6 +57,7 @@
bool Deprecated = false;
bool ReservedName = false; // __foo, _Foo are usually implementation details.
// FIXME: make these findable once user types _.
+ bool ImplementationDetail = false;
unsigned References = 0;
enum SymbolCategory {
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -7,6 +7,7 @@
//
//===--===//
#include "Quality.h"
+#include "AST.h"
#include "FileDistance.h"
#include "
[clang-tools-extra] r344736 - [clangd] Names that are not spelled in source code are reserved.
Author: ioeric
Date: Thu Oct 18 05:23:05 2018
New Revision: 344736
URL: http://llvm.org/viewvc/llvm-project?rev=344736&view=rev
Log:
[clangd] Names that are not spelled in source code are reserved.
Summary:
These are often not expected to be used directly e.g.
```
TEST_F(Fixture, X) {
^ // "Fixture_X_Test" expanded in the macro should be down ranked.
}
```
Only doing this for sema for now, as such symbols are mostly coming from sema
e.g. gtest macros expanded in the main file. We could also add a similar field
for the index symbol.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53374
Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=344736&r1=344735&r2=344736&view=diff
==
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Thu Oct 18 05:23:05 2018
@@ -11,6 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Index/USRGeneration.h"
@@ -18,25 +19,34 @@ namespace clang {
namespace clangd {
using namespace llvm;
-SourceLocation findNameLoc(const clang::Decl* D) {
- const auto& SM = D->getASTContext().getSourceManager();
+// Returns true if the complete name of decl \p D is spelled in the source
code.
+// This is not the case for
+// * symbols formed via macro concatenation, the spelling location will
+// be ""
+// * symbols controlled and defined by a compile command-line option
+// `-DName=foo`, the spelling location will be "".
+bool isSpelledInSourceCode(const Decl *D) {
+ const auto &SM = D->getASTContext().getSourceManager();
+ auto Loc = D->getLocation();
// FIXME: Revisit the strategy, the heuristic is limitted when handling
// macros, we should use the location where the whole definition occurs.
- SourceLocation SpellingLoc = SM.getSpellingLoc(D->getLocation());
- if (D->getLocation().isMacroID()) {
-std::string PrintLoc = SpellingLoc.printToString(SM);
+ if (Loc.isMacroID()) {
+std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
if (llvm::StringRef(PrintLoc).startswith("")) {
- // We use the expansion location for the following symbols, as spelling
- // locations of these symbols are not interesting to us:
- // * symbols formed via macro concatenation, the spelling location will
- // be ""
- // * symbols controlled and defined by a compile command-line option
- // `-DName=foo`, the spelling location will be "".
- SpellingLoc = SM.getExpansionRange(D->getLocation()).getBegin();
-}
+llvm::StringRef(PrintLoc).startswith(""))
+ return false;
}
- return SpellingLoc;
+ return true;
+}
+
+bool isImplementationDetail(const Decl *D) { return !isSpelledInSourceCode(D);
}
+
+SourceLocation findNameLoc(const clang::Decl* D) {
+ const auto &SM = D->getASTContext().getSourceManager();
+ if (!isSpelledInSourceCode(D))
+// Use the expansion location as spelling location is not interesting.
+return SM.getExpansionRange(D->getLocation()).getBegin();
+ return SM.getSpellingLoc(D->getLocation());
}
std::string printQualifiedName(const NamedDecl &ND) {
Modified: clang-tools-extra/trunk/clangd/AST.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=344736&r1=344735&r2=344736&view=diff
==
--- clang-tools-extra/trunk/clangd/AST.h (original)
+++ clang-tools-extra/trunk/clangd/AST.h Thu Oct 18 05:23:05 2018
@@ -24,6 +24,11 @@ class Decl;
namespace clangd {
+/// Returns true if the declaration is considered implementation detail based
on
+/// heuristics. For example, a declaration whose name is not explicitly spelled
+/// in code is considered implementation detail.
+bool isImplementationDetail(const Decl *D);
+
/// Find the identifier source location of the given D.
///
/// The returned location is usually the spelling location where the name of
the
Modified: clang-tools-extra/trunk/clangd/Quality.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=344736&r1=344735&r2=344736&view=diff
=
[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344736: [clangd] Names that are not spelled in source code
are reserved. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53374
Files:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -83,6 +83,9 @@
IsIndexedForCodeCompletion;
}
MATCHER(Deprecated, "") { return arg.Flags & Symbol::Deprecated; }
+MATCHER(ImplementationDetail, "") {
+ return arg.Flags & Symbol::ImplementationDetail;
+}
MATCHER(RefRange, "") {
const Ref &Pos = testing::get<0>(arg);
const Range &Range = testing::get<1>(arg);
@@ -1037,6 +1040,21 @@
AllOf(QName("TestClangd"), Not(Deprecated();
}
+TEST_F(SymbolCollectorTest, ImplementationDetail) {
+ const std::string Header = R"(
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class Public {};
+ )";
+ runSymbolCollector(Header, /**/ "");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("X_Y_Decl"), ImplementationDetail()),
+ AllOf(QName("Public"), Not(ImplementationDetail();
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
@@ -45,17 +45,34 @@
[[deprecated]]
int _f() { return _X; }
+
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class MAC {};
)cpp");
+ Header.ExtraArgs = {"-DMAC=mac_name"};
+
auto Symbols = Header.headerSymbols();
auto AST = Header.build();
SymbolQualitySignals Quality;
Quality.merge(findSymbol(Symbols, "_X"));
EXPECT_FALSE(Quality.Deprecated);
+ EXPECT_FALSE(Quality.ImplementationDetail);
EXPECT_TRUE(Quality.ReservedName);
EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
+ Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
+ Quality.ImplementationDetail = false;
+ Quality.merge(
+ CodeCompletionResult(&findDecl(AST, "mac_name"), /*Priority=*/42));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
Symbol F = findSymbol(Symbols, "_f");
F.References = 24; // TestTU doesn't count references, so fake it.
Quality = {};
@@ -182,6 +199,10 @@
ReservedName.ReservedName = true;
EXPECT_LT(ReservedName.evaluate(), Default.evaluate());
+ SymbolQualitySignals ImplementationDetail;
+ ImplementationDetail.ImplementationDetail = true;
+ EXPECT_LT(ImplementationDetail.evaluate(), Default.evaluate());
+
SymbolQualitySignals WithReferences, ManyReferences;
WithReferences.References = 20;
ManyReferences.References = 1000;
Index: clang-tools-extra/trunk/clangd/Quality.h
===
--- clang-tools-extra/trunk/clangd/Quality.h
+++ clang-tools-extra/trunk/clangd/Quality.h
@@ -57,6 +57,7 @@
bool Deprecated = false;
bool ReservedName = false; // __foo, _Foo are usually implementation details.
// FIXME: make these findable once user types _.
+ bool ImplementationDetail = false;
unsigned References = 0;
enum SymbolCategory {
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -536,6 +536,8 @@
if (isIndexedForCodeCompletion(ND, Ctx))
S.Flags |= Symbol::IndexedForCodeCompletion;
+ if (isImplementationDetail(&ND))
+S.Flags |= Symbol::ImplementationDetail;
S.SymInfo = index::getSymbolInfo(&ND);
std::string FileURI;
if (auto DeclLoc = getTokenLocation(findNameLoc(&ND), SM, Opts,
Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
nruslan added a comment. In https://reviews.llvm.org/D53102#1268272, @kristina wrote: > If the author doesn't mind I can just apply the style fix after patching and > then rebuild and run all the relevant tests (or would you prefer to do > that?). Seems easier than a new revision for an indentation change on one > line. @kristina : Thank you! Sure, that is fine. Go ahead. https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53387: [clangd] Lay JSONRPCDispatcher to rest.
sammccall marked an inline comment as done.
sammccall added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:148
+ if (Trace)
+(*Trace)["Reply"] = *Result;
+ Server.reply(ID, json::Value(std::move(*Result)));
ioeric wrote:
> do we really want to stash all responses in trace? It sounds like it could
> get pretty spammy (e.g. with completion results).
It's a fair point, I don't know the answer. I know I've looked at the replies,
but not all that often.
It's not new behavior though (it used to be in JSONRPCDispatcher), and I'd
rather not change it in this patch.
Comment at: clangd/ClangdLSPServer.cpp:409
+
+Reply(std::move(*Items));
+ },
ioeric wrote:
> We are not translating internal errors from ClangdServer to LSP errors
> anymore. It seems fine and makes code cleaner. Just want to ask if this is
> intentional.
We used to explicitly mark some of these with the "internal error" code, and
others with the "unknown error" code, without much consistency as to why.
I've tried to preserve the error codes that were semantically significant (e.g.
invalid params).
Comment at: clangd/ClangdLSPServer.h:34
+/// The server also supports $/cancelRequest (MessageHandler provides this).
+class ClangdLSPServer : private DiagnosticsConsumer {
public:
ioeric wrote:
> Just out of curiosity, why does `DiagnosticsConsumer` implemented in
> ClangdLSPServer instead of ClangdServer?
This is `clangd::DiagnosticsConsumer`, it's exactly the callback that
ClangdServer uses to send diagnostics to ClangdLSPServer.
So ClangdServer doesn't implement it but rather accepts it, because it doesn't
know what to do with the diagnostics.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53387
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344737 - [clangd] Lay JSONRPCDispatcher to rest.
Author: sammccall
Date: Thu Oct 18 05:32:04 2018
New Revision: 344737
URL: http://llvm.org/viewvc/llvm-project?rev=344737&view=rev
Log:
[clangd] Lay JSONRPCDispatcher to rest.
Summary:
Most of its functionality is moved into ClangdLSPServer.
The decoupling between JSONRPCDispatcher, ProtocolCallbacks, ClangdLSPServer
was never real, and only served to obfuscate.
Some previous implicit/magic stuff is now explicit:
- the return type of LSP method calls are now in the signature
- no more reply() that gets the ID using global context magic
- arg tracing no longer relies on RequestArgs::stash context magic either
This is mostly refactoring, but some deliberate fixes while here:
- LSP method params are now by const reference
- notifications and calls are now distinct namespaces.
(some tests had protocol errors and needed updating)
- we now reply to calls we failed to decode
- outgoing calls use distinct IDs
A few error codes and message IDs changed in unimportant ways (see tests).
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman,
jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53387
Removed:
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/test/clangd/crash-non-added-files.test
clang-tools-extra/trunk/test/clangd/delimited-input-comment-at-the-end.test
clang-tools-extra/trunk/test/clangd/fixits-command.test
clang-tools-extra/trunk/test/clangd/rename.test
clang-tools-extra/trunk/test/clangd/spaces-in-delimited-input.test
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=344737&r1=344736&r2=344737&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Oct 18 05:32:04 2018
@@ -25,11 +25,9 @@ add_clang_library(clangDaemon
FuzzyMatch.cpp
GlobalCompilationDatabase.cpp
Headers.cpp
- JSONRPCDispatcher.cpp
JSONTransport.cpp
Logger.cpp
Protocol.cpp
- ProtocolHandlers.cpp
Quality.cpp
RIFF.cpp
SourceCode.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344737&r1=344736&r2=344737&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct 18 05:32:04 2018
@@ -9,13 +9,14 @@
#include "ClangdLSPServer.h"
#include "Diagnostics.h"
-#include "JSONRPCDispatcher.h"
#include "SourceCode.h"
+#include "Trace.h"
#include "URI.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
using namespace clang::clangd;
using namespace clang;
@@ -80,7 +81,179 @@ CompletionItemKindBitset defaultCompleti
} // namespace
-void ClangdLSPServer::onInitialize(InitializeParams &Params) {
+// MessageHandler dispatches incoming LSP messages.
+// It handles cross-cutting concerns:
+// - serializes/deserializes protocol objects to JSON
+// - logging of inbound messages
+// - cancellation handling
+// - basic call tracing
+class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
+public:
+ MessageHandler(ClangdLSPServer &Server) : Server(Server) {}
+
+ bool onNotify(StringRef Method, json::Value Params) override {
+log("<-- {0}", Method);
+if (Method == "exit")
+ return false;
+if (Method == "$/cancelRequest")
+ onCancel(std::move(Params));
+else if (auto Handler = Notifications.lookup(Method))
+ Handler(std::move(Params));
+else
+ log("unhandled notification {0}", Method);
+return true;
+ }
+
+ bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
+log("<-- {0}({1})", Method, ID);
+if (auto Handler = Calls.lookup(Method))
+ Handler(std::move(Params), std::move(ID));
+else
+ Server.reply(ID, llvm::make_error("method not found",
+ ErrorCode::MethodNotFound));
+return true;
+ }
+
+ bool onReply(json::Value ID, Expected Result) override {
+// We ignore replies, just log them.
+if (Result)
+ log("<-- reply({0})", ID);
+else
+
[PATCH] D53387: [clangd] Lay JSONRPCDispatcher to rest.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344737: [clangd] Lay JSONRPCDispatcher to rest. (authored
by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53387?vs=170038&id=170072#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53387
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatcher.h
clangd/ProtocolHandlers.cpp
clangd/ProtocolHandlers.h
clangd/TUScheduler.cpp
clangd/tool/ClangdMain.cpp
test/clangd/crash-non-added-files.test
test/clangd/delimited-input-comment-at-the-end.test
test/clangd/fixits-command.test
test/clangd/rename.test
test/clangd/spaces-in-delimited-input.test
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -9,13 +9,14 @@
#include "ClangdLSPServer.h"
#include "Diagnostics.h"
-#include "JSONRPCDispatcher.h"
#include "SourceCode.h"
+#include "Trace.h"
#include "URI.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
using namespace clang::clangd;
using namespace clang;
@@ -80,7 +81,179 @@
} // namespace
-void ClangdLSPServer::onInitialize(InitializeParams &Params) {
+// MessageHandler dispatches incoming LSP messages.
+// It handles cross-cutting concerns:
+// - serializes/deserializes protocol objects to JSON
+// - logging of inbound messages
+// - cancellation handling
+// - basic call tracing
+class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
+public:
+ MessageHandler(ClangdLSPServer &Server) : Server(Server) {}
+
+ bool onNotify(StringRef Method, json::Value Params) override {
+log("<-- {0}", Method);
+if (Method == "exit")
+ return false;
+if (Method == "$/cancelRequest")
+ onCancel(std::move(Params));
+else if (auto Handler = Notifications.lookup(Method))
+ Handler(std::move(Params));
+else
+ log("unhandled notification {0}", Method);
+return true;
+ }
+
+ bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
+log("<-- {0}({1})", Method, ID);
+if (auto Handler = Calls.lookup(Method))
+ Handler(std::move(Params), std::move(ID));
+else
+ Server.reply(ID, llvm::make_error("method not found",
+ ErrorCode::MethodNotFound));
+return true;
+ }
+
+ bool onReply(json::Value ID, Expected Result) override {
+// We ignore replies, just log them.
+if (Result)
+ log("<-- reply({0})", ID);
+else
+ log("<-- reply({0}) error: {1}", ID, llvm::toString(Result.takeError()));
+return true;
+ }
+
+ // Bind an LSP method name to a call.
+ template
+ void bind(const char *Method,
+void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
+Calls[Method] = [Method, Handler, this](json::Value RawParams,
+json::Value ID) {
+ Param P;
+ if (!fromJSON(RawParams, P)) {
+elog("Failed to decode {0} request.", Method);
+Server.reply(ID, make_error("failed to decode request",
+ ErrorCode::InvalidRequest));
+return;
+ }
+ trace::Span Tracer(Method);
+ SPAN_ATTACH(Tracer, "Params", RawParams);
+ auto *Trace = Tracer.Args; // We attach reply from another thread.
+ // Calls can be canceled by the client. Add cancellation context.
+ WithContext WithCancel(cancelableRequestContext(ID));
+ // FIXME: this function should assert it's called exactly once.
+ (Server.*Handler)(P, [this, ID, Trace](llvm::Expected Result) {
+if (Result) {
+ if (Trace)
+(*Trace)["Reply"] = *Result;
+ Server.reply(ID, json::Value(std::move(*Result)));
+} else {
+ auto Err = Result.takeError();
+ if (Trace)
+(*Trace)["Error"] = llvm::to_string(Err);
+ Server.reply(ID, std::move(Err));
+}
+ });
+};
+ }
+
+ // Bind an LSP method name to a notification.
+ template
+ void bind(const char *Method,
+void (ClangdLSPServer::*Handler)(const Param &)) {
+Notifications[Method] = [Method, Handler, this](json::Value RawParams) {
+ Param P;
+ if (!fromJSON(RawParams, P)) {
+elog("Failed to decode {0} request.", Method);
+return;
+ }
+ trace::Span Tracer(Method);
+ SPAN_ATTACH(Tracer, "Params", RawParams);
+ (Server.*Handler)(P);
+};
+ }
+
+private:
+ llvm::StringMap> Notifications;
+ llvm::StringMap> Calls;
+
+ // Method calls may be cancelled by ID, so keep track of their state.
+ // This needs a mutex: handlers may finish on a different
[PATCH] D53392: [RISCV] Collect library directories and triples for riscv64 triple too
edward-jones created this revision.
edward-jones added a reviewer: asb.
Herald added subscribers: cfe-commits, jocewei, PkmX, rkruppe, the_o,
brucehoult, MartinMosbeck, rogfer01, mgrang, zzheng, jrtc27, shiva0217,
kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar, arichardson,
emaste.
Herald added a reviewer: espindola.
When setting up library and tools paths when detecting an accompanying GCC
installation only riscv32 was handled. As a consequence when targetting riscv64
neither the linker nor libraries would be found. This adds handling and tests
for riscv64.
Repository:
rC Clang
https://reviews.llvm.org/D53392
Files:
lib/Driver/ToolChains/Gnu.cpp
test/Driver/Inputs/basic_riscv64_tree/bin/riscv64-unknown-elf-ld
test/Driver/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/crtbegin.o
test/Driver/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/crtend.o
test/Driver/Inputs/basic_riscv64_tree/riscv64-unknown-elf/include/c++/8.0.1/.keep
test/Driver/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib/crt0.o
test/Driver/riscv64-toolchain.c
Index: test/Driver/riscv64-toolchain.c
===
--- test/Driver/riscv64-toolchain.c
+++ test/Driver/riscv64-toolchain.c
@@ -3,6 +3,102 @@
// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 2>&1 | FileCheck -check-prefix=CC1 %s
// CC1: clang{{.*}} "-cc1" "-triple" "riscv64"
+// RUN: %clang %s -### -no-canonical-prefixes \
+// RUN: -target riscv64-unknown-elf \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN: --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 \
+// RUN: | FileCheck -check-prefix=C-RV64-BAREMETAL-LP64 %s
+
+// C-RV64-BAREMETAL-LP64: "-fuse-init-array"
+// C-RV64-BAREMETAL-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../bin{{/|}}riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-LP64: "--sysroot={{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf"
+// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib{{/|}}crt0.o"
+// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
+// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
+
+// RUN: %clang %s -### -no-canonical-prefixes \
+// RUN: -target riscv64-unknown-elf \
+// RUN: --sysroot= \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
+// RUN: | FileCheck -check-prefix=C-RV64-BAREMETAL-NOSYSROOT-LP64 %s
+
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-fuse-init-array"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../bin{{/|}}riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../riscv64-unknown-elf/lib{{/|}}crt0.o"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../riscv64-unknown-elf{{/|}}lib"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
+
+// RUN: %clangxx %s -### -no-canonical-prefixes \
+// RUN: -target riscv64-unknown-elf -stdlib=libstdc++ \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN: --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 \
+// RUN: | FileCheck -check-prefix=CXX-RV64-BAREMETAL-LP64 %s
+
+// CXX-RV64-BAREMETAL-LP64: "-fuse-init-array"
+// CXX-RV64-BAREMETAL-LP64: "-internal-isystem" "{{.*}}Inputs/basic_riscv64_tree/riscv64-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV64-BAREMETAL-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../bin{{/|}}riscv64-unknown-elf-ld"
+// CXX-RV64-BAREMETAL-LP64: "--sysroot={{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf"
+// CXX-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib{{/|}}crt0.o"
+// CXX-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// CXX-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
+// CXX-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+/
[PATCH] D52988: [analyzer][PlistMacroExpansion] Part 5.: Support for # and ##
Szelethus updated this revision to Diff 170073.
Szelethus added a comment.
Fixed an issue where if `##` had spaces around it, those spaces weren't removed.
https://reviews.llvm.org/D52988
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -278,9 +278,17 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// TODO: Should expand correctly.
// CHECK: nameDECLARE_FUNC_AND_SET_TO_NULL
-// CHECK: expansionvoid generated_##whatever(); ptr = nullptr;
+// CHECK: expansionvoid generated_whatever(); ptr = nullptr;
+
+void macroArgContainsHashHashInStringTest() {
+ int *a;
+ TO_NULL_AND_PRINT(a, "Will this ## cause a crash?");
+ *a = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: nameTO_NULL_AND_PRINT
+// CHECK: expansiona = 0; print( "Will this ## cause a crash?")
#define PRINT_STR(str, ptr) \
print(#str); \
@@ -292,6 +300,30 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// TODO: Should expand correctly.
// CHECK: namePRINT_STR
-// CHECK: expansionprint(#Hello); ptr = nullptr
+// CHECK: expansionprint("Hello"); ptr = nullptr
+
+void macroArgContainsHashInStringTest() {
+ int *a;
+ TO_NULL_AND_PRINT(a, "Will this # cause a crash?");
+ *a = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: nameTO_NULL_AND_PRINT
+// CHECK: expansiona = 0; print( "Will this # cause a crash?")
+
+#define CONCAT(a, b, c) \
+ a ## _ ## b ## _ ## c
+
+void CONCAT(llvm, clang, ento)(int **ptr) {
+ *ptr = nullptr;
+}
+
+void spaceAroundHashHashTest() {
+ int* a;
+ CONCAT(llvm, clang, ento)(&a);
+ *a = 3; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: nameCONCAT
+// CHECK: expansionllvm_clang_ento
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -3755,7 +3755,7 @@
file0
nameDECLARE_FUNC_AND_SET_TO_NULL
- expansionvoid generated_##whatever(); ptr = nullptr;
+ expansionvoid generated_whatever(); ptr = nullptr;
kindevent
@@ -3887,25 +3887,192 @@
start
- line290
+ line285
col3
file0
- line290
+ line285
col5
file0
end
- line291
+ line286
col3
file0
- line291
+ line286
+ col19
+ file0
+
+
+
+
+
+
+ kindmacro_expansion
+ location
+
+ line286
+ col3
+ file0
+
+ nameTO_NULL_AND_PRINT
+ expansiona = 0; print( "Will this ## cause a crash?")
+
+
+ kindevent
+ location
+
+ line286
+ col3
+ file0
+
+ ranges
+
+
+
+ line286
+ col3
+ file0
+
+
+ line286
+ col53
+ file0
+
+
+
+ depth0
+ extended_message
+ Null pointer value stored to 'a'
+ message
+ Null pointer value stored to 'a'
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line287
+ col3
+ file0
+
+
+ line287
+ col3
+ file0
+
+
+end
+
+
+ line287
+ col6
+ file0
+
+
+ line287
+ col6
+ file0
+
+
+
+
+
+
+ kindevent
+ location
+
+ line287
+ col6
+ file0
+
+ ranges
+
+
+
+ line287
+ col4
+ file0
+
+
+ line287
+ col4
+ file0
+
+
+
+ depth0
+ extended_message
+ Dereference of null pointer (loaded from variable 'a')
+ message
+ Dereference of null pointer (loaded from variable 'a')
+
+
+ descriptionDereference of null pointer (loaded from variable 'a')
+ categoryLogic error
+ typeDereference of null pointer
+ check_namecore.NullDereference
+
+ issue_hash_content_of_line_in_context6817572ced27cb7d28fc87b2aba75fb4
+ issue_context_kindfunction
+ issue_contextmacroArgContainsHashHashInStringTest
+ issue_ha
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > - Apply minor wording nits. > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not **anything** > else (not even `llu`). I'll find out about the DCL16-C recommendation, as I suspect the intent is to cover `lu` and `llu` but not `ul` and `ull`. I've pinged the authors and can hopefully get an answer quickly, but if not, I'm fine with fixing that after the patch goes in. Comment at: clang-tidy/cert/CERTTidyModule.cpp:87 +ClangTidyOptions Options; +auto &Opts = Options.CheckOptions; +Opts["cert-dcl16-c.NewSuffixes"] = "L;LL"; Don't use `auto` here. Comment at: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst:6 + +`cert-dcl16-c` redirects here as an alias for this check. + You should consider calling out the CERT behavioral differences. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
kristina added a comment. By the way, out of curiosity is this for anything specific (alternative libc or some user-mode-scheduling implementation)? Not nitpicking, just curious since it's an interesting topic in general and it's frustrating that the architecture is so limited in terms of registers that can be used for TLS/related data unlike newer ARM/ARM64 architectures. Also FWIW `%gs` is generally free to use under x86_64 Linux which is where I usually place my thread control blocks which doesn't interfere with libc which uses `%fs`. (Just started build/test job, waiting on that for now) https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor nit. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 +ClangTidyOptions Options; +auto &Opts = Options.CheckOptions; + Don't use `auto` here. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
lebedev.ri added a comment. In https://reviews.llvm.org/D52771#1268367, @aaron.ballman wrote: > LGTM aside from a minor nit. Thank you for the review! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > > > - Apply minor wording nits. > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not > > **anything** else (not even `llu`). > > > I'll find out about the DCL16-C recommendation, as I suspect the intent is to > cover `lu` and `llu` but not `ul` and `ull`. I agree, i've thought so too. That will open an interesting question: in `lu`, `l` should be upper-case. What about `u`? We can't keep it as-is. We will either consistently upper-case it, or consistently lower-case it. I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`? > I've pinged the authors Thank you. > and can hopefully get an answer quickly, but if not, I'm fine with fixing > that after the patch goes in. Thank you. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53395: [OPENMP] Move OMPClausePrinter to OpenMPClause.h/OpenMPClause.cpp - NFC
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang https://reviews.llvm.org/D53395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53397: [MinGW] Link to correct openmp library
SquallATF created this revision.
Herald added subscribers: cfe-commits, guansong.
Repository:
rC Clang
https://reviews.llvm.org/D53397
Files:
lib/Driver/ToolChains/MinGW.cpp
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -220,8 +220,24 @@
CmdArgs.push_back("-lssp_nonshared");
CmdArgs.push_back("-lssp");
}
- if (Args.hasArg(options::OPT_fopenmp))
-CmdArgs.push_back("-lgomp");
+
+ if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+ options::OPT_fno_openmp, false)) {
+switch (TC.getDriver().getOpenMPRuntime(Args)) {
+case Driver::OMPRT_OMP:
+ CmdArgs.push_back("-lomp");
+ break;
+case Driver::OMPRT_IOMP5:
+ CmdArgs.push_back("-liomp5md");
+ break;
+case Driver::OMPRT_GOMP:
+ CmdArgs.push_back("-lgomp");
+ break;
+case Driver::OMPRT_Unknown:
+ // Already diagnosed.
+ break;
+}
+ }
AddLibGCC(Args, CmdArgs);
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -220,8 +220,24 @@
CmdArgs.push_back("-lssp_nonshared");
CmdArgs.push_back("-lssp");
}
- if (Args.hasArg(options::OPT_fopenmp))
-CmdArgs.push_back("-lgomp");
+
+ if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+ options::OPT_fno_openmp, false)) {
+switch (TC.getDriver().getOpenMPRuntime(Args)) {
+case Driver::OMPRT_OMP:
+ CmdArgs.push_back("-lomp");
+ break;
+case Driver::OMPRT_IOMP5:
+ CmdArgs.push_back("-liomp5md");
+ break;
+case Driver::OMPRT_GOMP:
+ CmdArgs.push_back("-lgomp");
+ break;
+case Driver::OMPRT_Unknown:
+ // Already diagnosed.
+ break;
+}
+ }
AddLibGCC(Args, CmdArgs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53398: [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
LSP is a slightly awkward map to C++ object lifetimes: the initialize request
is part of the protocol and provides information that doesn't change over the
lifetime of the server.
Until now, we handled this by initializing ClangdServer and ClangdLSPServer
right away, and making anything that can be set in the "initialize" request
mutable.
With this patch, we create ClangdLSPServer immediately, but defer creating
ClangdServer until "initialize". This opens the door to passing the relevant
initialize params in the constructor and storing them immutably.
(That change isn't actually done in this patch).
To make this safe, we have the MessageDispatcher enforce that the "initialize"
method is called before any other (as required by LSP). That way each method
handler can assume Server is initialized, as today.
As usual, while implementing this I found places where our test cases violated
the protocol.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53398
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
test/clangd/exit-with-shutdown.test
test/clangd/exit-without-shutdown.test
test/clangd/initialize-sequence.test
Index: test/clangd/initialize-sequence.test
===
--- /dev/null
+++ test/clangd/initialize-sequence.test
@@ -0,0 +1,21 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"workspace/symbol","params":{"query":"std::basic_ostringstream"}}
+# CHECK: "error": {
+# CHECK-NEXT:"code": -32002
+# CHECK-NEXT:"message": "server not initialized"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 0,
+---
+{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","id":2,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+# CHECK: "error": {
+# CHECK-NEXT:"code": -32600
+# CHECK-NEXT:"message": "server already initialized"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 2,
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: test/clangd/exit-without-shutdown.test
===
--- test/clangd/exit-without-shutdown.test
+++ test/clangd/exit-without-shutdown.test
@@ -1,2 +1,4 @@
# RUN: not clangd -lit-test < %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/exit-with-shutdown.test
===
--- test/clangd/exit-with-shutdown.test
+++ test/clangd/exit-with-shutdown.test
@@ -1,4 +1,6 @@
# RUN: clangd -lit-test < %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
---
{"jsonrpc":"2.0","method":"exit"}
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -90,7 +90,7 @@
/// defaults and -resource-dir compiler flag).
/// If None, ClangdServer calls CompilerInvocation::GetResourcePath() to
/// obtain the standard resource directory.
-llvm::Optional ResourceDir = llvm::None;
+llvm::Optional ResourceDir = llvm::None;
/// Time to wait after a new file version before computing diagnostics.
std::chrono::steady_clock::duration UpdateDebounce =
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -103,7 +103,7 @@
DiagnosticsConsumer &DiagConsumer,
const Options &Opts)
: CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
- ResourceDir(Opts.ResourceDir ? Opts.ResourceDir->str()
+ ResourceDir(Opts.ResourceDir ? *Opts.ResourceDir
: getStandardResourceDir()),
DynamicIdx(Opts.BuildDynamicSymbolIndex
? new FileIndex(Opts.URISchemes,
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -183,12 +183,10 @@
// Store of the current versions of the open documents.
DraftStore DraftMgr;
- // Server must be the last member of the class to allow its destructor to exit
- // the worker thread that may otherwise run an async callback on partially
- // destructed instance of ClangdLSPServer.
- // Set in construtor and destroye
[PATCH] D53398: [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Looks good! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
nruslan added a comment. In https://reviews.llvm.org/D53102#1268364, @kristina wrote: > By the way, out of curiosity is this for anything specific (alternative libc > or some user-mode-scheduling implementation)? Not nitpicking, just curious > since it's an interesting topic in general and it's frustrating that the > architecture is so limited in terms of registers that can be used for > TLS/related data unlike newer ARM/ARM64 architectures. > > Also FWIW `%gs` is generally free to use under x86_64 Linux which is where I > usually place my thread control blocks which doesn't interfere with libc > which uses `%fs`. (Just started build/test job, waiting on that for now) @kristina : Yes, there are some recent use cases mentioned in the bug report (see the link above). Also, I know that it was used for x86 (32-bit only) Xen guests where it would be recommended to compile an OS with this flag. I also used it in my fork of the NPTL pthread library in VirtuOS to support M:N threading model: http://sigops.org/sosp/sosp13/papers/p116-nikolaev.pdf https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344739 - Add support for -mno-tls-direct-seg-refs to Clang
Author: kristina
Date: Thu Oct 18 07:07:02 2018
New Revision: 344739
URL: http://llvm.org/viewvc/llvm-project?rev=344739&view=rev
Log:
Add support for -mno-tls-direct-seg-refs to Clang
This patch exposes functionality added in rL344723 to the Clang driver/frontend
as a flag and adds appropriate metadata.
Driver tests pass:
```
ninja check-clang-driver
-snip-
Expected Passes: 472
Expected Failures : 3
Unsupported Tests : 65
```
Odd failure in CodeGen tests but unrelated to this:
```
ninja check-clang-codegen
-snip-
/SourceCache/llvm-trunk-8.0/tools/clang/test/CodeGen/builtins-wasm.c:87:10:
error: cannot compile this builtin function yet
-snip-
Failing Tests (1):
Clang :: CodeGen/builtins-wasm.c
Expected Passes: 1250
Expected Failures : 2
Unsupported Tests : 120
Unexpected Failures: 1
```
Original commit:
[X86] Support for the mno-tls-direct-seg-refs flag
Allows to disable direct TLS segment access (%fs or %gs). GCC supports a
similar flag, it can be useful in some circumstances, e.g. when a thread
context block needs to be updated directly from user space. More info and
specific use cases: https://bugs.llvm.org/show_bug.cgi?id=16145
Patch by nruslan (Ruslan Nikolaev).
Differential Revision: https://reviews.llvm.org/D53102
Added:
cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c
cfe/trunk/test/Driver/indirect-tls-seg-refs.c
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=344739&r1=344738&r2=344739&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Oct 18 07:07:02 2018
@@ -2011,6 +2011,8 @@ def mno_global_merge : Flag<["-"], "mno-
def mno_pascal_strings : Flag<["-"], "mno-pascal-strings">,
Alias;
def mno_red_zone : Flag<["-"], "mno-red-zone">, Group;
+def mno_tls_direct_seg_refs : Flag<["-"], "mno-tls-direct-seg-refs">,
Group, Flags<[CC1Option]>,
+ HelpText<"Disable direct TLS access through segment registers">;
def mno_relax_all : Flag<["-"], "mno-relax-all">, Group;
def mno_rtd: Flag<["-"], "mno-rtd">, Group;
def mno_soft_float : Flag<["-"], "mno-soft-float">, Group;
@@ -2171,6 +2173,8 @@ def momit_leaf_frame_pointer : Flag<["-"
def moslib_EQ : Joined<["-"], "moslib=">, Group;
def mpascal_strings : Flag<["-"], "mpascal-strings">, Alias;
def mred_zone : Flag<["-"], "mred-zone">, Group;
+def mtls_direct_seg_refs : Flag<["-"], "mtls-direct-seg-refs">, Group,
+ HelpText<"Enable direct TLS access through segment registers (default)">;
def mregparm_EQ : Joined<["-"], "mregparm=">, Group;
def mrelax_all : Flag<["-"], "mrelax-all">, Group,
Flags<[CC1Option,CC1AsOption]>,
HelpText<"(integrated-as) Relax all machine instructions">;
Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=344739&r1=344738&r2=344739&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Thu Oct 18 07:07:02 2018
@@ -62,6 +62,8 @@ CODEGENOPT(ExperimentalNewPassManager, 1
CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
+ ///< is specified.
CODEGENOPT(DisableTailCalls , 1, 0) ///< Do not emit tail calls.
CODEGENOPT(NoEscapingBlockTailCalls, 1, 0) ///< Do not emit tail calls from
///< escaping blocks.
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=344739&r1=344738&r2=344739&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Oct 18 07:07:02 2018
@@ -1709,6 +1709,8 @@ void CodeGenModule::ConstructDefaultFnAt
if (CodeGenOpts.DisableRedZone)
FuncAttrs.addAttribute(llvm::Attribute::NoRedZone);
+ if (CodeGenOpts.IndirectTlsSegRefs)
+FuncAttrs.addAttribute("indirect-tls-seg-refs");
if (CodeGenOpts.NoImplicitFloat)
FuncAttrs.addAttribute(llvm::Attribute::NoImplicitFloat);
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=3447
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344739: Add support for -mno-tls-direct-seg-refs to Clang
(authored by kristina, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53102?vs=169224&id=170082#toc
Repository:
rC Clang
https://reviews.llvm.org/D53102
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CGCall.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/indirect-tls-seg-refs.c
test/Driver/indirect-tls-seg-refs.c
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -62,6 +62,8 @@
CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
+ ///< is specified.
CODEGENOPT(DisableTailCalls , 1, 0) ///< Do not emit tail calls.
CODEGENOPT(NoEscapingBlockTailCalls, 1, 0) ///< Do not emit tail calls from
///< escaping blocks.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2011,6 +2011,8 @@
def mno_pascal_strings : Flag<["-"], "mno-pascal-strings">,
Alias;
def mno_red_zone : Flag<["-"], "mno-red-zone">, Group;
+def mno_tls_direct_seg_refs : Flag<["-"], "mno-tls-direct-seg-refs">,
Group, Flags<[CC1Option]>,
+ HelpText<"Disable direct TLS access through segment registers">;
def mno_relax_all : Flag<["-"], "mno-relax-all">, Group;
def mno_rtd: Flag<["-"], "mno-rtd">, Group;
def mno_soft_float : Flag<["-"], "mno-soft-float">, Group;
@@ -2171,6 +2173,8 @@
def moslib_EQ : Joined<["-"], "moslib=">, Group;
def mpascal_strings : Flag<["-"], "mpascal-strings">, Alias;
def mred_zone : Flag<["-"], "mred-zone">, Group;
+def mtls_direct_seg_refs : Flag<["-"], "mtls-direct-seg-refs">, Group,
+ HelpText<"Enable direct TLS access through segment registers (default)">;
def mregparm_EQ : Joined<["-"], "mregparm=">, Group;
def mrelax_all : Flag<["-"], "mrelax-all">, Group,
Flags<[CC1Option,CC1AsOption]>,
HelpText<"(integrated-as) Relax all machine instructions">;
Index: test/CodeGen/indirect-tls-seg-refs.c
===
--- test/CodeGen/indirect-tls-seg-refs.c
+++ test/CodeGen/indirect-tls-seg-refs.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -mno-tls-direct-seg-refs | FileCheck %s
-check-prefix=NO-TLSDIRECT
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s -check-prefix=TLSDIRECT
+
+// NO-TLSDIRECT: attributes #{{[0-9]+}} = {{{.*}} "indirect-tls-seg-refs"
+// TLSDIRECT-NOT: attributes #{{[0-9]+}} = {{{.*}} "indirect-tls-seg-refs"
+
+void test1() {
+}
Index: test/Driver/indirect-tls-seg-refs.c
===
--- test/Driver/indirect-tls-seg-refs.c
+++ test/Driver/indirect-tls-seg-refs.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### %s 2>&1 | FileCheck %s -check-prefix=TLSDIRECT
+// RUN: %clang -### -mno-tls-direct-seg-refs -mtls-direct-seg-refs %s 2>&1 |
FileCheck %s -check-prefix=TLSDIRECT
+// RUN: %clang -### -mtls-direct-seg-refs -mno-tls-direct-seg-refs %s 2>&1 |
FileCheck %s -check-prefix=NO-TLSDIRECT
+// REQUIRES: clang-driver
+
+// NO-TLSDIRECT: -mno-tls-direct-seg-refs
+// TLSDIRECT-NOT: -mno-tls-direct-seg-refs
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1709,6 +1709,8 @@
if (CodeGenOpts.DisableRedZone)
FuncAttrs.addAttribute(llvm::Attribute::NoRedZone);
+ if (CodeGenOpts.IndirectTlsSegRefs)
+FuncAttrs.addAttribute("indirect-tls-seg-refs");
if (CodeGenOpts.NoImplicitFloat)
FuncAttrs.addAttribute(llvm::Attribute::NoImplicitFloat);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1739,6 +1739,10 @@
Args.hasArg(options::OPT_fapple_kext))
CmdArgs.push_back("-disable-red-zone");
+ if (!Args.hasFlag(options::OPT_mtls_direct_seg_refs,
+options::OPT_mno_tls_direct_seg_refs, true))
+CmdArgs.push_back("-mno-tls-direct-seg-refs");
+
// Default to avoid implicit floating-point for kernel/kext code, but allow
// that to be overridden with -mno-soft-float.
bool NoImplicitFloat = (Args.hasArg(options::OPT_mkernel) ||
Index: lib/Frontend/CompilerInvocation.cpp
==
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
aaron.ballman added inline comments. Comment at: test/clang-tidy/readability-else-after-return-if-constexpr.cpp:9 +return; + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: + Please add some of the warning text -- any warning will match this. Comment at: test/clang-tidy/readability-else-after-return-if-constexpr.cpp:23 +} +// CHECK-NOT: warning: This seems unnecessary. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check
hwright updated this revision to Diff 170083.
hwright marked 6 inline comments as done.
hwright added a comment.
Addressed reviewer comments.
https://reviews.llvm.org/D53339
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationFactoryFloatCheck.cpp
clang-tidy/abseil/DurationFactoryFloatCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-factory-float.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-factory-float.cpp
Index: test/clang-tidy/abseil-duration-factory-float.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-factory-float.cpp
@@ -0,0 +1,115 @@
+// RUN: %check_clang_tidy %s abseil-duration-factory-float %t
+
+// Mimic the implementation of absl::Duration
+namespace absl {
+
+class Duration {};
+
+Duration Nanoseconds(long long);
+Duration Microseconds(long long);
+Duration Milliseconds(long long);
+Duration Seconds(long long);
+Duration Minutes(long long);
+Duration Hours(long long);
+
+#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \
+ Duration NAME(float n); \
+ Duration NAME(double n);\
+ template\
+ Duration NAME(T n);
+
+GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Seconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Minutes);
+GENERATE_DURATION_FACTORY_OVERLOADS(Hours);
+#undef GENERATE_DURATION_FACTORY_OVERLOADS
+
+} // namespace absl
+
+void ConvertFloatTest() {
+ absl::Duration d;
+
+ d = absl::Seconds(60.0);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(60);
+ d = absl::Minutes(300.0);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Minutes [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Minutes(300);
+
+ d = absl::Milliseconds(1e2);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Milliseconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Milliseconds(100);
+ d = absl::Seconds(3.0f);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(3);
+ d = absl::Seconds(3.);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(3);
+ d = absl::Seconds(0x3.p0);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(3);
+ d = absl::Seconds(0x3.p1);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(6);
+
+
+ // Ignored expressions
+ d = absl::Seconds(.001);
+ d = absl::Seconds(.100);
+ d = ::absl::Seconds(1);
+ d = ::absl::Minutes(1);
+ d = ::absl::Hours(1);
+ d = absl::Seconds(0x3.4p1);
+
+ // Negative literals (we don't yet handle this case)
+ d = absl::Seconds(-3.0);
+
+ // This is bigger than we can safely fit in a positive int32, so we ignore it.
+ d = absl::Seconds(1e12);
+
+ int x;
+ d = absl::Seconds(x);
+ float y;
+ d = absl::Minutes(y);
+
+#define SECONDS(x) absl::Seconds(x)
+ SECONDS(60);
+#undef SECONDS
+#define THIRTY 30.0
+ d = absl::Seconds(THIRTY);
+#undef THIRTY
+}
+
+template
+void InTemplate() {
+ absl::Duration d;
+
+ d = absl::Seconds(N); // 1
+ // ^ No replacement here.
+
+ d = absl::Minutes(1.0); // 2
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Minutes [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Minutes(1); // 2
+}
+
+void Instantiate() {
+ InTemplate<60>();
+ InTemplate<1>();
+}
+
+void ConvertStaticCastTest() {
+ absl::Duration d;
+
+ d = absl::Seconds(static_cast(5));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(5);
+
+ d = absl::Minutes(static_cast(5));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Minutes [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Minutes(5);
+
+ // This should not be flagged
+ d = absl::Seconds(static_cast(5.0));
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
.. toctree::
abseil-duration-division
+ abseil-duration-factory-float
abseil-faster-strsplit-delimiter
abseil-no-internal-dependencies
abseil-no-namespace
Index: docs/clang-tidy/checks/abseil-duration-factory-float.rst
===
[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check
hwright added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:34
+llvm::APSInt ApInt(/*BitWidth=*/64, /*isUnsigned=*/false);
+ApInt = static_cast(value);
+if (is_negative)
JonasToth wrote:
> Wouldn't it make more sense to use `std::uint64_t` instead to correspond to
> the line above? And where does the signedness difference come from?
> (`/*isUnsigned=*/false`)
I don't remember where the signedness difference came from, so I've made this
`std::int64_t`.
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:69
+ // Macros are ignored.
+ if (Arg->getBeginLoc().isMacroID())
+return;
JonasToth wrote:
> maybe `assert` instead, as your comment above suggests that macros are
> already filtered out?
This is a different check than above.
In the first case, we want to be sure we aren't replacing cases inside of a
macro, such as:
```
#define SECONDS(x) absl::Seconds(x)
SECONDS(1.0)
```
In this one, we want to avoid changing the argument if it is itself a macro:
```
#define VAL 1.0
absl::Seconds(VAL);
```
So it is a separate check, not just a re-assertion of the first one.
Comment at: test/clang-tidy/abseil-duration-factory-float.cpp:32
+void ConvertFloatTest() {
+ absl::Duration d;
+
JonasToth wrote:
> Could you also provide test cases with hexadecimal floating literals, which
> are C++17? The thousand-separators could be checked as well (dunno the
> correct term right now, but the `1'000'000` feature).
> Please add test cases for negative literals, too.
Added the hexadecimal floating literal tests.
The testing infrastructure wouldn't build the test source with `3'000` as a
literal argument. (There's also an argument that by the time we get to the
AST, that distinction is gone anyway and this test isn't the place to check
comprehensive literal parsing.)
I've also added a negative literal test, to illustrate that we don't yet handle
that case (which is in practice pretty rare). I'd rather add it in a separate
change.
https://reviews.llvm.org/D53339
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
nruslan added a comment. @kristina : Thank you very much for taking care of the patchsets! Repository: rC Clang https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344740 - [OPENMP] Move OMPClausePrinter to OpenMPClause.h/OpenMPClause.cpp - NFC. Differential Revision: https://reviews.llvm.org/D53102
Author: plyster
Date: Thu Oct 18 07:28:23 2018
New Revision: 344740
URL: http://llvm.org/viewvc/llvm-project?rev=344740&view=rev
Log:
[OPENMP] Move OMPClausePrinter to OpenMPClause.h/OpenMPClause.cpp - NFC.
Differential Revision: https://reviews.llvm.org/D53102
Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/AST/StmtPrinter.cpp
Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=344740&r1=344739&r2=344740&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Thu Oct 18 07:28:23 2018
@@ -5203,6 +5203,22 @@ class OMPClauseVisitor :
template
class ConstOMPClauseVisitor :
public OMPClauseVisitorBase {};
+
+class OMPClausePrinter final : public OMPClauseVisitor {
+ raw_ostream &OS;
+ const PrintingPolicy &Policy;
+
+ /// Process clauses with list of variables.
+ template void VisitOMPClauseList(T *Node, char StartSym);
+
+public:
+ OMPClausePrinter(raw_ostream &OS, const PrintingPolicy &Policy)
+ : OS(OS), Policy(Policy) {}
+
+#define OPENMP_CLAUSE(Name, Class) void Visit##Class(Class *S);
+#include "clang/Basic/OpenMPKinds.def"
+};
+
} // namespace clang
#endif // LLVM_CLANG_AST_OPENMPCLAUSE_H
Modified: cfe/trunk/lib/AST/OpenMPClause.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/OpenMPClause.cpp?rev=344740&r1=344739&r2=344740&view=diff
==
--- cfe/trunk/lib/AST/OpenMPClause.cpp (original)
+++ cfe/trunk/lib/AST/OpenMPClause.cpp Thu Oct 18 07:28:23 2018
@@ -14,6 +14,7 @@
#include "clang/AST/OpenMPClause.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/DeclOpenMP.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/Support/Casting.h"
@@ -1049,3 +1050,434 @@ OMPIsDevicePtrClause *OMPIsDevicePtrClau
return new (Mem) OMPIsDevicePtrClause(NumVars, NumUniqueDeclarations,
NumComponentLists, NumComponents);
}
+
+//===--===//
+// OpenMP clauses printing methods
+//===--===//
+
+void OMPClausePrinter::VisitOMPIfClause(OMPIfClause *Node) {
+ OS << "if(";
+ if (Node->getNameModifier() != OMPD_unknown)
+OS << getOpenMPDirectiveName(Node->getNameModifier()) << ": ";
+ Node->getCondition()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPFinalClause(OMPFinalClause *Node) {
+ OS << "final(";
+ Node->getCondition()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPNumThreadsClause(OMPNumThreadsClause *Node) {
+ OS << "num_threads(";
+ Node->getNumThreads()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPSafelenClause(OMPSafelenClause *Node) {
+ OS << "safelen(";
+ Node->getSafelen()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPSimdlenClause(OMPSimdlenClause *Node) {
+ OS << "simdlen(";
+ Node->getSimdlen()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPCollapseClause(OMPCollapseClause *Node) {
+ OS << "collapse(";
+ Node->getNumForLoops()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPDefaultClause(OMPDefaultClause *Node) {
+ OS << "default("
+ << getOpenMPSimpleClauseTypeName(OMPC_default, Node->getDefaultKind())
+ << ")";
+}
+
+void OMPClausePrinter::VisitOMPProcBindClause(OMPProcBindClause *Node) {
+ OS << "proc_bind("
+ << getOpenMPSimpleClauseTypeName(OMPC_proc_bind, Node->getProcBindKind())
+ << ")";
+}
+
+void OMPClausePrinter::VisitOMPUnifiedAddressClause(OMPUnifiedAddressClause *)
{
+ OS << "unified_address";
+}
+
+void OMPClausePrinter::VisitOMPUnifiedSharedMemoryClause(
+OMPUnifiedSharedMemoryClause *) {
+ OS << "unified_shared_memory";
+}
+
+void OMPClausePrinter::VisitOMPReverseOffloadClause(OMPReverseOffloadClause *)
{
+ OS << "reverse_offload";
+}
+
+void OMPClausePrinter::VisitOMPDynamicAllocatorsClause(
+OMPDynamicAllocatorsClause *) {
+ OS << "dynamic_allocators";
+}
+
+void OMPClausePrinter::VisitOMPScheduleClause(OMPScheduleClause *Node) {
+ OS << "schedule(";
+ if (Node->getFirstScheduleModifier() != OMPC_SCHEDULE_MODIFIER_unknown) {
+OS << getOpenMPSimpleClauseTypeName(OMPC_schedule,
+Node->getFirstScheduleModifier());
+if (Node->getSecondScheduleModifier() != OMPC_SCHEDULE_MODIFIER_unknown) {
+ OS << ", ";
+ OS << getOpenMPSimpleClauseTypeName(OMPC_schedule,
+
[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, jfb, arphaman, jkorous,
MaskRay, ilya-biryukov.
In debug builds, getting this wrong will trigger asserts.
In production builds, it will send an error reply if none was sent,
and drop redundant replies. (And log).
No tests because this is always a programming error.
(We did have some cases of this, but I fixed them with the new dispatcher).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53399
Files:
clangd/ClangdLSPServer.cpp
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -106,11 +106,14 @@
bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
log("<-- {0}({1})", Method, ID);
+// Calls can be canceled by the client. Add cancellation context.
+WithContext WithCancel(cancelableRequestContext(ID));
+ReplyOnce Reply(ID, Method, &Server);
if (auto Handler = Calls.lookup(Method))
- Handler(std::move(Params), std::move(ID));
+ Handler(std::move(Params), std::move(Reply));
else
- Server.reply(ID, llvm::make_error("method not found",
- ErrorCode::MethodNotFound));
+ Reply(llvm::make_error("method not found",
+ ErrorCode::MethodNotFound));
return true;
}
@@ -124,34 +127,31 @@
}
// Bind an LSP method name to a call.
- template
+ template
void bind(const char *Method,
-void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
+void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
Calls[Method] = [Method, Handler, this](json::Value RawParams,
-json::Value ID) {
+ReplyOnce Reply) {
Param P;
if (!fromJSON(RawParams, P)) {
elog("Failed to decode {0} request.", Method);
-Server.reply(ID, make_error("failed to decode request",
- ErrorCode::InvalidRequest));
+Reply(make_error("failed to decode request",
+ ErrorCode::InvalidRequest));
return;
}
trace::Span Tracer(Method);
SPAN_ATTACH(Tracer, "Params", RawParams);
auto *Trace = Tracer.Args; // We attach reply from another thread.
- // Calls can be canceled by the client. Add cancellation context.
- WithContext WithCancel(cancelableRequestContext(ID));
- // FIXME: this function should assert it's called exactly once.
- (Server.*Handler)(P, [this, ID, Trace](llvm::Expected Result) {
-if (Result) {
+ (Server.*Handler)(P, [Trace, Reply](llvm::Expected Res) {
+if (Res) {
if (Trace)
-(*Trace)["Reply"] = *Result;
- Server.reply(ID, json::Value(std::move(*Result)));
+(*Trace)["Reply"] = *Res;
+ Reply(json::Value(std::move(*Res)));
} else {
- auto Err = Result.takeError();
+ auto Err = Res.takeError();
if (Trace)
(*Trace)["Error"] = llvm::to_string(Err);
- Server.reply(ID, std::move(Err));
+ Reply(std::move(Err));
}
});
};
@@ -174,8 +174,47 @@
}
private:
+ // Function object to reply to an LSP call.
+ // Each instance must be called exactly once, otherwise:
+ // - the bug is logged, and (in debug mode) an assert will fire
+ // - if there was no reply, an error reply is sent
+ // - if there were multiple replies, only the first is sent
+ class ReplyOnce {
+struct State {
+ std::atomic Replied = {false};
+ json::Value ID;
+ std::string Method;
+ ClangdLSPServer *Server;
+
+ State(const json::Value &ID, StringRef Method, ClangdLSPServer *Server)
+ : ID(ID), Method(Method), Server(Server) {}
+ ~State() {
+if (!Replied) {
+ elog("No reply to message {0}({1})", Method, ID);
+ assert(false && "must reply to all calls!");
+ Server->reply(ID, make_error("server failed to reply",
+ ErrorCode::InternalError));
+}
+ }
+};
+std::shared_ptr MyState;
+
+ public:
+ReplyOnce(const json::Value &ID, StringRef Method, ClangdLSPServer *Server)
+: MyState(std::make_shared(ID, Method, Server)) {}
+
+void operator()(llvm::Expected Reply) const {
+ if (MyState->Replied.exchange(true)) {
+elog("Replied twice to message {0}({1})", MyState->Method, MyState->ID);
+assert(false && "must reply to each call only once!");
+return;
+ }
+ MyState->Server->reply(MyState->ID, std::move(Reply));
+}
+ };
+
llvm::StringMap> Notifications;
- llvm::StringMap> Calls;
+ llvm::StringMap> Calls;
// Metho
[clang-tools-extra] r344741 - [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.
Author: sammccall
Date: Thu Oct 18 07:41:50 2018
New Revision: 344741
URL: http://llvm.org/viewvc/llvm-project?rev=344741&view=rev
Log:
[clangd] Enforce rules around "initialize" request, and create ClangdServer
lazily.
Summary:
LSP is a slightly awkward map to C++ object lifetimes: the initialize request
is part of the protocol and provides information that doesn't change over the
lifetime of the server.
Until now, we handled this by initializing ClangdServer and ClangdLSPServer
right away, and making anything that can be set in the "initialize" request
mutable.
With this patch, we create ClangdLSPServer immediately, but defer creating
ClangdServer until "initialize". This opens the door to passing the relevant
initialize params in the constructor and storing them immutably.
(That change isn't actually done in this patch).
To make this safe, we have the MessageDispatcher enforce that the "initialize"
method is called before any other (as required by LSP). That way each method
handler can assume Server is initialized, as today.
As usual, while implementing this I found places where our test cases violated
the protocol.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53398
Added:
clang-tools-extra/trunk/test/clangd/initialize-sequence.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/test/clangd/exit-with-shutdown.test
clang-tools-extra/trunk/test/clangd/exit-without-shutdown.test
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344741&r1=344740&r2=344741&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct 18 07:41:50 2018
@@ -87,6 +87,7 @@ CompletionItemKindBitset defaultCompleti
// - logging of inbound messages
// - cancellation handling
// - basic call tracing
+// MessageHandler ensures that initialize() is called before any other handler.
class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
public:
MessageHandler(ClangdLSPServer &Server) : Server(Server) {}
@@ -95,7 +96,9 @@ public:
log("<-- {0}", Method);
if (Method == "exit")
return false;
-if (Method == "$/cancelRequest")
+if (!Server.Server)
+ elog("Notification {0} before initialization", Method);
+else if (Method == "$/cancelRequest")
onCancel(std::move(Params));
else if (auto Handler = Notifications.lookup(Method))
Handler(std::move(Params));
@@ -106,7 +109,11 @@ public:
bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
log("<-- {0}({1})", Method, ID);
-if (auto Handler = Calls.lookup(Method))
+if (!Server.Server && Method != "initialize") {
+ elog("Call {0} before initialization.", Method);
+ Server.reply(ID, make_error("server not initialized",
+ErrorCode::ServerNotInitialized));
+} else if (auto Handler = Calls.lookup(Method))
Handler(std::move(Params), std::move(ID));
else
Server.reply(ID, llvm::make_error("method not found",
@@ -254,6 +261,11 @@ void ClangdLSPServer::reply(llvm::json::
void ClangdLSPServer::onInitialize(const InitializeParams &Params,
Callback Reply) {
+ if (Server)
+return Reply(make_error("server already initialized",
+ ErrorCode::InvalidRequest));
+ Server.emplace(CDB.getCDB(), FSProvider,
+ static_cast(*this), ClangdServerOpts);
if (Params.initializationOptions) {
const ClangdInitializationOptions &Opts = *Params.initializationOptions;
@@ -663,8 +675,7 @@ ClangdLSPServer::ClangdLSPServer(class T
std::move(CompileCommandsDir))),
CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
SupportedCompletionItemKinds(defaultCompletionItemKinds()),
- Server(new ClangdServer(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this,
- Opts)) {
+ ClangdServerOpts(Opts) {
// clang-format off
MsgHandler->bind("initialize", &ClangdLSPServer::onInitialize);
MsgHandler->bind("shutdown", &ClangdLSPServer::onShutdown);
@@ -694,8 +705,6 @@ ClangdLSPServer::ClangdLSPServer(class T
ClangdLSPServer::~ClangdLSPServer() = default;
bool ClangdLSPServer::run() {
- assert(Server);
-
// Run the Language Server loop.
bool CleanExit = true;
if (auto Err = Transp.loop(*MsgHandler)) {
@@ -705,7 +714,6 @@ bool ClangdLSPServer::run() {
// Destroy C
[PATCH] D53398: [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344741: [clangd] Enforce rules around "initialize"
request, and create ClangdServer… (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53398
Files:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/test/clangd/exit-with-shutdown.test
clang-tools-extra/trunk/test/clangd/exit-without-shutdown.test
clang-tools-extra/trunk/test/clangd/initialize-sequence.test
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h
@@ -183,12 +183,10 @@
// Store of the current versions of the open documents.
DraftStore DraftMgr;
- // Server must be the last member of the class to allow its destructor to exit
- // the worker thread that may otherwise run an async callback on partially
- // destructed instance of ClangdLSPServer.
- // Set in construtor and destroyed when run() finishes. To ensure all worker
- // threads exit before run() returns.
- std::unique_ptr Server;
+ // The ClangdServer is created by the "initialize" LSP method.
+ // It is destroyed before run() returns, to ensure worker threads exit.
+ ClangdServer::Options ClangdServerOpts;
+ llvm::Optional Server;
};
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -103,7 +103,7 @@
DiagnosticsConsumer &DiagConsumer,
const Options &Opts)
: CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
- ResourceDir(Opts.ResourceDir ? Opts.ResourceDir->str()
+ ResourceDir(Opts.ResourceDir ? *Opts.ResourceDir
: getStandardResourceDir()),
DynamicIdx(Opts.BuildDynamicSymbolIndex
? new FileIndex(Opts.URISchemes,
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -87,15 +87,18 @@
// - logging of inbound messages
// - cancellation handling
// - basic call tracing
+// MessageHandler ensures that initialize() is called before any other handler.
class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
public:
MessageHandler(ClangdLSPServer &Server) : Server(Server) {}
bool onNotify(StringRef Method, json::Value Params) override {
log("<-- {0}", Method);
if (Method == "exit")
return false;
-if (Method == "$/cancelRequest")
+if (!Server.Server)
+ elog("Notification {0} before initialization", Method);
+else if (Method == "$/cancelRequest")
onCancel(std::move(Params));
else if (auto Handler = Notifications.lookup(Method))
Handler(std::move(Params));
@@ -106,7 +109,11 @@
bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
log("<-- {0}({1})", Method, ID);
-if (auto Handler = Calls.lookup(Method))
+if (!Server.Server && Method != "initialize") {
+ elog("Call {0} before initialization.", Method);
+ Server.reply(ID, make_error("server not initialized",
+ErrorCode::ServerNotInitialized));
+} else if (auto Handler = Calls.lookup(Method))
Handler(std::move(Params), std::move(ID));
else
Server.reply(ID, llvm::make_error("method not found",
@@ -254,6 +261,11 @@
void ClangdLSPServer::onInitialize(const InitializeParams &Params,
Callback Reply) {
+ if (Server)
+return Reply(make_error("server already initialized",
+ ErrorCode::InvalidRequest));
+ Server.emplace(CDB.getCDB(), FSProvider,
+ static_cast(*this), ClangdServerOpts);
if (Params.initializationOptions) {
const ClangdInitializationOptions &Opts = *Params.initializationOptions;
@@ -663,8 +675,7 @@
std::move(CompileCommandsDir))),
CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
SupportedCompletionItemKinds(defaultCompletionItemKinds()),
- Server(new ClangdServer(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this,
- Opts)) {
+ ClangdServerOpts(Opts) {
// clang-format off
MsgHandler->bind("initialize", &ClangdLSPServer::onInitialize);
MsgHandler-
[PATCH] D53400: [clangd] Remove the overflow log.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
LLVM codebase has generated files (all are build/Target/XXX/*.inc) that
exceed the MaxLine & MaxColumn. Printing these log would be noisy.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53400
Files:
clangd/index/Index.cpp
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
constexpr uint32_t SymbolLocation::Position::MaxColumn;
void SymbolLocation::Position::setLine(uint32_t L) {
if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
Line = MaxLine;
return;
}
Line = L;
}
void SymbolLocation::Position::setColumn(uint32_t Col) {
if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
Column = MaxColumn;
return;
}
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
constexpr uint32_t SymbolLocation::Position::MaxColumn;
void SymbolLocation::Position::setLine(uint32_t L) {
if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
Line = MaxLine;
return;
}
Line = L;
}
void SymbolLocation::Position::setColumn(uint32_t Col) {
if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
Column = MaxColumn;
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344742 - [X86][Tests] Make sure tls-direct-seg-refs tests only run where supported
Author: kristina
Date: Thu Oct 18 07:44:25 2018
New Revision: 344742
URL: http://llvm.org/viewvc/llvm-project?rev=344742&view=rev
Log:
[X86][Tests] Make sure tls-direct-seg-refs tests only run where supported
This flag is only supported for x86 targets, make sure the tests only run
for those.
Modified:
cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c
cfe/trunk/test/Driver/indirect-tls-seg-refs.c
Modified: cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c?rev=344742&r1=344741&r2=344742&view=diff
==
--- cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c (original)
+++ cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c Thu Oct 18 07:44:25 2018
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -mno-tls-direct-seg-refs | FileCheck %s
-check-prefix=NO-TLSDIRECT
-// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s -check-prefix=TLSDIRECT
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-unknown-linux -o -
-mno-tls-direct-seg-refs | FileCheck %s -check-prefix=NO-TLSDIRECT
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-unknown-linux -o - | FileCheck
%s -check-prefix=TLSDIRECT
// NO-TLSDIRECT: attributes #{{[0-9]+}} = {{{.*}} "indirect-tls-seg-refs"
// TLSDIRECT-NOT: attributes #{{[0-9]+}} = {{{.*}} "indirect-tls-seg-refs"
Modified: cfe/trunk/test/Driver/indirect-tls-seg-refs.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/indirect-tls-seg-refs.c?rev=344742&r1=344741&r2=344742&view=diff
==
--- cfe/trunk/test/Driver/indirect-tls-seg-refs.c (original)
+++ cfe/trunk/test/Driver/indirect-tls-seg-refs.c Thu Oct 18 07:44:25 2018
@@ -1,7 +1,8 @@
-// RUN: %clang -### %s 2>&1 | FileCheck %s -check-prefix=TLSDIRECT
-// RUN: %clang -### -mno-tls-direct-seg-refs -mtls-direct-seg-refs %s 2>&1 |
FileCheck %s -check-prefix=TLSDIRECT
-// RUN: %clang -### -mtls-direct-seg-refs -mno-tls-direct-seg-refs %s 2>&1 |
FileCheck %s -check-prefix=NO-TLSDIRECT
-// REQUIRES: clang-driver
+// REQUIRES: clang-driver, x86-registered-target
+
+// RUN: %clang -### -target x86_64-unknown-linux %s 2>&1 | FileCheck %s
-check-prefix=TLSDIRECT
+// RUN: %clang -### -target x86_64-unknown-linux -mno-tls-direct-seg-refs
-mtls-direct-seg-refs %s 2>&1 | FileCheck %s -check-prefix=TLSDIRECT
+// RUN: %clang -### -target x86_64-unknown-linux -mtls-direct-seg-refs
-mno-tls-direct-seg-refs %s 2>&1 | FileCheck %s -check-prefix=NO-TLSDIRECT
// NO-TLSDIRECT: -mno-tls-direct-seg-refs
// TLSDIRECT-NOT: -mno-tls-direct-seg-refs
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
sammccall added a comment. After looking at the examples, I'm reassured that we don't really care about tracking precise locations of those references :-) I'd suggest adding the logging just to where the field is *read* in XRefs.cpp (check if it's max-value). That would cover a bunch of the cases where an incorrect location is likely to be visible. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53400 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: clangd/ClangdLSPServer.cpp:558
json::Object{
{"uri", URIForFile{File}},
+{"diagnostics", std::move(LSPDiagnostics)},
nit: we can reuse `URI` insteading of calling `URIForFile` again.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
hokein updated this revision to Diff 170091.
hokein added a comment.
Add log in XRefs.cpp.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53400
Files:
clangd/XRefs.cpp
clangd/index/Index.cpp
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
constexpr uint32_t SymbolLocation::Position::MaxColumn;
void SymbolLocation::Position::setLine(uint32_t L) {
if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
Line = MaxLine;
return;
}
Line = L;
}
void SymbolLocation::Position::setColumn(uint32_t Col) {
if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
Column = MaxColumn;
return;
}
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -37,6 +37,20 @@
return nullptr;
}
+uint32_t getLine(const SymbolLocation::Position Pos) {
+ uint32_t Line = Pos.line();
+ if (Line >= SymbolLocation::Position::MaxLine)
+log("Get an overflowed line");
+ return Line;
+}
+
+uint32_t getColumn(const SymbolLocation::Position Pos) {
+ uint32_t Column = Pos.column();
+ if (Column >= SymbolLocation::Position::MaxColumn)
+log("Get an overflowed column");
+ return Column;
+}
+
// Convert a SymbolLocation to LSP's Location.
// HintPath is used to resolve the path of URI.
// FIXME: figure out a good home for it, and share the implementation with
@@ -57,10 +71,10 @@
}
Location LSPLoc;
LSPLoc.uri = URIForFile(*Path);
- LSPLoc.range.start.line = Loc.Start.line();
- LSPLoc.range.start.character = Loc.Start.column();
- LSPLoc.range.end.line = Loc.End.line();
- LSPLoc.range.end.character = Loc.End.column();
+ LSPLoc.range.start.line = getLine(Loc.Start);
+ LSPLoc.range.start.character = getColumn(Loc.Start);
+ LSPLoc.range.end.line = getLine(Loc.End);
+ LSPLoc.range.end.character = getColumn(Loc.End);
return LSPLoc;
}
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
constexpr uint32_t SymbolLocation::Position::MaxColumn;
void SymbolLocation::Position::setLine(uint32_t L) {
if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
Line = MaxLine;
return;
}
Line = L;
}
void SymbolLocation::Position::setColumn(uint32_t Col) {
if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
Column = MaxColumn;
return;
}
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -37,6 +37,20 @@
return nullptr;
}
+uint32_t getLine(const SymbolLocation::Position Pos) {
+ uint32_t Line = Pos.line();
+ if (Line >= SymbolLocation::Position::MaxLine)
+log("Get an overflowed line");
+ return Line;
+}
+
+uint32_t getColumn(const SymbolLocation::Position Pos) {
+ uint32_t Column = Pos.column();
+ if (Column >= SymbolLocation::Position::MaxColumn)
+log("Get an overflowed column");
+ return Column;
+}
+
// Convert a SymbolLocation to LSP's Location.
// HintPath is used to resolve the path of URI.
// FIXME: figure out a good home for it, and share the implementation with
@@ -57,10 +71,10 @@
}
Location LSPLoc;
LSPLoc.uri = URIForFile(*Path);
- LSPLoc.range.start.line = Loc.Start.line();
- LSPLoc.range.start.character = Loc.Start.column();
- LSPLoc.range.end.line = Loc.End.line();
- LSPLoc.range.end.character = Loc.End.column();
+ LSPLoc.range.start.line = getLine(Loc.Start);
+ LSPLoc.range.start.character = getColumn(Loc.Start);
+ LSPLoc.range.end.line = getLine(Loc.End);
+ LSPLoc.range.end.character = getColumn(Loc.End);
return LSPLoc;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344745 - [clangd] Clear the semantic of RefSlab::size.
Author: hokein
Date: Thu Oct 18 08:33:20 2018
New Revision: 344745
URL: http://llvm.org/viewvc/llvm-project?rev=344745&view=rev
Log:
[clangd] Clear the semantic of RefSlab::size.
Summary:
The RefSlab::size can easily cause confusions, it returns the number of
different symbols, rahter than the number of all references.
- add numRefs() method and cache it, since calculating it everytime is
nontrivial.
- clear misused places.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet,
cfe-commits
Differential Revision: https://reviews.llvm.org/D53389
Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=344745&r1=344744&r2=344745&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Oct 18 08:33:20 2018
@@ -174,9 +174,9 @@ llvm::Error BackgroundIndex::index(tooli
Action->EndSourceFile();
log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
- Symbols.size(), Refs.size());
+ Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
- SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
+ SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique(std::move(Symbols)),
Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=344745&r1=344744&r2=344745&view=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Thu Oct 18 08:33:20 2018
@@ -63,9 +63,9 @@ indexSymbols(ASTContext &AST, std::share
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
- " ref slab: {4} symbols, {5} bytes",
+ " ref slab: {4} symbols, {5} refs, {6} bytes",
FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
- Refs.bytes());
+ Refs.numRefs(), Refs.bytes());
return {std::move(Syms), std::move(Refs)};
}
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=344745&r1=344744&r2=344745&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Thu Oct 18 08:33:20 2018
@@ -172,17 +172,19 @@ RefSlab RefSlab::Builder::build() && {
// Reallocate refs on the arena to reduce waste and indirections when
reading.
std::vector>> Result;
Result.reserve(Refs.size());
+ size_t NumRefs = 0;
for (auto &Sym : Refs) {
auto &SymRefs = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
+NumRefs += SymRefs.size();
auto *Array = Arena.Allocate(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef(Array, SymRefs.size()));
}
- return RefSlab(std::move(Result), std::move(Arena));
+ return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}
void SwapIndex::reset(std::unique_ptr Index) {
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=344745&r1=344744&r2=344745&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Thu Oct 18 08:33:20 2018
@@ -407,7 +407,9 @@ public:
const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
+ /// Gets the number of symbols.
size_t size() const { return Refs.size(); }
+ size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }
size_t bytes() const {
@@ -431,11 +433,14 @@ public:
};
private:
- RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena)
- : Arena(std::move(Arena)), Refs(std::move(Refs)) {}
+ RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena,
[PATCH] D53389: [clangd] Clear the semantic of RefSlab::size.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344745: [clangd] Clear the semantic of RefSlab::size.
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53389
Files:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
Index: clang-tools-extra/trunk/clangd/index/Background.cpp
===
--- clang-tools-extra/trunk/clangd/index/Background.cpp
+++ clang-tools-extra/trunk/clangd/index/Background.cpp
@@ -174,9 +174,9 @@
Action->EndSourceFile();
log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
- Symbols.size(), Refs.size());
+ Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
- SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
+ SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique(std::move(Symbols)),
Index: clang-tools-extra/trunk/clangd/index/Serialization.cpp
===
--- clang-tools-extra/trunk/clangd/index/Serialization.cpp
+++ clang-tools-extra/trunk/clangd/index/Serialization.cpp
@@ -496,18 +496,18 @@
}
}
- size_t SymSize = Symbols.size();
- size_t RefSize = Refs.size();
+ size_t NumSym = Symbols.size();
+ size_t NumRefs = Refs.numRefs();
+
trace::Span Tracer("BuildIndex");
auto Index =
UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
: MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2} bytes\n"
- " - number of symbos: {3}\n"
+ " - number of symbols: {3}\n"
" - number of refs: {4}\n",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
- Index->estimateMemoryUsage(),
- SymSize, RefSize);
+ Index->estimateMemoryUsage(), NumSym, NumRefs);
return Index;
}
Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -407,7 +407,9 @@
const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
+ /// Gets the number of symbols.
size_t size() const { return Refs.size(); }
+ size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }
size_t bytes() const {
@@ -431,11 +433,14 @@
};
private:
- RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena)
- : Arena(std::move(Arena)), Refs(std::move(Refs)) {}
+ RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena,
+ size_t NumRefs)
+ : Arena(std::move(Arena)), Refs(std::move(Refs)), NumRefs(NumRefs) {}
llvm::BumpPtrAllocator Arena;
std::vector Refs;
+ // Number of all references.
+ size_t NumRefs = 0;
};
struct FuzzyFindRequest {
Index: clang-tools-extra/trunk/clangd/index/Index.cpp
===
--- clang-tools-extra/trunk/clangd/index/Index.cpp
+++ clang-tools-extra/trunk/clangd/index/Index.cpp
@@ -172,17 +172,19 @@
// Reallocate refs on the arena to reduce waste and indirections when reading.
std::vector>> Result;
Result.reserve(Refs.size());
+ size_t NumRefs = 0;
for (auto &Sym : Refs) {
auto &SymRefs = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
+NumRefs += SymRefs.size();
auto *Array = Arena.Allocate(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef(Array, SymRefs.size()));
}
- return RefSlab(std::move(Result), std::move(Arena));
+ return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}
void SwapIndex::reset(std::unique_ptr Index) {
Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp
@@ -63,9 +63,9 @@
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
- " ref slab: {4} symbols, {5} bytes",
+ " ref slab: {4} symbols, {5} refs, {6} bytes",
FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
- Refs.bytes());
+ Refs.numRefs(), Refs.
[PATCH] D53389: [clangd] Clear the semantic of RefSlab::size.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344745: [clangd] Clear the semantic of RefSlab::size.
(authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53389?vs=170063&id=170094#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53389
Files:
clangd/index/Background.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Serialization.cpp
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -174,9 +174,9 @@
Action->EndSourceFile();
log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
- Symbols.size(), Refs.size());
+ Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
- SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
+ SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique(std::move(Symbols)),
Index: clangd/index/Serialization.cpp
===
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -496,18 +496,18 @@
}
}
- size_t SymSize = Symbols.size();
- size_t RefSize = Refs.size();
+ size_t NumSym = Symbols.size();
+ size_t NumRefs = Refs.numRefs();
+
trace::Span Tracer("BuildIndex");
auto Index =
UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
: MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2} bytes\n"
- " - number of symbos: {3}\n"
+ " - number of symbols: {3}\n"
" - number of refs: {4}\n",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
- Index->estimateMemoryUsage(),
- SymSize, RefSize);
+ Index->estimateMemoryUsage(), NumSym, NumRefs);
return Index;
}
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -407,7 +407,9 @@
const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
+ /// Gets the number of symbols.
size_t size() const { return Refs.size(); }
+ size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }
size_t bytes() const {
@@ -431,11 +433,14 @@
};
private:
- RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena)
- : Arena(std::move(Arena)), Refs(std::move(Refs)) {}
+ RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena,
+ size_t NumRefs)
+ : Arena(std::move(Arena)), Refs(std::move(Refs)), NumRefs(NumRefs) {}
llvm::BumpPtrAllocator Arena;
std::vector Refs;
+ // Number of all references.
+ size_t NumRefs = 0;
};
struct FuzzyFindRequest {
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -172,17 +172,19 @@
// Reallocate refs on the arena to reduce waste and indirections when reading.
std::vector>> Result;
Result.reserve(Refs.size());
+ size_t NumRefs = 0;
for (auto &Sym : Refs) {
auto &SymRefs = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
+NumRefs += SymRefs.size();
auto *Array = Arena.Allocate(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef(Array, SymRefs.size()));
}
- return RefSlab(std::move(Result), std::move(Arena));
+ return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}
void SwapIndex::reset(std::unique_ptr Index) {
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -63,9 +63,9 @@
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
- " ref slab: {4} symbols, {5} bytes",
+ " ref slab: {4} symbols, {5} refs, {6} bytes",
FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
- Refs.bytes());
+ Refs.numRefs(), Refs.bytes());
return {std::move(Syms), std::move(Refs)};
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Some nitty ideas about the log message, but whatever you think is best.
Comment at: clangd/XRefs.cpp:43
+ if (Line >= SymbolLocation::Position::MaxLine)
+log("Get an overflowed line");
+ return Line;
Log message could use more context. And I think it'd be really useful to print
the whole location here.
bikeshed: you could add a method `bool Position::hasOverflow()`, and then write
below
```
if (LSPLoc.range.start.hasOverflow() || LSPLoc.range.end.hasOverflow())
log("Possible overflow in symbol location: {0}", LSPLoc);
```
Distinguishing between line/column overflow isn't important if you're printing
the full range anyway.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52730: [analyzer] ConversionChecker: handle floating point
donat.nagy added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164 +// double and possibly long double on some systems +RepresentsUntilExp = 53; break; + case 32: xazax.hun wrote: > A link to the source of these number would be useful. How are these > calculated. Also, as far as I know the current C++ standard does not require > anything about how floating points are represented in an implementation. So > it would be great to somehow refer to the representation used by clang rather > than hardcoding these values. (Note that I am perfectly fine with warning for > implementation defined behavior as the original version also warn for such in > case of integers.) I took these magic numbers from the IEEE 754 standard; I completely agree that their introduction is far from being elegant. Unfortunately it seems that referring to the representation used by clang seems to be somewhat difficult, see e.g. this old [[ https://stackoverflow.com/questions/13780931/how-do-i-get-llvm-types-from-clang | stackoverflow answer ]]. In the Z3 solver a similar problem was solved by defining a static function ([[ https://clang.llvm.org/doxygen/Z3ConstraintManager_8cpp_source.html#l00269 | getFloatSemantics() ]]) which uses a switch to translate the bit width of a floating point type into an llvm::fltSemantics value (which contains the precision value as a field). I could imagine three solutions: - reimplementing the logic getFloatSemantics, - moving getFloatSemantics to some utility library and using it from there, - keeping the current code, with comments describing my assumptions and referencing the IEEE standard. Which of these is the best? Note: According to the documentation [[ https://releases.llvm.org/2.5/docs/LangRef.html#t_floating | the floating point types ]] supported by the LLVM IR are just float, double and some high precision extension types (which are handled by the `default:` branch of my code). Unfortunately, I do not know what happens to the [[ https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point | `_Float16` ]] half-width float type. Repository: rC Clang https://reviews.llvm.org/D52730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53404: [clangd] Set workspace root when initializing ClangdServer, disallow mutation.
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Rename instance variable to WorkspaceRoot to match what we call it internally.
Add fixme to set it automatically. Don't do it yet, clients have assumptions
that the constructor won't access the FS.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53404
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/FindSymbolsTests.cpp
Index: unittests/clangd/FindSymbolsTests.cpp
===
--- unittests/clangd/FindSymbolsTests.cpp
+++ unittests/clangd/FindSymbolsTests.cpp
@@ -42,6 +42,7 @@
ClangdServer::Options optsForTests() {
auto ServerOpts = ClangdServer::optsForTest();
+ ServerOpts.WorkspaceRoot = testRoot();
ServerOpts.BuildDynamicSymbolIndex = true;
ServerOpts.URISchemes = {"unittest", "file"};
return ServerOpts;
@@ -53,7 +54,6 @@
: Server(CDB, FSProvider, DiagConsumer, optsForTests()) {
// Make sure the test root directory is created.
FSProvider.Files[testPath("unused")] = "";
-Server.setRootPath(testRoot());
}
protected:
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -86,6 +86,11 @@
/// If set, use this index to augment code completion results.
SymbolIndex *StaticIndex = nullptr;
+/// Clangd's workspace root. Relevant for "workspace" operations not bound
+/// to a particular file.
+/// FIXME: If not set, should use the current working directory.
+llvm::Optional WorkspaceRoot;
+
/// The resource directory is used to find internal headers, overriding
/// defaults and -resource-dir compiler flag).
/// If None, ClangdServer calls CompilerInvocation::GetResourcePath() to
@@ -116,9 +121,6 @@
const FileSystemProvider &FSProvider,
DiagnosticsConsumer &DiagConsumer, const Options &Opts);
- /// Set the root path of the workspace.
- void setRootPath(PathRef RootPath);
-
/// Add a \p File to the list of tracked C++ files or update the contents if
/// \p File is already tracked. Also schedules parsing of the AST for it on a
/// separate thread. When the parsing is complete, DiagConsumer passed in
@@ -255,8 +257,7 @@
CachedCompletionFuzzyFindRequestByFile;
mutable std::mutex CachedCompletionFuzzyFindRequestMutex;
- // If set, this represents the workspace path.
- llvm::Optional RootPath;
+ llvm::Optional WorkspaceRoot;
std::shared_ptr PCHs;
/// Used to serialize diagnostic callbacks.
/// FIXME(ibiryukov): get rid of an extra map and put all version counters
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -109,6 +109,7 @@
? new FileIndex(Opts.URISchemes,
Opts.HeavyweightDynamicSymbolIndex)
: nullptr),
+ WorkspaceRoot(Opts.WorkspaceRoot),
PCHs(std::make_shared()),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
// parsed file and rebuild the file index synchronously each time an AST
@@ -131,18 +132,6 @@
Index = nullptr;
}
-void ClangdServer::setRootPath(PathRef RootPath) {
- auto FS = FSProvider.getFileSystem();
- auto Status = FS->status(RootPath);
- if (!Status)
-elog("Failed to get status for RootPath {0}: {1}", RootPath,
- Status.getError().message());
- else if (Status->isDirectory())
-this->RootPath = RootPath;
- else
-elog("The provided RootPath {0} is not a directory.", RootPath);
-}
-
void ClangdServer::addDocument(PathRef File, StringRef Contents,
WantDiagnostics WantDiags) {
DocVersion Version = ++InternalVersion[File];
@@ -495,7 +484,7 @@
void ClangdServer::workspaceSymbols(
StringRef Query, int Limit, Callback> CB) {
CB(clangd::getWorkspaceSymbols(Query, Limit, Index,
- RootPath ? *RootPath : ""));
+ WorkspaceRoot.getValueOr("")));
}
void ClangdServer::documentSymbols(
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -261,6 +261,10 @@
void ClangdLSPServer::onInitialize(const InitializeParams &Params,
Callback Reply) {
+ if (Params.rootUri && *Params.rootUri)
+ClangdServerOpts.WorkspaceRoot = Params.rootUri->file();
+ else if (Params.rootPath && !Params.rootPath->empty())
+ClangdServerOpts.WorkspaceRoot = *Params.rootPath;
if (Server)
return Reply(make_error("server already initialized",
[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag
Szelethus marked 3 inline comments as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:469
+DisplayMacroExpansions =
+getBooleanOption("expand-macros", /*Default=*/false);
+ return DisplayMacroExpansions.getValue();
NoQ wrote:
> Should we say something about plists in the option name?
Sure, but should my `AnalyzerOptions` refactoring effort go through first, I
intend emit warnings if flags aren't set correctly (eg. the output isn't set to
plist but this flag is enabled). Should probably rename `"serialize-stats"` too
then.
http://lists.llvm.org/pipermail/cfe-dev/2018-October/059842.html
Comment at:
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:44-54
+
+ kindmacro_expansion
+ location
+
+ line26
+ col3
+ file0
NoQ wrote:
> Because we're adding an element of an `` rather than a key of a
> ``, I'm not entirely sure this is backwards compatible. Clients may
> crash if they iterate over the `path` array and encounter an unexpected
> element kind. Is it going to be bad for your use case if we put expansions
> into a separate array alongside the `path` array?
It shouldn't be :)
https://reviews.llvm.org/D52742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
curdeius updated this revision to Diff 170101. curdeius added a comment. Applied changes as per comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 Files: test/clang-tidy/readability-else-after-return-if-constexpr.cpp Index: test/clang-tidy/readability-else-after-return-if-constexpr.cpp === --- test/clang-tidy/readability-else-after-return-if-constexpr.cpp +++ test/clang-tidy/readability-else-after-return-if-constexpr.cpp @@ -6,7 +6,7 @@ return; else return; - // CHECK-MESSAGES: [[@LINE-2]]:3: warning: + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return' if constexpr (sizeof(int) > 4) return; @@ -20,4 +20,3 @@ else return; } -// CHECK-NOT: warning: Index: test/clang-tidy/readability-else-after-return-if-constexpr.cpp === --- test/clang-tidy/readability-else-after-return-if-constexpr.cpp +++ test/clang-tidy/readability-else-after-return-if-constexpr.cpp @@ -6,7 +6,7 @@ return; else return; - // CHECK-MESSAGES: [[@LINE-2]]:3: warning: + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return' if constexpr (sizeof(int) > 4) return; @@ -20,4 +20,3 @@ else return; } -// CHECK-NOT: warning: ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53392: [RISCV] Collect library directories and triples for riscv64 triple too
jrtc27 added inline comments.
Comment at: lib/Driver/ToolChains/Gnu.cpp:1912
- static const char *const RISCV32LibDirs[] = {"/lib", "/lib32"};
+ static const char *const RISCVLibDirs[] = {"/lib", "/lib32"};
static const char *const RISCVTriples[] = {"riscv32-unknown-linux-gnu",
Surely this should remain as `RISCV32LibDirs`? Also, should we reverse the
order (put `"/lib32"` first) to match every single other architecture?
Comment at: lib/Driver/ToolChains/Gnu.cpp:1913
+ static const char *const RISCVLibDirs[] = {"/lib", "/lib32"};
static const char *const RISCVTriples[] = {"riscv32-unknown-linux-gnu",
"riscv32-unknown-elf"};
`RISCV32Triples`? Also, why do we have no `"riscv32-linux-gnu"` entry like the
other architectures?
Comment at: lib/Driver/ToolChains/Gnu.cpp:1915
"riscv32-unknown-elf"};
+ static const char *const RISCV64LibDirs[] = {"/lib", "/lib64"};
+ static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
Order as for 32.
Comment at: lib/Driver/ToolChains/Gnu.cpp:1916
+ static const char *const RISCV64LibDirs[] = {"/lib", "/lib64"};
+ static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
+ "riscv64-unknown-elf"};
`riscv64-linux-gnu`?
Repository:
rC Clang
https://reviews.llvm.org/D53392
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53392: [RISCV] Collect library directories and triples for riscv64 triple too
jrtc27 added inline comments. Comment at: test/Driver/riscv64-toolchain.c:71 +// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \ +// RUN: -target riscv64-linux-unknown-elf \ +// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \ This (and below) are the only instances of `CPU-linux-unknown-elf` in the whole of clang (and there are only two instances of `CPU-linux-elf`, both in `test/Modules/pch_container.m`), apart from the riscv32 ones they were copied from. Perhaps they should be `riscv64-linux-unknown-gnu` instead? Notably, when parsed as `CPU-company-kernel-OS` this in fact sets *company* to linux and *kernel* to unknown, whereas if you really mean the full 4-tuple should the linux and unknown not be interchanged? Comment at: test/Driver/riscv64-toolchain.c:87 +// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \ +// RUN: -target riscv64-linux-unknown-elf -march=rv64imafd -mabi=lp64d \ +// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \ `riscv64-linux-unknown-gnu` as before. Repository: rC Clang https://reviews.llvm.org/D53392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
ddunbar added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
What is our feeling w.r.t. _Pragma, which can in theory influence the
preprocessor. I'm not sure this model can sanely support it?
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53207: Fix bug 26547 - alignof should return ABI alignment, not preferred alignment
ubsan added a comment. I'm having real difficulty with this - I get really odd errors in seemingly unrelated tests when I change things. Repository: rC Clang https://reviews.llvm.org/D53207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
arphaman added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
ddunbar wrote:
> What is our feeling w.r.t. _Pragma, which can in theory influence the
> preprocessor. I'm not sure this model can sanely support it?
We need to look into that. In the worst case we can always avoid minimizing the
file if it has a _Pragma anywhere in it.
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344749 - Add language standard aliases for -std=c18, -std=gnu18, and -std=iso9899:2018.
Author: aaronballman
Date: Thu Oct 18 10:42:41 2018
New Revision: 344749
URL: http://llvm.org/viewvc/llvm-project?rev=344749&view=rev
Log:
Add language standard aliases for -std=c18, -std=gnu18, and -std=iso9899:2018.
As described in D40225, the C17 standard was balloted and approved in 2017, but
the ISO publication process delayed the actual publication until 2018. WG14
considers the release to be C17 and describes it as such, but users can still
be confused by the publication year which is why -std=c18 adds value. These
aliases map to c17 and are all supported by GCC 8.x with the same behavior.
Note that the value of __STDC_VERSION__ remains at 201710L.
Modified:
cfe/trunk/include/clang/Frontend/LangStandards.def
cfe/trunk/test/Driver/unknown-std.c
cfe/trunk/test/Preprocessor/c17.c
Modified: cfe/trunk/include/clang/Frontend/LangStandards.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/LangStandards.def?rev=344749&r1=344748&r2=344749&view=diff
==
--- cfe/trunk/include/clang/Frontend/LangStandards.def (original)
+++ cfe/trunk/include/clang/Frontend/LangStandards.def Thu Oct 18 10:42:41 2018
@@ -82,9 +82,12 @@ LANGSTANDARD(c17, "c17",
C, "ISO C 2017",
LineComment | C99 | C11 | C17 | Digraphs | HexFloat)
LANGSTANDARD_ALIAS(c17, "iso9899:2017")
+LANGSTANDARD_ALIAS(c17, "c18")
+LANGSTANDARD_ALIAS(c17, "iso9899:2018")
LANGSTANDARD(gnu17, "gnu17",
C, "ISO C 2017 with GNU extensions",
LineComment | C99 | C11 | C17 | Digraphs | GNUMode | HexFloat)
+LANGSTANDARD_ALIAS(gnu17, "gnu18")
// C++ modes
LANGSTANDARD(cxx98, "c++98",
Modified: cfe/trunk/test/Driver/unknown-std.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/unknown-std.c?rev=344749&r1=344748&r2=344749&view=diff
==
--- cfe/trunk/test/Driver/unknown-std.c (original)
+++ cfe/trunk/test/Driver/unknown-std.c Thu Oct 18 10:42:41 2018
@@ -14,8 +14,8 @@
// CHECK-NEXT: note: use 'gnu99' for 'ISO C 1999 with GNU extensions' standard
// CHECK-NEXT: note: use 'c11' or 'iso9899:2011' for 'ISO C 2011' standard
// CHECK-NEXT: note: use 'gnu11' for 'ISO C 2011 with GNU extensions' standard
-// CHECK-NEXT: note: use 'c17' or 'iso9899:2017' for 'ISO C 2017' standard
-// CHECK-NEXT: note: use 'gnu17' for 'ISO C 2017 with GNU extensions' standard
+// CHECK-NEXT: note: use 'c17', 'iso9899:2017', 'c18', or 'iso9899:2018' for
'ISO C 2017' standard
+// CHECK-NEXT: note: use 'gnu17' or 'gnu18' for 'ISO C 2017 with GNU
extensions' standard
// Make sure that no other output is present.
// CHECK-NOT: {{^.+$}}
Modified: cfe/trunk/test/Preprocessor/c17.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/c17.c?rev=344749&r1=344748&r2=344749&view=diff
==
--- cfe/trunk/test/Preprocessor/c17.c (original)
+++ cfe/trunk/test/Preprocessor/c17.c Thu Oct 18 10:42:41 2018
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c18 %s
// expected-no-diagnostics
_Static_assert(__STDC_VERSION__ == 201710L, "Incorrect __STDC_VERSION__");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344750 - Add check-clang-python to the Clang tests directory in IDEs; NFC.
Author: aaronballman
Date: Thu Oct 18 10:47:18 2018
New Revision: 344750
URL: http://llvm.org/viewvc/llvm-project?rev=344750&view=rev
Log:
Add check-clang-python to the Clang tests directory in IDEs; NFC.
Modified:
cfe/trunk/bindings/python/tests/CMakeLists.txt
Modified: cfe/trunk/bindings/python/tests/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/CMakeLists.txt?rev=344750&r1=344749&r2=344750&view=diff
==
--- cfe/trunk/bindings/python/tests/CMakeLists.txt (original)
+++ cfe/trunk/bindings/python/tests/CMakeLists.txt Thu Oct 18 10:47:18 2018
@@ -8,6 +8,7 @@ add_custom_target(check-clang-python
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/..)
set(RUN_PYTHON_TESTS TRUE)
+set_target_properties(check-clang-python PROPERTIES FOLDER "Clang tests")
# Do not try to run if libclang was built with ASan because
# the sanitizer library will likely be loaded too late to perform
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53406: [clangd] Provide excuses for bad code completions, based on diagnostics. C++ API only.
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric.
In some circumstances we provide bad completions or no completions, because of
problems in the code. This can be puzzling and opaque to users.
If we can tell the user that this is the case and why, they'll be happy.
Experiments with go language servers have shown this to be a big win.
Two major problems:
- Sema doesn't provide the information we need
- LSP provides no protocol mechanims, and editors don't have UI for this
This patch attempts to guess the information, looking at diagnostics on the
line.
Other heuristics are possible (e.g. using completion context). It's unclear how
useful or successful they will be. This is mostly a quick hack to test
viability.
This is exposed as an extension of the C++ API only (not bound to LSP).
The idea is to test viability with a non-public editor that happens to have the
right UI already (and doesn't use LSP), and experiment with approaches.
If something fairly simple works well, we'll keep this and expose an LSP
extension.
If it's not useful, we should drop this idea.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53406
Files:
clangd/ClangdLSPServer.cpp
clangd/CodeComplete.cpp
clangd/CodeComplete.h
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2176,6 +2176,50 @@
AllOf(Qualifier("nx::"), Named("Clangd2";
}
+TEST(CompletionTest, Excuses) {
+ struct {
+const char *Code;
+const char *Excuse;
+ } Tests[] = {
+ {
+ R"cpp(
+class X;
+X* x;
+int main() {
+ x->^
+}
+ )cpp",
+ "member access into incomplete type 'X'",
+ },
+ {
+ R"cpp(
+// Error is on same line.
+template struct X;
+X x; ^
+ )cpp",
+ "implicit instantiation of undefined template 'X'",
+ },
+ {
+ R"cpp(
+// Error is on previous line.
+template struct X;
+X x;
+^
+ )cpp",
+ "",
+ },
+ {
+ R"cpp(
+// Just a warning (declaration does not declare anything).
+int; x^
+ )cpp",
+ "",
+ },
+ };
+ for (const auto &Test : Tests)
+EXPECT_EQ(completions(Test.Code).Excuse, Test.Excuse) << Test.Code;
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -196,6 +196,10 @@
std::vector Completions;
bool HasMore = false;
CodeCompletionContext::Kind Context = CodeCompletionContext::CCC_Other;
+
+ // Our best guess at why completions might be poor (human-readable string).
+ // Only set if we suspect completions are poor *and* we know why.
+ std::string Excuse;
};
raw_ostream &operator<<(raw_ostream &, const CodeCompleteResult &);
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -671,6 +671,42 @@
return false;
}
+// Tries to come up with a convincing excuse for our bad completion results,
+// based on the diagnostics near the completion point.
+class ExcuseMaker : public DiagnosticConsumer {
+ unsigned StartOfLine, Offset;
+ std::string &Excuse;
+ SmallString<256> ExcuseBuf;
+
+ public:
+ ExcuseMaker(StringRef Code, unsigned Offset, std::string &Excuse)
+ : Offset(Offset), Excuse(Excuse) {
+ assert(Offset <= Code.size());
+ for (StartOfLine = Offset;
+ StartOfLine > 0 && Code[StartOfLine - 1] != '\n'; --StartOfLine)
+ ;
+ }
+
+ void HandleDiagnostic(DiagnosticsEngine::Level Level,
+ const clang::Diagnostic &Diag) override {
+ // We accept >= error diagnostics, before the cursor but on the same line.
+ if (Level < DiagnosticsEngine::Error || !Diag.hasSourceManager())
+ return;
+ auto& SM = Diag.getSourceManager();
+ auto Loc = SM.getDecomposedLoc(Diag.getLocation());
+ if (Loc.first != SM.getMainFileID())
+ return;
+ unsigned DiagLoc = Loc.second;
+ if (DiagLoc < StartOfLine || DiagLoc > Offset)
+ return;
+ ExcuseBuf.clear();
+ Diag.FormatDiagnostic(ExcuseBuf);
+ }
+
+ ~ExcuseMaker() { Excuse = ExcuseBuf.str(); }
+};
+
+
// The CompletionRecorder captures Sema code-complete output, including context.
// It filters out ignored results (but doesn't apply fuzzy-filtering yet).
// It doesn't do scoring or conver
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
arphaman added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
arphaman wrote:
> ddunbar wrote:
> > What is our feeling w.r.t. _Pragma, which can in theory influence the
> > preprocessor. I'm not sure this model can sanely support it?
> We need to look into that. In the worst case we can always avoid minimizing
> the file if it has a _Pragma anywhere in it.
We also have to handle cases like this one:
foo.h:
```
#define PP _Pragma
```
bar.h:
```
PP ("foo")
```
Unfortunately this can't be handled by just disabling minimization for `foo.h`
as PP will be stripped out from `bar.h`. Furthermore, this pattern is quite
reasonable, so we should expect it in the code we receive. Right now I can't
think of a reasonable solution for this problem.
There's also a more "evil" case:
```
#define P(A, B) A##B
P(_Pra,gma)("foo")
```
It would be reasonable to introduce a warning for the use of `_Pragma` token
that was created using PP token concatenation and just hope that our users
won't use this pattern.
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268372, @lebedev.ri wrote: > In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > > > > > - Apply minor wording nits. > > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not > > > **anything** else (not even `llu`). > > > > > > I'll find out about the DCL16-C recommendation, as I suspect the intent is > > to cover `lu` and `llu` but not `ul` and `ull`. > > > I agree, i've thought so too. > > That will open an interesting question: in `lu`, `l` should be upper-case. > What about `u`? We can't keep it as-is. > We will either consistently upper-case it, or consistently lower-case it. > I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`? I talked to someone at CERT responsible for maintaining DCL16-C to get their opinion on tightening the wording of the rule and their stated intent is: "If the first character is 'ell', it should be capitalized. The other ells need not be, and the yew's need not be capitalized either." e.g., 11lu -> diagnose 11ul -> fine 11llu -> diagnose 11lLu -> diagnose 11Llu -> fine 11ul -> fine That said, the author (and I) agree that it'd be perfectly okay to diagnose things like `11Llu` and recommend `11LLU` as a replacement. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
arphaman added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
arphaman wrote:
> arphaman wrote:
> > ddunbar wrote:
> > > What is our feeling w.r.t. _Pragma, which can in theory influence the
> > > preprocessor. I'm not sure this model can sanely support it?
> > We need to look into that. In the worst case we can always avoid minimizing
> > the file if it has a _Pragma anywhere in it.
> We also have to handle cases like this one:
>
> foo.h:
> ```
> #define PP _Pragma
> ```
>
> bar.h:
> ```
> PP ("foo")
> ```
>
> Unfortunately this can't be handled by just disabling minimization for
> `foo.h` as PP will be stripped out from `bar.h`. Furthermore, this pattern is
> quite reasonable, so we should expect it in the code we receive. Right now I
> can't think of a reasonable solution for this problem.
>
> There's also a more "evil" case:
>
> ```
> #define P(A, B) A##B
>
> P(_Pra,gma)("foo")
> ```
>
> It would be reasonable to introduce a warning for the use of `_Pragma` token
> that was created using PP token concatenation and just hope that our users
> won't use this pattern.
One possible solution to the first issue is to simply fallback to the regular
-Eonly invocation if we detect an include of a file that has a macro with a
`_Pragma` in it. Then we could emit a remark to the user saying that their
build could be faster if they rewrite their code so that this pattern no longer
occurs.
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268372, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > > > > > > > - Apply minor wording nits. > > > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not > > > > **anything** else (not even `llu`). > > > > > > > > > I'll find out about the DCL16-C recommendation, as I suspect the intent > > > is to cover `lu` and `llu` but not `ul` and `ull`. > > > > > > I agree, i've thought so too. > > > > That will open an interesting question: in `lu`, `l` should be upper-case. > > What about `u`? We can't keep it as-is. > > We will either consistently upper-case it, or consistently lower-case it. > > I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`? > > > I talked to someone at CERT responsible for maintaining DCL16-C to get their > opinion on tightening the wording of the rule and their stated intent is: Thank you! > "If the first character is 'ell', it should be capitalized. The other ells > need not be, and the yew's need not be capitalized either." > > e.g., > 11lu -> diagnose > 11ul -> fine > 11llu -> diagnose > 11lLu -> diagnose > 11Llu -> fine > 11ul -> fine > > That said, the author (and I) agree that it'd be perfectly okay to diagnose > things like `11Llu` and recommend `11LLU` as a replacement. Ok, nothing unexpected. So the full revised list is: "L;LL:LU;LLU". Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268569, @lebedev.ri wrote: > In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > > > I talked to someone at CERT responsible for maintaining DCL16-C to get > > their opinion on tightening the wording of the rule and their stated intent > > is: > > > Thank you! > > > "If the first character is 'ell', it should be capitalized. The other ells > > need not be, and the yew's need not be capitalized either." > > > > e.g., > > 11lu -> diagnose > > 11ul -> fine > > 11llu -> diagnose > > 11lLu -> diagnose > > 11Llu -> fine > > 11ul -> fine > > > > That said, the author (and I) agree that it'd be perfectly okay to diagnose > > things like `11Llu` and recommend `11LLU` as a replacement. > > Ok, nothing unexpected. > So the full revised list is: "L;LL:LU;LLU". Agreed. It might be reasonable to add the above as some extra test cases for the CERT check if they're not already covered elsewhere. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268572, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268569, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > > > > > I talked to someone at CERT responsible for maintaining DCL16-C to get > > > their opinion on tightening the wording of the rule and their stated > > > intent is: > > > > > > Thank you! > > > > > "If the first character is 'ell', it should be capitalized. The other > > > ells need not be, and the yew's need not be capitalized either." > > > > > > e.g., > > > 11lu -> diagnose > > > 11ul -> fine > > > 11llu -> diagnose > > > 11lLu -> diagnose > > > 11Llu -> fine > > > 11ul -> fine > > > > > > That said, the author (and I) agree that it'd be perfectly okay to > > > diagnose things like `11Llu` and recommend `11LLU` as a replacement. > > > > Ok, nothing unexpected. > > So the full revised list is: "L;LL:LU;LLU". > > > Agreed. It might be reasonable to add the above as some extra test cases for > the CERT check if they're not already covered elsewhere. Those exist as the test cases for CERT (although, i only test the integer case for CERT, not float.), i will only need to adjust the expected output. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
aaron.ballman added a comment. Hmm, the latest patch only seems to have the changes to the test but not the implementation? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53308: [Fixed Point Arithmetic] Fixed Point to Boolean Cast
rjmccall added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9599 + return false; +return Success(Val.getInt().getBoolValue(), E); + } I know you haven't really done constant-evaluation yet, but I think you should at least be setting up operations like this correctly: 1. There should be an `EvaluateFixedPoint` function that evaluates an expression as a fixed-point value, just like `EvaluateFloat` and the others, which can be structured to use `FixedPointExprEvaluator`. 2. There should be a `getBoolValue` method on `APFixedPoint`. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2026 +return EmitScalarConversion(Visit(E), E->getType(), DestTy, +CE->getExprLoc()); Why are you pushing these casts through `EmitScalarConversion` when the cast kind already tells you which operation you're doing? Repository: rC Clang https://reviews.llvm.org/D53308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri updated this revision to Diff 170108.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.
CERT: also handle "lu", "llu".
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52670
Files:
clang-tidy/cert/CERTTidyModule.cpp
clang-tidy/cert/CMakeLists.txt
clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tidy/readability/UppercaseLiteralSuffixCheck.h
clang-tidy/utils/ASTUtils.cpp
clang-tidy/utils/ASTUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cert-dcl16-c.rst
docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
test/clang-tidy/readability-uppercase-literal-suffix.h
Index: test/clang-tidy/readability-uppercase-literal-suffix.h
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix.h
@@ -0,0 +1,16 @@
+template
+struct integral_constant {
+ static constexpr T value = v;
+ typedef T value_type;
+ typedef integral_constant type; // using injected-class-name
+ constexpr operator value_type() const noexcept { return value; }
+};
+
+using false_type = integral_constant;
+using true_type = integral_constant;
+
+template
+struct is_same : false_type {};
+
+template
+struct is_same : true_type {};
Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S
+
+#include "readability-uppercase-literal-suffix.h"
+
+void integer_suffix() {
+ static constexpr auto v0 = __LINE__; // synthetic
+ static_assert(v0 == 9 || v0 == 5, "");
+
+ static constexpr auto v1 = __cplusplus; // synthetic, long
+
+ static constexpr auto v2 = 1; // no literal
+ static_assert(is_same::value, "");
+ static_assert(v2 == 1, "");
+
+ // Unsigned
+
+ static constexpr auto v3 = 1u;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'u', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v3 = 1u;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}}
+ // CHECK-FIXES: static constexpr auto v3 = 1U;
+ static_assert(is_same::value, "");
+ static_assert(v3 == 1, "");
+
+ static constexpr auto v4 = 1U; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v4 == 1, "");
+
+ // Long
+
+ static constexpr auto v5 = 1l;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'l', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v5 = 1l;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}L{{$}}
+ // CHECK-FIXES: static constexpr auto v5 = 1L;
+ static_assert(is_same::value, "");
+ static_assert(v5 == 1, "");
+
+ static constexpr auto v6 = 1L; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v6 == 1, "");
+
+ // Long Long
+
+ static constexpr auto v7 = 1ll;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'll', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v7 = 1ll;
+ // CHECK-MESSAGES-NEXT: ^~~
+ // CHECK-MESSAGES-NEXT: {{^ *}}LL{{$}}
+ // CHECK-FIXES: static constexpr auto v7 = 1LL;
+ static_assert(is_same::value, "");
+ static_assert(v7 == 1, "");
+
+ static constexpr auto v8 = 1LL; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v8 == 1, "");
+
+ // Unsigned Long
+
+ static constexpr auto v9 = 1ul;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'ul', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1ul;
+ // CHECK-MESSAGES-NEXT: ^~~
+ // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}}
+ // CHECK-FIXES: static
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
lebedev.ri updated this revision to Diff 170109.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
Don't use `auto` in `getModuleOptions()`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52771
Files:
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -0,0 +1,380 @@
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s cppcoreguidelines-non-private-member-variables-in-classes %t -- --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+
+////
+
+// Only data, do not warn
+
+struct S0 {
+ int S0_v0;
+
+public:
+ int S0_v1;
+
+protected:
+ int S0_v2;
+
+private:
+ int S0_v3;
+};
+
+class S1 {
+ int S1_v0;
+
+public:
+ int S1_v1;
+
+protected:
+ int S1_v2;
+
+private:
+ int S1_v3;
+};
+
+////
+
+// All functions are static, do not warn.
+
+struct S2 {
+ static void S2_m0();
+ int S2_v0;
+
+public:
+ static void S2_m1();
+ int S2_v1;
+
+protected:
+ static void S2_m3();
+ int S2_v2;
+
+private:
+ static void S2_m4();
+ int S2_v3;
+};
+
+class S3 {
+ static void S3_m0();
+ int S3_v0;
+
+public:
+ static void S3_m1();
+ int S3_v1;
+
+protected:
+ static void S3_m3();
+ int S3_v2;
+
+private:
+ static void S3_m4();
+ int S3_v3;
+};
+
+////
+
+// union != struct/class. do not diagnose.
+
+union U0 {
+ void U0_m0();
+ int U0_v0;
+
+public:
+ void U0_m1();
+ int U0_v1;
+
+protected:
+ void U0_m2();
+ int U0_v2;
+
+private:
+ void U0_m3();
+ int U0_v3;
+};
+
+////
+
+// Has non-static method with default visibility.
+
+struct S4 {
+ void S4_m0();
+
+ int S4_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v0' has public visibility
+public:
+ int S4_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v1' has public visibility
+protected:
+ int S4_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S4_v2' has protected visibility
+private:
+ int S4_v3;
+};
+
+class S5 {
+ void S5_m0();
+
+ int S5_v0;
+
+public:
+ int S5_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S5_v1' has public visibility
+protected:
+ int S5_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S5_v2' has protected visibility
+private:
+ int S5_v3;
+};
+
+////
+
+// Has non-static method with public visibility.
+
+struct S6 {
+ int S6_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S6_v0' has publi
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Thanks for the changes! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. We can leave the cleanup of the expression evaluator to another change. https://reviews.llvm.org/D52750 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344755 - [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
Author: lebedevri
Date: Thu Oct 18 13:06:40 2018
New Revision: 344755
URL: http://llvm.org/viewvc/llvm-project?rev=344755&view=rev
Log:
[clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT
DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.
All valid combinations of suffixes are supported.
```
auto x = 1; // OK, no suffix.
auto x = 1u; // warning: integer literal suffix 'u' is not upper-case
auto x = 1U; // OK, suffix is uppercase.
...
```
References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241
| CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a
literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52670
Added:
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl16-c.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
clang-tools-extra/trunk/test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix.h
Modified:
clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp?rev=344755&r1=344754&r2=344755&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp Thu Oct 18
13:06:40 2018
@@ -16,6 +16,7 @@
#include "../misc/StaticAssertCheck.h"
#include "../misc/ThrowByValueCatchByReferenceCheck.h"
#include "../performance/MoveConstructorInitCheck.h"
+#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "CommandProcessorCheck.h"
#include "DontModifyStdNamespaceCheck.h"
#include "FloatLoopCounter.h"
@@ -65,6 +66,8 @@ public:
// C checkers
// DCL
CheckFactories.registerCheck("cert-dcl03-c");
+CheckFactories.registerCheck(
+"cert-dcl16-c");
// ENV
CheckFactories.registerCheck("cert-env33-c");
// FLP
@@ -78,6 +81,13 @@ public:
CheckFactories.registerCheck(
"cert-msc32-c");
}
+
+ ClangTidyOptions getModuleOptions() override {
+ClangTidyOptions Options;
+ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
+Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
+return Options;
+ }
};
} // namespace cert
Modified: clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt?rev=344755&r1=344754&r2=344755&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt Thu Oct 18 13:06:40
2018
@@ -24,5 +24,6 @@ add_clang_library(clangTidyCERTModule
clangTidyGoo
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344755: [clang-tidy] Add new
'readability-uppercase-literal-suffix' check (CERT DCL16-C… (authored
by lebedevri, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52670?vs=170108&id=170111#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52670
Files:
clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl16-c.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
clang-tools-extra/trunk/test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix.h
Index: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -35,6 +35,7 @@
#include "../readability/BracesAroundStatementsCheck.h"
#include "../readability/FunctionSizeCheck.h"
#include "../readability/IdentifierNamingCheck.h"
+#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "ExceptionBaseclassCheck.h"
#include "MultiwayPathsCoveredCheck.h"
#include "NoAssemblerCheck.h"
@@ -100,6 +101,8 @@
"hicpp-use-nullptr");
CheckFactories.registerCheck(
"hicpp-use-override");
+CheckFactories.registerCheck(
+"hicpp-uppercase-literal-suffix");
CheckFactories.registerCheck(
"hicpp-vararg");
}
Index: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
@@ -27,6 +27,18 @@
bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM,
const LangOptions &LangOpts,
StringRef FlagName);
+
+// Check if the range is entirely contained within a macro argument.
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+const SourceManager *SM);
+
+// Check if the range contains any locations from a macro expansion.
+bool rangeContainsMacroExpansion(SourceRange Range, const SourceManager *SM);
+
+// Can a fix-it be issued for this whole Range?
+// FIXME: false-negative if the entire range is fully expanded from a macro.
+bool rangeCanBeFixed(SourceRange Range, const SourceManager *SM);
+
} // namespace utils
} // namespace tidy
} // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
@@ -67,6 +67,32 @@
return true;
}
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+const SourceManager *SM) {
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SM &&
+ SM->isMacroArgExpansion(Range.getBegin(),
+ &MacroArgExpansionStartForRangeBegin) &&
+ SM->isMacroArgExpansion(Ra
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
lebedev.ri updated this revision to Diff 170112.
lebedev.ri added a comment.
Rebased to fix a merge conflict in release notes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52771
Files:
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -0,0 +1,380 @@
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s cppcoreguidelines-non-private-member-variables-in-classes %t -- --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+
+////
+
+// Only data, do not warn
+
+struct S0 {
+ int S0_v0;
+
+public:
+ int S0_v1;
+
+protected:
+ int S0_v2;
+
+private:
+ int S0_v3;
+};
+
+class S1 {
+ int S1_v0;
+
+public:
+ int S1_v1;
+
+protected:
+ int S1_v2;
+
+private:
+ int S1_v3;
+};
+
+////
+
+// All functions are static, do not warn.
+
+struct S2 {
+ static void S2_m0();
+ int S2_v0;
+
+public:
+ static void S2_m1();
+ int S2_v1;
+
+protected:
+ static void S2_m3();
+ int S2_v2;
+
+private:
+ static void S2_m4();
+ int S2_v3;
+};
+
+class S3 {
+ static void S3_m0();
+ int S3_v0;
+
+public:
+ static void S3_m1();
+ int S3_v1;
+
+protected:
+ static void S3_m3();
+ int S3_v2;
+
+private:
+ static void S3_m4();
+ int S3_v3;
+};
+
+////
+
+// union != struct/class. do not diagnose.
+
+union U0 {
+ void U0_m0();
+ int U0_v0;
+
+public:
+ void U0_m1();
+ int U0_v1;
+
+protected:
+ void U0_m2();
+ int U0_v2;
+
+private:
+ void U0_m3();
+ int U0_v3;
+};
+
+////
+
+// Has non-static method with default visibility.
+
+struct S4 {
+ void S4_m0();
+
+ int S4_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v0' has public visibility
+public:
+ int S4_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v1' has public visibility
+protected:
+ int S4_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S4_v2' has protected visibility
+private:
+ int S4_v3;
+};
+
+class S5 {
+ void S5_m0();
+
+ int S5_v0;
+
+public:
+ int S5_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S5_v1' has public visibility
+protected:
+ int S5_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S5_v2' has protected visibility
+private:
+ int S5_v3;
+};
+
+////
+
+// Has non-static method with public visibility.
+
+struct S6 {
+ int S6_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S6_v0' has public visibility
+public:
+ void S6_m0()
[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions
xbolva00 added a comment. Is PrecisionConversion acceptable? If not, any suggestions for name? https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344757 - [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
Author: lebedevri
Date: Thu Oct 18 13:16:44 2018
New Revision: 344757
URL: http://llvm.org/viewvc/llvm-project?rev=344757&view=rev
Log:
[clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines,
HICPP)
Summary:
Finds classes that not only contain the data (non-static member variables),
but also have logic (non-static member functions), and diagnoses all member
variables that have any other scope other than `private`. They should be
made `private`, and manipulated exclusively via the member functions.
Optionally, classes with all member variables being `public` could be
ignored, and optionally all `public` member variables could be ignored.
Options
---
* IgnoreClassesWithAllMemberVariablesBeingPublic
Allows to completely ignore classes if **all** the member variables in that
class have `public` visibility.
* IgnorePublicMemberVariables
Allows to ignore (not diagnose) **all** the member variables with `public`
visibility scope.
References:
* MISRA 11-0-1 Member data in non-POD class types shall be private.
*
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c2-use-class-if-the-class-has-an-invariant-use-struct-if-the-data-members-can-vary-independently
*
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-private
*
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rh-protected
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, zinovy.nis, cfe-commits, rnkovacs, nemanjai,
mgorny, xazax.hun, kbarton
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52771
Added:
clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
clang-tools-extra/trunk/test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.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/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=344757&r1=344756&r2=344757&view=diff
==
---
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
(original)
+++
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
Thu Oct 18 13:16:44 2018
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
#include "../misc/UnconventionalAssignOperatorCheck.h"
#include "../readability/MagicNumbersCheck.h"
#include "AvoidGotoCheck.h"
@@ -47,6 +48,8 @@ public:
CheckFactories.registerCheck(
"cppcoreguidelines-narrowing-conversions");
CheckFactories.registerCheck("cppcoreguidelines-no-malloc");
+
CheckFactories.registerCheck(
+"cppcoreguidelines-non-private-member-variables-in-classes");
CheckFactories.registerCheck(
"cppcoreguidelines-owning-memory");
CheckFactories.registerCheck(
@@ -75,6 +78,16 @@ public:
CheckFactories.registerCheck(
"cppcoreguidelines-c-copy-assignment-signature");
}
+
+ ClangTidyOptions getModuleOptions() override {
+ClangTidyOptions Options;
+ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
+
+Opts["cppcoreguidelines-non-private-member-variables-in-classes."
+ "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "1";
+
+return Options;
+ }
};
// Register the LLVMTidyModule using this statically initialized variable.
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=344757&r1=344756&r2=344757&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Thu Oct 18 13:16:44
2018
@@ -6,6 +6,7 @@ add_clang_library(clangTidyMiscModule
MisplacedConstCheck.cpp
NewDeleteOverloadsCheck.cpp
NonCopyableObjects.cpp
+ NonPrivateMemberVariablesInClassesCheck.cpp
RedundantExpressionCheck.cpp
StaticAssertChec
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344757: [clang-tidy] Non-private member variables in classes
(MISRA, CppCoreGuidelines… (authored by lebedevri, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52771?vs=170112&id=170114#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52771
Files:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
clang-tools-extra/trunk/test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Index: clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
+++ clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
@@ -0,0 +1,46 @@
+//===--- NonPrivateMemberVariablesInClassesCheck.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_MISC_NONPRIVATEMEMBERVARIABLESINCLASSESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONPRIVATEMEMBERVARIABLESINCLASSESCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds classes that not only contain the data
+/// (non-static member variables), but also have logic (non-static member
+/// functions), and diagnoses all member variables that have any other scope
+/// other than `private`. They should be made `private`, and manipulated
+/// exclusively via the member functions.
+///
+/// Optionally, classes with all member variables being `public` could be
+/// ignored and optionally all `public` member variables could be ignored.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-non-private-member-variables-in-classes.html
+class NonPrivateMemberVariablesInClassesCheck : public ClangTidyCheck {
+public:
+ NonPrivateMemberVariablesInClassesCheck(StringRef Name,
+ ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const bool IgnoreClassesWithAllMemberVariablesBeingPublic;
+ const bool IgnorePublicMemberVariables;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONPRIVATEMEMBERVARIABLESINCLASSESCHECK_H
Index: clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
@@ -0,0 +1,93 @@
+//===--- NonPrivateMemberVariablesInClassesCheck.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 "NonPrivateMemberVariablesInClassesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+AST_MATCHER(CXXRecordDecl, hasMethods) {
+ return std::distance(Node.method_begin(), Node.method_end()) != 0;
+}
+
+AST_MATCHER(CXXRecordDecl, hasNonStaticMethod) {
+ return hasMethod(unless(isStaticStorageClass()))
+ .matches(Node, Finder, Builder);
+}
+
+AST_MATCHER(CXXRecordDecl, hasNonPublicMemberVariable) {
+ return cxxRecordDecl(has(fieldDecl(unless(isPublic()
+ .matches(Node, Finder, Builder);
+}
+
+AST_POLYMORPHIC_MATCHER_P(boolean, AST_POLYMORPHIC_SUPPORTED_TYPES(Stmt, Decl),
+ bool, Boolean) {
+ return Boolean;
+}
+
+} // namespace
+
+NonPriv
[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div
xbolva00 added a comment.
Second thought, I don't think we should recommend std::size here (maybe it
should be okay for clang static analyzers)
uint32_t data[] = {10, 20, 30, 40};
len = sizeof(data)/sizeof(*data); // "warn" on valid code to recommend
std::size? I dont agree with such behaviour.
@rsmith?
https://reviews.llvm.org/D52949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53410: Add missing PATH_MAX for GNU Hurd support
sylvestre.ledru created this revision.
sylvestre.ledru added a reviewer: rnk.
Herald added a subscriber: krytarowski.
If you have a better place to put them, don't hesitate to let me know :)
Patch by Svante Signell & myself
Repository:
rC Clang
https://reviews.llvm.org/D53410
Files:
lib/Basic/FileManager.cpp
lib/Frontend/ModuleDependencyCollector.cpp
Index: lib/Frontend/ModuleDependencyCollector.cpp
===
--- lib/Frontend/ModuleDependencyCollector.cpp
+++ lib/Frontend/ModuleDependencyCollector.cpp
@@ -99,6 +99,11 @@
}
+// For GNU Hurd
+#if defined(__GNU__) && !defined(PATH_MAX)
+# define PATH_MAX 4096
+#endif
+
// TODO: move this to Support/Path.h and check for HAVE_REALPATH?
static bool real_path(StringRef SrcPath, SmallVectorImpl &RealPath) {
#ifdef LLVM_ON_UNIX
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -511,6 +511,12 @@
UniqueRealFiles.erase(Entry->getUniqueID());
}
+// For GNU Hurd
+#if defined(__GNU__) && !defined(PATH_MAX)
+# define PATH_MAX 4096
+#endif
+
+
void FileManager::GetUniqueIDMapping(
SmallVectorImpl &UIDToFiles) const {
UIDToFiles.clear();
Index: lib/Frontend/ModuleDependencyCollector.cpp
===
--- lib/Frontend/ModuleDependencyCollector.cpp
+++ lib/Frontend/ModuleDependencyCollector.cpp
@@ -99,6 +99,11 @@
}
+// For GNU Hurd
+#if defined(__GNU__) && !defined(PATH_MAX)
+# define PATH_MAX 4096
+#endif
+
// TODO: move this to Support/Path.h and check for HAVE_REALPATH?
static bool real_path(StringRef SrcPath, SmallVectorImpl &RealPath) {
#ifdef LLVM_ON_UNIX
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -511,6 +511,12 @@
UniqueRealFiles.erase(Entry->getUniqueID());
}
+// For GNU Hurd
+#if defined(__GNU__) && !defined(PATH_MAX)
+# define PATH_MAX 4096
+#endif
+
+
void FileManager::GetUniqueIDMapping(
SmallVectorImpl &UIDToFiles) const {
UIDToFiles.clear();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344758 - [clang-tidy] readability-uppercase-literal-suffix: specify target for fp tests
Author: lebedevri Date: Thu Oct 18 13:27:10 2018 New Revision: 344758 URL: http://llvm.org/viewvc/llvm-project?rev=344758&view=rev Log: [clang-tidy] readability-uppercase-literal-suffix: specify target for fp tests __float128 isn't universally avaliable. Modified: clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp Modified: clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp?rev=344758&r1=344757&r2=344758&view=diff == --- clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp Thu Oct 18 13:27:10 2018 @@ -1,7 +1,7 @@ -// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -target x86_64-pc-linux-gnu -I %S // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -target x86_64-pc-linux-gnu -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -target x86_64-pc-linux-gnu -I %S #include "readability-uppercase-literal-suffix.h" Modified: clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp?rev=344758&r1=344757&r2=344758&view=diff == --- clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp Thu Oct 18 13:27:10 2018 @@ -1,7 +1,7 @@ -// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -target x86_64-pc-linux-gnu -I %S // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -target x86_64-pc-linux-gnu -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -target x86_64-pc-linux-gnu -I %S #include "readability-uppercase-literal-suffix.h" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker
xbolva00 added inline comments. Comment at: lib/Driver/ToolChains/Gnu.cpp:241 + case llvm::Triple::thumbeb: +IsBigEndian = true; + case llvm::Triple::arm: Gnu.cpp:241:17: warning: this statement may fall through [-Wimplicit-fallthrough=] IsBigEndian = true; Repository: rC Clang https://reviews.llvm.org/D52784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344759 - [Diagnostics] Check for integer overflow in array size expressions
Author: xbolva00
Date: Thu Oct 18 13:49:06 2018
New Revision: 344759
URL: http://llvm.org/viewvc/llvm-project?rev=344759&view=rev
Log:
[Diagnostics] Check for integer overflow in array size expressions
Summary: Fixes PR27439
Reviewers: rsmith, Rakete
Reviewed By: rsmith
Subscribers: Rakete, cfe-commits
Differential Revision: https://reviews.llvm.org/D52750
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/integer-overflow.c
Modified: cfe/trunk/include/clang/AST/Expr.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=344759&r1=344758&r2=344759&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Oct 18 13:49:06 2018
@@ -631,8 +631,13 @@ public:
/// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
/// integer. This must be called on an expression that constant folds to an
/// integer.
- llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx,
-SmallVectorImpl *Diag = nullptr)
const;
+ llvm::APSInt EvaluateKnownConstInt(
+ const ASTContext &Ctx,
+ SmallVectorImpl *Diag = nullptr) const;
+
+ llvm::APSInt EvaluateKnownConstIntCheckOverflow(
+ const ASTContext &Ctx,
+ SmallVectorImpl *Diag = nullptr) const;
void EvaluateForOverflow(const ASTContext &Ctx) const;
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=344759&r1=344758&r2=344759&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Oct 18 13:49:06 2018
@@ -10851,6 +10851,19 @@ APSInt Expr::EvaluateKnownConstInt(const
return EvalResult.Val.getInt();
}
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext &Ctx, SmallVectorImpl *Diag) const {
+ EvalResult EvalResult;
+ EvalResult.Diag = Diag;
+ EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+ bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+ (void)Result;
+ assert(Result && "Could not evaluate expression");
+ assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+ return EvalResult.Val.getInt();
+}
+
void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
bool IsConst;
EvalResult EvalResult;
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=344759&r1=344758&r2=344759&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Oct 18 13:49:06 2018
@@ -14105,7 +14105,7 @@ Sema::VerifyIntegerConstantExpression(Ex
// in the non-ICE case.
if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
if (Result)
- *Result = E->EvaluateKnownConstInt(Context);
+ *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
return E;
}
Modified: cfe/trunk/test/Sema/integer-overflow.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/integer-overflow.c?rev=344759&r1=344758&r2=344759&view=diff
==
--- cfe/trunk/test/Sema/integer-overflow.c (original)
+++ cfe/trunk/test/Sema/integer-overflow.c Thu Oct 18 13:49:06 2018
@@ -172,6 +172,9 @@ void check_integer_overflows_in_function
// expected-warning@+1 {{overflow in expression; result is 536870912 with type
'int'}}
(void)f2(0, f0(4608 * 1024 * 1024));
}
+void check_integer_overflows_in_array_size() {
+ int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression;
result is 536870912 with type 'int'}}
+}
struct s {
unsigned x;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
