[PATCH] D51200: Introduce per-callsite inline intrinsics
rsmith added a comment. Please also teach constant expression evaluation (lib/AST/ExprConstant.cpp) to look through these builtins when evaluating. https://reviews.llvm.org/D51200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
kadircet updated this revision to Diff 163971.
kadircet added a comment.
- Update comments.
Repository:
rC Clang
https://reviews.llvm.org/D51038
Files:
include/clang/Parse/Parser.h
lib/Parse/ParseExpr.cpp
test/CodeCompletion/function-overloads-inside-param.cpp
test/CodeCompletion/function-overloads.cpp
test/Index/complete-block-property-assignment.m
Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,15 @@
// RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck
-check-prefix=CHECK-NO %s
// RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck
-check-prefix=CHECK-NO %s
// RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck
-check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck
-check-prefix=CHECK-NO %s
// RUN: c-index-test -code-completion-at=%s:56:15 %s | FileCheck
-check-prefix=CHECK-NO %s
// CHECK-NO: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText
onAction} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText
onEventHandler} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText
onReadonly} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText
processEvent} (35)
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: FunctionDecl:{ResultType void}{TypedText func}{LeftParen
(}{Placeholder int x}{RightParen )} (50)
@end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+ ::f(
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - |
FileCheck -check-prefix=CHECK-CC1 %s
+ // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+ // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: test/CodeCompletion/function-overloads-inside-param.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads-inside-param.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+ ::f(1
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:8 %s -o - |
FileCheck -check-prefix=CHECK-CC1 %s
+ // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+ // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1658,11 +1658,21 @@
if (OpKind == tok::l_paren || !LHS.isInvalid()) {
if (Tok.isNot(tok::r_paren)) {
- if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
- PT.getOpenLocation());
- })) {
+ auto Completer = [&] {
+if (CalledOverloadCompletion)
+ return;
+Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
+ PT.getOpenLocation());
+CalledOverloadCompletion = true;
+ };
+ if (ParseExpressionList(ArgExprs, CommaLocs, Completer)) {
(void)Actions.CorrectDelayedTyposInExpr(LHS);
+// If we got an error when parsing expression list, we don't call
+// the CodeCompleteCall handler inside the parser. So call it here
+// to make sure we get overload suggestions even when we are in the
+// middle of a parameter.
+if (PP.isCodeCompletionReached())
+ Completer();
LHS = ExprError();
} else if (LHS.isInvalid()) {
for (auto : ArgExprs)
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -214,6 +214,10 @@
/// should not be set directly.
bool InMessageExpression;
+ /// Gets set to true after calling CodeCompleteCall, it is for a workaround
to
+ /// make sure CodeComleteCall is only called at the deepest level.
+ bool CalledOverloadCompletion = false;
+
/// The "depth" of the template parameters currently being parsed.
unsigned TemplateParameterDepth;
Index: test/Index/complete-block-property-assignment.m
[PATCH] D51038: [clang] Make sure codecompletion is called for calls even when inside a token.
kadircet updated this revision to Diff 163970.
kadircet added a comment.
- Update tests.
- Update comment.
- Rebase.
Repository:
rC Clang
https://reviews.llvm.org/D51038
Files:
include/clang/Parse/Parser.h
lib/Parse/ParseExpr.cpp
test/CodeCompletion/function-overloads-inside-param.cpp
test/CodeCompletion/function-overloads.cpp
test/Index/complete-block-property-assignment.m
Index: test/Index/complete-block-property-assignment.m
===
--- test/Index/complete-block-property-assignment.m
+++ test/Index/complete-block-property-assignment.m
@@ -60,13 +60,15 @@
// RUN: c-index-test -code-completion-at=%s:51:16 %s | FileCheck
-check-prefix=CHECK-NO %s
// RUN: c-index-test -code-completion-at=%s:52:23 %s | FileCheck
-check-prefix=CHECK-NO %s
// RUN: c-index-test -code-completion-at=%s:53:12 %s | FileCheck
-check-prefix=CHECK-NO %s
-// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck
-check-prefix=CHECK-NO %s
// RUN: c-index-test -code-completion-at=%s:56:15 %s | FileCheck
-check-prefix=CHECK-NO %s
// CHECK-NO: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType Obj *}{TypedText obj} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType void (^)(Obj *)}{TypedText
onAction} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType FooBlock}{TypedText
onEventHandler} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType void (^)(int *)}{TypedText
onReadonly} (35)
// CHECK-NO-NEXT: ObjCPropertyDecl:{ResultType int (^)(int)}{TypedText
processEvent} (35)
+// RUN: c-index-test -code-completion-at=%s:54:15 %s | FileCheck
-check-prefix=CHECK-NO1 %s
+// CHECK-NO1: ObjCPropertyDecl:{ResultType int}{TypedText foo} (35)
+// CHECK-NO1-NEXT: FunctionDecl:{ResultType void}{TypedText func}{LeftParen
(}{Placeholder int x}{RightParen )} (50)
@end
Index: test/CodeCompletion/function-overloads.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+ ::f(
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - |
FileCheck -check-prefix=CHECK-CC1 %s
+ // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+ // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: test/CodeCompletion/function-overloads-inside-param.cpp
===
--- /dev/null
+++ test/CodeCompletion/function-overloads-inside-param.cpp
@@ -0,0 +1,8 @@
+void f(int i, int j = 2, int k = 5);
+void f(float x, float y...);
+
+void test() {
+ ::f(1
+ // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:8 %s -o - |
FileCheck -check-prefix=CHECK-CC1 %s
+ // CHECK-CC1: f(<#int i#>{#, <#int j = 2#>{#, <#int k = 5#>#}#})
+ // CHECK-CC1: f(<#float x#>, <#float y, ...#>)
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -1658,11 +1658,21 @@
if (OpKind == tok::l_paren || !LHS.isInvalid()) {
if (Tok.isNot(tok::r_paren)) {
- if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
-Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
- PT.getOpenLocation());
- })) {
+ auto Completer = [&] {
+if (CalledOverloadCompletion)
+ return;
+Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs,
+ PT.getOpenLocation());
+CalledOverloadCompletion = true;
+ };
+ if (ParseExpressionList(ArgExprs, CommaLocs, Completer)) {
(void)Actions.CorrectDelayedTyposInExpr(LHS);
+// If we got an error when parsing expression list, we don't call
+// the CodeCompleteCall handler inside the parser. So call it here
+// to make sure we get overload suggestions even when we are in the
+// middle of a parameter.
+if (PP.isCodeCompletionReached())
+ Completer();
LHS = ExprError();
} else if (LHS.isInvalid()) {
for (auto : ArgExprs)
Index: include/clang/Parse/Parser.h
===
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -214,6 +214,10 @@
/// should not be set directly.
bool InMessageExpression;
+ /// Gets set to true after calling CodeCompleteCall, it is for a hack to make
+ /// signature help working even when it is triggered inside a token.
+ bool CalledOverloadCompletion = false;
+
/// The "depth" of the template parameters currently being parsed.
unsigned TemplateParameterDepth;
Index: test/Index/complete-block-property-assignment.m
[PATCH] D51039: [clangd] Add unittests for D51038
kadircet updated this revision to Diff 163968.
kadircet added a comment.
Rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51039
Files:
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1882,6 +1882,56 @@
AllOf(Named("Func"), HasInclude("\"foo.h\""),
Not(InsertInclude();
}
+TEST(SignatureHelpTest, InsideArgument) {
+ {
+const auto Results = signatures(R"cpp(
+ void foo(int x);
+ void foo(int x, int y);
+ int main() { foo(1+^); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+ }
+ {
+const auto Results = signatures(R"cpp(
+ void foo(int x);
+ void foo(int x, int y);
+ int main() { foo(1^); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+ }
+ {
+const auto Results = signatures(R"cpp(
+ void foo(int x);
+ void foo(int x, int y);
+ int main() { foo(1^0); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+ }
+ {
+const auto Results = signatures(R"cpp(
+ void foo(int x);
+ void foo(int x, int y);
+ int bar(int x, int y);
+ int main() { bar(foo(2, 3^)); }
+)cpp");
+EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo(int x, int y) ->
void",
+{"int x", "int y"})));
+EXPECT_EQ(1, Results.activeParameter);
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1882,6 +1882,56 @@
AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude();
}
+TEST(SignatureHelpTest, InsideArgument) {
+ {
+const auto Results = signatures(R"cpp(
+ void foo(int x);
+ void foo(int x, int y);
+ int main() { foo(1+^); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+ }
+ {
+const auto Results = signatures(R"cpp(
+ void foo(int x);
+ void foo(int x, int y);
+ int main() { foo(1^); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+ }
+ {
+const auto Results = signatures(R"cpp(
+ void foo(int x);
+ void foo(int x, int y);
+ int main() { foo(1^0); }
+)cpp");
+EXPECT_THAT(
+Results.signatures,
+ElementsAre(Sig("foo(int x) -> void", {"int x"}),
+Sig("foo(int x, int y) -> void", {"int x", "int y"})));
+EXPECT_EQ(0, Results.activeParameter);
+ }
+ {
+const auto Results = signatures(R"cpp(
+ void foo(int x);
+ void foo(int x, int y);
+ int bar(int x, int y);
+ int main() { bar(foo(2, 3^)); }
+)cpp");
+EXPECT_THAT(Results.signatures, ElementsAre(Sig("foo(int x, int y) -> void",
+{"int x", "int y"})));
+EXPECT_EQ(1, Results.activeParameter);
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50947: lambdas in modules: make PendingFakeDefinitionData a set
rsmith added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:1766 + +// FIXME: handle serialized lambdas assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?"); Did you mean to add this FIXME? Comment at: lib/Serialization/ASTReaderDecl.cpp:4246 OldDD && (OldDD->Definition != RD || !Reader.PendingFakeDefinitionData.count(OldDD)); RD->setParamDestroyedInCallee(Record.readInt()); This does not appear to be NFC: the `FakeLoaded` vs absent-from-map distinction matters here. I think the idea here is that we still need to load the lexical contents of the declaration of the class that we pick to be the definition even if we've already found and merged another definition into it. That said, if this //is// necessary, we should have some test coverage for it, and based on this change not breaking any of our tests, we evidently don't. :( I'll try this change out against our codebase and see if I can find a testcase. Repository: rC Clang https://reviews.llvm.org/D50947 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51200: Introduce per-callsite inline intrinsics
rjmccall added a comment.
In https://reviews.llvm.org/D51200#1223768, @kuhar wrote:
> In https://reviews.llvm.org/D51200#1223752, @rsmith wrote:
>
> > +rjmccall for CodeGen changes
>
>
> @rsmith @rjmccall
> I have one high-level question about the CodeGen part that I wasn't able to
> figure out: is it possible to bail out of custom CodeGen in CGBuiltin and
> somehow say: emit whatever is inside (IE treat the builtin call node as a
> no-op)?
You can just emit the sub-expression. Make sure you pass down the
`ReturnValueSlot`, and make sure you test C++ calls that return references
where the result is really treated as an l-value.
I've commented about the language design on your RFC thread.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8240
+def err_argument_to_inline_intrinsic_block_call : Error<
+ "argument to %0 must not be a block call">;
+
This seems pretty trivial to add.
Comment at: include/clang/CodeGen/CGFunctionInfo.h:517
+ unsigned InlineCall : 2;
+
Please don't propagate this information through the `CGFunctionInfo`. I really
want this type to be *less* site-specific, not *more*.
Comment at: lib/CodeGen/CGBuiltin.cpp:2998
+ case Builtin::BI__builtin_no_inline:
+ case Builtin::BI__builtin_always_inline: {
+CGFunctionInfo::CallInlineKind CIK =
I wonder if it would make more sense to give this its own `Expr` subclass. We
do that with several other "builtin" calls, like `__builtin_choose_expr` and
`__builtin_offsetof`. That would avoid the problem of ending up in
`CGBuiltin`, and it would make it easier to recognize and look through these
annotation calls in Sema or IRGen.
Comment at: lib/CodeGen/CodeGenFunction.h:3570
+ CGFunctionInfo::CallInlineKind CIK =
+ CGFunctionInfo::CallInlineKind::DefaultInline);
RValue EmitCall(const CGFunctionInfo , const CGCallee ,
I feel like the type of this argument should definitely be generalized to not
just be about inlining.
https://reviews.llvm.org/D51200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51608: [modules] when deserializing method, ensure it has correct exception spec
rsmith added a comment.
I think this is addressing a symptom rather than the cause. The bug appears to
be that when parsing a.h, we end up with inconsistent exception specifications
along the redeclaration chain. It looks like
`Sema::AdjustDestructorExceptionSpec` doesn't do the right thing when
befriending a destructor. Here's a non-modules testcase based on yours that
demonstrates either the same problem or at least a closely-related one:
struct B {
virtual ~B();
struct C { friend B::~B() noexcept; };
};
We produce a bogus error here ("exception specification in declaration does not
match previous declaration"). But we think the exception specifications match
if class `C` is moved outside class `B`.
Generally, we skip checking the exception specification for a destructor until
we get to the end of the class. But when the destructor declaration is a friend
declaration for a destructor of an enclosing class, we need to instead delay
until that enclosing class is complete. I expect the logic to do that is what's
missing here.
Repository:
rC Clang
https://reviews.llvm.org/D51608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51507: Allow all supportable attributes to be used with #pragma clang attribute.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341437: Allow all supportable non-type attributes to be used
with #pragma clang… (authored by rsmith, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D51507
Files:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/test/Parser/pragma-attribute.cpp
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -545,7 +545,6 @@
let ASTNode = 0;
let SemaHandler = 0;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
//
@@ -564,15 +563,13 @@
let Spellings = [Clang<"address_space">];
let Args = [IntArgument<"AddressSpace">];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Alias : Attr {
let Spellings = [GCC<"alias">];
let Args = [StringArgument<"Aliasee">];
let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Aligned : InheritableAttr {
@@ -585,7 +582,6 @@
Keyword<"_Alignas">]>,
Accessor<"isDeclspec",[Declspec<"align">]>];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def AlignValue : Attr {
@@ -613,14 +609,12 @@
let Spellings = [];
let SemaHandler = 0;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def AlwaysInline : InheritableAttr {
let Spellings = [GCC<"always_inline">, Keyword<"__forceinline">];
let Subjects = SubjectList<[Function]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Artificial : InheritableAttr {
@@ -665,8 +659,8 @@
// vendor namespace, or should it use a vendor namespace specific to the
// analyzer?
let Spellings = [GNU<"analyzer_noreturn">];
+ // TODO: Add subject list.
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Annotate : InheritableParamAttr {
@@ -709,7 +703,6 @@
let Args = [StringArgument<"Label">];
let SemaHandler = 0;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Availability : InheritableAttr {
@@ -776,7 +769,6 @@
let Spellings = [Clang<"blocks">];
let Args = [EnumArgument<"Type", "BlockType", ["byref"], ["ByRef"]>];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Bounded : IgnoredAttr {
@@ -795,7 +787,6 @@
let Spellings = [GCC<"cdecl">, Keyword<"__cdecl">, Keyword<"_cdecl">];
// let Subjects = [Function, ObjCMethod];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
// cf_audited_transfer indicates that the given function has been
@@ -806,7 +797,6 @@
let Spellings = [Clang<"cf_audited_transfer">];
let Subjects = SubjectList<[Function], ErrorDiag>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
// cf_unknown_transfer is an explicit opt-out of cf_audited_transfer.
@@ -816,64 +806,55 @@
let Spellings = [Clang<"cf_unknown_transfer">];
let Subjects = SubjectList<[Function], ErrorDiag>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def CFReturnsRetained : InheritableAttr {
let Spellings = [Clang<"cf_returns_retained">];
// let Subjects = SubjectList<[ObjCMethod, ObjCProperty, Function]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def CFReturnsNotRetained : InheritableAttr {
let Spellings = [Clang<"cf_returns_not_retained">];
// let Subjects = SubjectList<[ObjCMethod, ObjCProperty, Function]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def CFConsumed : InheritableParamAttr {
let Spellings = [Clang<"cf_consumed">];
let Subjects = SubjectList<[ParmVar]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Cleanup : InheritableAttr {
let Spellings = [GCC<"cleanup">];
let Args = [FunctionArgument<"FunctionDecl">];
let Subjects = SubjectList<[LocalVar]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Cold : InheritableAttr {
let Spellings = [GCC<"cold">];
let Subjects = SubjectList<[Function]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Common : InheritableAttr {
let Spellings = [GCC<"common">];
let Subjects = SubjectList<[Var]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Const : InheritableAttr {
let Spellings = [GCC<"const">, GCC<"__const">];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Constructor : InheritableAttr {
let Spellings =
r341437 - Allow all supportable non-type attributes to be used with #pragma clang attribute.
Author: rsmith
Date: Tue Sep 4 17:28:57 2018
New Revision: 341437
URL: http://llvm.org/viewvc/llvm-project?rev=341437=rev
Log:
Allow all supportable non-type attributes to be used with #pragma clang
attribute.
Summary:
We previously disallowed use of undocumented attributes with #pragma clang
attribute, but the justification for doing so was weak and it prevented many
reasonable use cases.
Reviewers: aaron.ballman, arphaman
Subscribers: cfe-commits, rnk, benlangmuir, dexonsmith, erik.pilkington
Differential Revision: https://reviews.llvm.org/D51507
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/test/Parser/pragma-attribute.cpp
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=341437=341436=341437=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Sep 4 17:28:57 2018
@@ -545,7 +545,6 @@ class IgnoredAttr : Attr {
let ASTNode = 0;
let SemaHandler = 0;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
//
@@ -564,7 +563,6 @@ def AddressSpace : TypeAttr {
let Spellings = [Clang<"address_space">];
let Args = [IntArgument<"AddressSpace">];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Alias : Attr {
@@ -572,7 +570,6 @@ def Alias : Attr {
let Args = [StringArgument<"Aliasee">];
let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Aligned : InheritableAttr {
@@ -585,7 +582,6 @@ def Aligned : InheritableAttr {
Keyword<"_Alignas">]>,
Accessor<"isDeclspec",[Declspec<"align">]>];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def AlignValue : Attr {
@@ -613,14 +609,12 @@ def AlignMac68k : InheritableAttr {
let Spellings = [];
let SemaHandler = 0;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def AlwaysInline : InheritableAttr {
let Spellings = [GCC<"always_inline">, Keyword<"__forceinline">];
let Subjects = SubjectList<[Function]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Artificial : InheritableAttr {
@@ -665,8 +659,8 @@ def AnalyzerNoReturn : InheritableAttr {
// vendor namespace, or should it use a vendor namespace specific to the
// analyzer?
let Spellings = [GNU<"analyzer_noreturn">];
+ // TODO: Add subject list.
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Annotate : InheritableParamAttr {
@@ -709,7 +703,6 @@ def AsmLabel : InheritableAttr {
let Args = [StringArgument<"Label">];
let SemaHandler = 0;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Availability : InheritableAttr {
@@ -776,7 +769,6 @@ def Blocks : InheritableAttr {
let Spellings = [Clang<"blocks">];
let Args = [EnumArgument<"Type", "BlockType", ["byref"], ["ByRef"]>];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Bounded : IgnoredAttr {
@@ -795,7 +787,6 @@ def CDecl : DeclOrTypeAttr {
let Spellings = [GCC<"cdecl">, Keyword<"__cdecl">, Keyword<"_cdecl">];
// let Subjects = [Function, ObjCMethod];
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
// cf_audited_transfer indicates that the given function has been
@@ -806,7 +797,6 @@ def CFAuditedTransfer : InheritableAttr
let Spellings = [Clang<"cf_audited_transfer">];
let Subjects = SubjectList<[Function], ErrorDiag>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
// cf_unknown_transfer is an explicit opt-out of cf_audited_transfer.
@@ -816,28 +806,24 @@ def CFUnknownTransfer : InheritableAttr
let Spellings = [Clang<"cf_unknown_transfer">];
let Subjects = SubjectList<[Function], ErrorDiag>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def CFReturnsRetained : InheritableAttr {
let Spellings = [Clang<"cf_returns_retained">];
// let Subjects = SubjectList<[ObjCMethod, ObjCProperty, Function]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def CFReturnsNotRetained : InheritableAttr {
let Spellings = [Clang<"cf_returns_not_retained">];
// let Subjects = SubjectList<[ObjCMethod, ObjCProperty, Function]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def CFConsumed : InheritableParamAttr {
let Spellings = [Clang<"cf_consumed">];
let Subjects = SubjectList<[ParmVar]>;
let Documentation = [Undocumented];
- let PragmaAttributeSupport = 0;
}
def Cleanup : InheritableAttr {
@@ -845,27 +831,23 @@ def Cleanup :
[PATCH] D51649: [Sema] Don't warn about omitting unavailable enum constants in a switch
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang https://reviews.llvm.org/D51649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space
mcgrathr added inline comments.
Comment at: lib/Parse/ParseDecl.cpp:244-252
+// If this was declared in a macro, attatch the macro IdentifierInfo to the
+// parsed attribute.
+for (const auto : PP.getAttributeMacros()) {
+ if (SourceLocInSourceRange(AttrTok.getLocation(), MacroPair.first,
+ PP.getSourceManager())) {
+ApplyMacroIIToParsedAttrs(attrs, NumParsedAttrs, MacroPair.second);
+break;
rsmith wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > You shouldn't do this if `NumParsedAttrs != 1`. We're only looking for a
> > > macro that exactly covers one attribute.
> > >
> > > (Also, your computation of the number of attributes in the attribute list
> > > is not correct in the presence of late-parsed attributes.)
> > One of the things we would like is for this to cover more than one
> > attribute in the attribute list since in sparse, `address_space` is
> > sometimes used with `noderef`.
> >
> > So given `# define __user __attribute__((noderef, address_space(1)))`,
> > `__user` would be saved into the `ParsedAttr` made for `noderef` and
> > `address_space`.
> >
> > What would be the appropriate way to track newly added attributes into the
> > `ParsedAttributes`, including late-parsed attributes?
> > One of the things we would like is for this to cover more than one
> > attribute in the attribute list since in sparse, `address_space` is
> > sometimes used with `noderef`.
>
> Hold on, this is a new requirement compared to what we'd previously discussed
> (giving a name to an address space). How important is this use case to you?
>
> I don't think it's a reasonable AST model to assign a macro identifier to an
> `AttributedType` if the macro defines multiple attributes. If you really want
> to handle that use case, I think that an identifier on the `AttributedType`
> is the wrong way to model it, and we should instead be creating a new type
> sugar node representing a type decoration written as a macro.
>
> Assuming you want to go ahead with the current patch direction (at least in
> the short term), please add the "only one attribute in the macro" check.
A single macro that uses multiple attributes is the central use case for us.
It would be fine if it's constrained to a single __attribute__ clause (or
[[...]] clause) with the attributes comma-separated, as in __attribute__((foo,
bar)) as opposed to two separate __attribute__((foo)) __attribute__((bar)) in
the macro, if that matters. It's even fine if it's constrained to the macro
being nothing but the __attribute__((foo, bar)) clause (aside from whitespace
and comments).
Leo can decide how he wants to proceed as far as incremental implementation.
But we won't have a real-world use for the feature only covering a single
attribute.
We'll start using when it can cover the cases like the Linux __user and __iomem
examples (address_space + noderef).
Repository:
rC Clang
https://reviews.llvm.org/D51329
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space
rsmith added a comment.
I think this patch is nearly ready to commit. However... if you want to handle
macros that specify a sequence of attributes, we should switch to treating the
macro name as a separate type sugar node rather than modeling it as a name for
the `AttributedType` node. Up to you how we proceed from here -- if you want to
land this patch with a one-attribute-in-the-macro restriction, that seems fine;
if you'd prefer to go straight to a more general approach where we add a new
type sugar representation for a single macro describing (potentially) multiple
attributes, that's fine too.
Comment at: lib/AST/ASTDiagnostic.cpp:78
+AttributedType::getNullabilityAttrKind(*nullability), RT, RT,
+
FT->getReturnType()->getAs()->getMacroIdentifier());
}
Use `castAs` here and below (because it would be a programming error if the
type isn't an `AttributedType`).
Comment at: lib/AST/ASTImporter.cpp:991
+ T->getAttrKind(), ToModifiedType, ToEquivalentType,
+ T->getMacroIdentifier());
}
You need to import the identifier; the source and destination ASTs have
different identifier tables. (`Importer.Import(T->getMacroIdentifier())`).
Comment at: lib/AST/TypePrinter.cpp:1370
+
+// Remove the underlying address space so it won't be printed.
+SplitQualType SplitTy = T->getModifiedType().split();
leonardchan wrote:
> rsmith wrote:
> > leonardchan wrote:
> > > rsmith wrote:
> > > > leonardchan wrote:
> > > > > rsmith wrote:
> > > > > > This is unnecessary; just print the modified type here. (The
> > > > > > modified type by definition does not have the attribute applied to
> > > > > > it.)
> > > > > When you say the modified type, do you mean just the type without
> > > > > it's qualifiers? I wasn't sure if removing all the qualifiers would
> > > > > suffice since there were also other non-address_space qualifiers
> > > > > that could be printed.
> > > > I mean `T->getModifiedType()`, which tracks what the type was before
> > > > the attribute was applied.
> > > Oh, I understand that you meant `T->getModifiedType()`. This is a special
> > > case when printing the `address_space` since even though the attribute is
> > > applied and printed here, when we reach the qualifiers of the modified
> > > type, the address space will still be printed unless we remove it here.
> > >
> > > I'm not sure if there's a better way to do this though.
> > If the address space qualifiers are present on the modified type, then
> > that's a bug in the creation of the `AttributedType`. And indeed looking at
> > r340765, I see we are passing the wrong type in as the modified type when
> > creating the `AttributedType`. Please fix that, and then just use
> > `getModifiedType` here.
> Making another patch for this.
Incidentally, the last two arguments to the `AddressSpaceAttr` constructor in
that patch (address space and spelling list index) are also reversed.
Comment at: lib/Parse/ParseDecl.cpp:138
while (Tok.is(tok::kw___attribute)) {
+Token AttrTok = Tok;
+unsigned OldNumAttrs = attrs.size();
No need to copy the entire `Token` here, you only use its location before. It's
also idiomatic to fold this into consuming the token:
`SourceLocation AttrTokLoc = ConsumeToken();`
Comment at: lib/Parse/ParseDecl.cpp:215-220
+bool AttrStartIsInMacro =
+(AttrTokLoc.isMacroID() && Lexer::isAtStartOfMacroExpansion(
+ AttrTokLoc, SrcMgr, PP.getLangOpts()));
+bool AttrEndIsInMacro =
+(Loc.isMacroID() &&
+ Lexer::isAtEndOfMacroExpansion(Loc, SrcMgr, PP.getLangOpts()));
You're only looking at the innermost macro expansion; I still think we should
take the outermost one. (You can iteratively walk the expansion locations of
the start and end locations (`SourceManager::getExpansionLocStart`/`End`) to
build a list of `FileID`s representing macro expansions that each of them is
included in, and then take the last common such `FileID` that represents an
object-like macro. You can use `SourceManager::getSLocEntry` to get the
corresponding `ExpansionInfo` for the macro, `isFunctionMacroExpansion` to
determine if it's a function-like or object-like macro, and once you know it's
object-like, take the spelling locations of the `ExpansionLocStart` and
`ExpansionLocEnd` to find the name of the macro.
Comment at: lib/Parse/ParseDecl.cpp:244-252
+// If this was declared in a macro, attatch the macro IdentifierInfo to the
+// parsed attribute.
+for (const auto : PP.getAttributeMacros()) {
+ if (SourceLocInSourceRange(AttrTok.getLocation(), MacroPair.first,
+ PP.getSourceManager())) {
+ApplyMacroIIToParsedAttrs(attrs, NumParsedAttrs, MacroPair.second);
+
[PATCH] D51391: [clang-cl,PCH] Add support for #pragma hdrstop
mikerice added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:2862 + Opts.PCHWithHdrStopCreate = + Args.getLastArgValue(OPT_pch_through_hdrstop_EQ) == "create"; Opts.PCHThroughHeader = Args.getLastArgValue(OPT_pch_through_header_EQ); hans wrote: > hmm, what if the value is not "create", but also not "use" but something > else? maybe that should be diagnosed, or maybe the flag should be split into > two? I am not totally happy with this but I thought one option might be a little better than two. It would be equivalent to create two options OPT_pch_through_hdrstop_create and OPT_pch_through_hdrstop_use if that seems better (or more usual) to everyone. I added a diagnostic if the value is not "create" or "use". A user should never see that though if the front end and driver as in sync. Comment at: lib/Lex/Pragma.cpp:904 +CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof); +CurLexer->cutOffLexing(); + } hans wrote: > Nice! I have to give Nico credit for this. It's from his patch that I based these changes. https://reviews.llvm.org/D51391 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51391: [clang-cl,PCH] Add support for #pragma hdrstop
mikerice updated this revision to Diff 163946.
mikerice marked 7 inline comments as done.
mikerice added a comment.
Thanks for the review. Updated based on comments from Hans.
https://reviews.llvm.org/D51391
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Basic/DiagnosticLexKinds.td
include/clang/Driver/CC1Options.td
include/clang/Lex/Preprocessor.h
include/clang/Lex/PreprocessorOptions.h
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Lex/PPDirectives.cpp
lib/Lex/Pragma.cpp
lib/Lex/Preprocessor.cpp
lib/Parse/ParseAST.cpp
test/Driver/cl-pch.cpp
test/PCH/Inputs/pch-hdrstop-use.cpp
test/PCH/Inputs/pch-no-hdrstop-use.cpp
test/PCH/pch-hdrstop-err.cpp
test/PCH/pch-hdrstop-warn.cpp
test/PCH/pch-hdrstop.cpp
test/PCH/pch-no-hdrstop.cpp
Index: lib/Parse/ParseAST.cpp
===
--- lib/Parse/ParseAST.cpp
+++ lib/Parse/ParseAST.cpp
@@ -141,26 +141,26 @@
CleanupParser(ParseOP.get());
S.getPreprocessor().EnterMainSourceFile();
- if (!S.getPreprocessor().getCurrentLexer()) {
-// If a PCH through header is specified that does not have an include in
-// the source, there won't be any tokens or a Lexer.
-return;
- }
-
- P.Initialize();
-
- Parser::DeclGroupPtrTy ADecl;
ExternalASTSource *External = S.getASTContext().getExternalSource();
if (External)
External->StartTranslationUnit(Consumer);
- for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
- AtEOF = P.ParseTopLevelDecl(ADecl)) {
-// If we got a null return and something *was* parsed, ignore it. This
-// is due to a top-level semicolon, an action override, or a parse error
-// skipping something.
-if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
- return;
+ // If a PCH through header is specified that does not have an include in
+ // the source, or a PCH is being created with #pragma hdrstop with nothing
+ // after the pragma, there won't be any tokens or a Lexer.
+ bool HaveLexer = S.getPreprocessor().getCurrentLexer();
+
+ if (HaveLexer) {
+P.Initialize();
+Parser::DeclGroupPtrTy ADecl;
+for (bool AtEOF = P.ParseFirstTopLevelDecl(ADecl); !AtEOF;
+ AtEOF = P.ParseTopLevelDecl(ADecl)) {
+ // If we got a null return and something *was* parsed, ignore it. This
+ // is due to a top-level semicolon, an action override, or a parse error
+ // skipping something.
+ if (ADecl && !Consumer->HandleTopLevelDecl(ADecl.get()))
+return;
+}
}
// Process any TopLevelDecls generated by #pragma weak.
@@ -179,7 +179,7 @@
std::swap(OldCollectStats, S.CollectStats);
if (PrintStats) {
llvm::errs() << "\nSTATISTICS:\n";
-P.getActions().PrintStats();
+if (HaveLexer) P.getActions().PrintStats();
S.getASTContext().PrintStats();
Decl::PrintStats();
Stmt::PrintStats();
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2982,22 +2982,9 @@
}
}
- // Diagnose unsupported forms of /Yc /Yu. Ignore /Yc/Yu for now if:
- // * no filename after it
- // * both /Yc and /Yu passed but with different filenames
- // * corresponding file not also passed as /FI
+ // Ignore /Yc/Yu if both /Yc and /Yu passed but with different filenames.
Arg *YcArg = Args.getLastArg(options::OPT__SLASH_Yc);
Arg *YuArg = Args.getLastArg(options::OPT__SLASH_Yu);
- if (YcArg && YcArg->getValue()[0] == '\0') {
-Diag(clang::diag::warn_drv_ycyu_no_arg_clang_cl) << YcArg->getSpelling();
-Args.eraseArg(options::OPT__SLASH_Yc);
-YcArg = nullptr;
- }
- if (YuArg && YuArg->getValue()[0] == '\0') {
-Diag(clang::diag::warn_drv_ycyu_no_arg_clang_cl) << YuArg->getSpelling();
-Args.eraseArg(options::OPT__SLASH_Yu);
-YuArg = nullptr;
- }
if (YcArg && YuArg && strcmp(YcArg->getValue(), YuArg->getValue()) != 0) {
Diag(clang::diag::warn_drv_ycyu_different_arg_clang_cl);
Args.eraseArg(options::OPT__SLASH_Yc);
@@ -4279,11 +4266,11 @@
// extension of .pch is assumed. "
if (!llvm::sys::path::has_extension(Output))
Output += ".pch";
- } else if (Arg *YcArg = C.getArgs().getLastArg(options::OPT__SLASH_Yc)) {
-Output = YcArg->getValue();
-llvm::sys::path::replace_extension(Output, ".pch");
} else {
-Output = BaseName;
+if (Arg *YcArg = C.getArgs().getLastArg(options::OPT__SLASH_Yc))
+ Output = YcArg->getValue();
+if (Output.empty())
+ Output = BaseName;
llvm::sys::path::replace_extension(Output, ".pch");
}
return Output.str();
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1105,10 +1105,19 @@
StringRef ThroughHeader = YcArg ? YcArg->getValue() :
[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.
dblaikie added a comment. In https://reviews.llvm.org/D51554#1224049, @echristo wrote: > The change in name here from "line tables" to "directives only" feels a bit > confusing. "Limited" seems to be a bit more clear, or even remaining line > tables only. Can you explain where you were going with this particular set of > changes in a bit more detail please? Can't say I have much of an informed opinion about the parts that are only in the CUDA code. The "line directives only" terminology did come from a suggestion I made in one of the other reviews I can't seem to find right now.. ah, here: https://reviews.llvm.org/D51177 - whether or not that matches up with the use in the CUDA ToolChain code, I'm not sure. Repository: rC Clang https://reviews.llvm.org/D51554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.
echristo added a reviewer: dblaikie. echristo added a comment. The change in name here from "line tables" to "directives only" feels a bit confusing. "Limited" seems to be a bit more clear, or even remaining line tables only. Can you explain where you were going with this particular set of changes in a bit more detail please? Thanks! -eric Repository: rC Clang https://reviews.llvm.org/D51554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341421 - [ODRHash] Extend hash to support all Type's.
Author: rtrieu
Date: Tue Sep 4 15:53:19 2018
New Revision: 341421
URL: http://llvm.org/viewvc/llvm-project?rev=341421=rev
Log:
[ODRHash] Extend hash to support all Type's.
Added:
cfe/trunk/test/Modules/odr_hash-gnu.cpp
cfe/trunk/test/Modules/odr_hash-vector.cpp
cfe/trunk/test/Modules/odr_hash.cl
Modified:
cfe/trunk/include/clang/AST/ODRHash.h
cfe/trunk/lib/AST/ODRHash.cpp
cfe/trunk/lib/AST/StmtProfile.cpp
cfe/trunk/test/Modules/odr_hash-blocks.cpp
cfe/trunk/test/Modules/odr_hash.cpp
cfe/trunk/test/Modules/odr_hash.mm
Modified: cfe/trunk/include/clang/AST/ODRHash.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ODRHash.h?rev=341421=341420=341421=diff
==
--- cfe/trunk/include/clang/AST/ODRHash.h (original)
+++ cfe/trunk/include/clang/AST/ODRHash.h Tue Sep 4 15:53:19 2018
@@ -83,7 +83,7 @@ public:
void AddIdentifierInfo(const IdentifierInfo *II);
void AddNestedNameSpecifier(const NestedNameSpecifier *NNS);
void AddTemplateName(TemplateName Name);
- void AddDeclarationName(DeclarationName Name);
+ void AddDeclarationName(DeclarationName Name, bool TreatAsDecl = false);
void AddTemplateArgument(TemplateArgument TA);
void AddTemplateParameterList(const TemplateParameterList *TPL);
Modified: cfe/trunk/lib/AST/ODRHash.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ODRHash.cpp?rev=341421=341420=341421=diff
==
--- cfe/trunk/lib/AST/ODRHash.cpp (original)
+++ cfe/trunk/lib/AST/ODRHash.cpp Tue Sep 4 15:53:19 2018
@@ -32,7 +32,10 @@ void ODRHash::AddIdentifierInfo(const Id
ID.AddString(II->getName());
}
-void ODRHash::AddDeclarationName(DeclarationName Name) {
+void ODRHash::AddDeclarationName(DeclarationName Name, bool TreatAsDecl) {
+ if (TreatAsDecl)
+AddBoolean(true);
+
// Index all DeclarationName and use index numbers to refer to them.
auto Result = DeclNameMap.insert(std::make_pair(Name, DeclNameMap.size()));
ID.AddInteger(Result.first->second);
@@ -88,6 +91,9 @@ void ODRHash::AddDeclarationName(Declara
}
}
}
+
+ if (TreatAsDecl)
+AddBoolean(false);
}
void ODRHash::AddNestedNameSpecifier(const NestedNameSpecifier *NNS) {
@@ -405,6 +411,7 @@ public:
void VisitFunctionTemplateDecl(const FunctionTemplateDecl *D) {
AddDecl(D->getTemplatedDecl());
+ID.AddInteger(D->getTemplatedDecl()->getODRHash());
Inherited::VisitFunctionTemplateDecl(D);
}
@@ -552,11 +559,27 @@ void ODRHash::AddFunctionDecl(const Func
!Function->isDefaulted() && !Function->isDeleted() &&
!Function->isLateTemplateParsed();
AddBoolean(HasBody);
- if (HasBody) {
-auto *Body = Function->getBody();
-AddBoolean(Body);
-if (Body)
- AddStmt(Body);
+ if (!HasBody) {
+return;
+ }
+
+ auto *Body = Function->getBody();
+ AddBoolean(Body);
+ if (Body)
+AddStmt(Body);
+
+ // Filter out sub-Decls which will not be processed in order to get an
+ // accurate count of Decl's.
+ llvm::SmallVector Decls;
+ for (Decl *SubDecl : Function->decls()) {
+if (isWhitelistedDecl(SubDecl, Function)) {
+ Decls.push_back(SubDecl);
+}
+ }
+
+ ID.AddInteger(Decls.size());
+ for (auto SubDecl : Decls) {
+AddSubDecl(SubDecl);
}
}
@@ -592,13 +615,24 @@ void ODRHash::AddDecl(const Decl *D) {
assert(D && "Expecting non-null pointer.");
D = D->getCanonicalDecl();
- if (const NamedDecl *ND = dyn_cast(D)) {
-AddDeclarationName(ND->getDeclName());
+ const NamedDecl *ND = dyn_cast(D);
+ AddBoolean(ND);
+ if (!ND) {
+ID.AddInteger(D->getKind());
return;
}
- ID.AddInteger(D->getKind());
- // TODO: Handle non-NamedDecl here.
+ AddDeclarationName(ND->getDeclName());
+
+ const auto *Specialization =
+dyn_cast(D);
+ AddBoolean(Specialization);
+ if (Specialization) {
+const TemplateArgumentList = Specialization->getTemplateArgs();
+ID.AddInteger(List.size());
+for (const TemplateArgument : List.asArray())
+ AddTemplateArgument(TA);
+ }
}
namespace {
@@ -700,11 +734,67 @@ public:
VisitArrayType(T);
}
+ void VisitAttributedType(const AttributedType *T) {
+ID.AddInteger(T->getAttrKind());
+AddQualType(T->getModifiedType());
+AddQualType(T->getEquivalentType());
+
+VisitType(T);
+ }
+
+ void VisitBlockPointerType(const BlockPointerType *T) {
+AddQualType(T->getPointeeType());
+VisitType(T);
+ }
+
void VisitBuiltinType(const BuiltinType *T) {
ID.AddInteger(T->getKind());
VisitType(T);
}
+ void VisitComplexType(const ComplexType *T) {
+AddQualType(T->getElementType());
+VisitType(T);
+ }
+
+ void VisitDecltypeType(const DecltypeType *T) {
+AddStmt(T->getUnderlyingExpr());
+AddQualType(T->getUnderlyingType());
+VisitType(T);
+ }
+
+
r341418 - Revert r341373, since it fails on some targets.
Author: timshen
Date: Tue Sep 4 15:20:11 2018
New Revision: 341418
URL: http://llvm.org/viewvc/llvm-project?rev=341418=rev
Log:
Revert r341373, since it fails on some targets.
Differential Revision: https://reviews.llvm.org/D51354
Removed:
cfe/trunk/test/Driver/print-multi-directory.c
Modified:
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=341418=341417=341418=diff
==
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Tue Sep 4 15:20:11 2018
@@ -149,7 +149,6 @@ private:
protected:
MultilibSet Multilibs;
- Multilib SelectedMultilib;
ToolChain(const Driver , const llvm::Triple ,
const llvm::opt::ArgList );
@@ -228,8 +227,6 @@ public:
const MultilibSet () const { return Multilibs; }
- const Multilib () const { return SelectedMultilib; }
-
const SanitizerArgs& getSanitizerArgs() const;
const XRayArgs& getXRayArgs() const;
Modified: cfe/trunk/lib/Driver/Driver.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=341418=341417=341418=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Tue Sep 4 15:20:11 2018
@@ -1661,13 +1661,14 @@ bool Driver::HandleImmediateArgs(const C
}
if (C.getArgs().hasArg(options::OPT_print_multi_directory)) {
-const Multilib = TC.getMultilib();
-if (Multilib.gccSuffix().empty())
- llvm::outs() << ".\n";
-else {
- StringRef Suffix(Multilib.gccSuffix());
- assert(Suffix.front() == '/');
- llvm::outs() << Suffix.substr(1) << "\n";
+for (const Multilib : TC.getMultilibs()) {
+ if (Multilib.gccSuffix().empty())
+llvm::outs() << ".\n";
+ else {
+StringRef Suffix(Multilib.gccSuffix());
+assert(Suffix.front() == '/');
+llvm::outs() << Suffix.substr(1) << "\n";
+ }
}
return false;
}
Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=341418=341417=341418=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Tue Sep 4 15:20:11 2018
@@ -210,7 +210,6 @@ Linux::Linux(const Driver , const llvm
: Generic_ELF(D, Triple, Args) {
GCCInstallation.init(Triple, Args);
Multilibs = GCCInstallation.getMultilibs();
- SelectedMultilib = GCCInstallation.getMultilib();
llvm::Triple::ArchType Arch = Triple.getArch();
std::string SysRoot = computeSysRoot();
@@ -300,14 +299,16 @@ Linux::Linux(const Driver , const llvm
if (GCCInstallation.isValid()) {
const llvm::Triple = GCCInstallation.getTriple();
const std::string = GCCInstallation.getParentLibPath();
+const Multilib = GCCInstallation.getMultilib();
+const MultilibSet = GCCInstallation.getMultilibs();
// Add toolchain / multilib specific file paths.
-addMultilibsFilePaths(D, Multilibs, SelectedMultilib,
+addMultilibsFilePaths(D, Multilibs, Multilib,
GCCInstallation.getInstallPath(), Paths);
// Sourcery CodeBench MIPS toolchain holds some libraries under
// a biarch-like suffix of the GCC installation.
-addPathIfExists(D, GCCInstallation.getInstallPath() +
SelectedMultilib.gccSuffix(),
+addPathIfExists(D, GCCInstallation.getInstallPath() + Multilib.gccSuffix(),
Paths);
// GCC cross compiling toolchains will install target libraries which ship
@@ -329,7 +330,7 @@ Linux::Linux(const Driver , const llvm
// Note that this matches the GCC behavior. See the below comment for where
// Clang diverges from GCC's behavior.
addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib/../" +
- OSLibDir + SelectedMultilib.osSuffix(),
+ OSLibDir + Multilib.osSuffix(),
Paths);
// If the GCC installation we found is inside of the sysroot, we want to
Removed: cfe/trunk/test/Driver/print-multi-directory.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/print-multi-directory.c?rev=341417=auto
==
--- cfe/trunk/test/Driver/print-multi-directory.c (original)
+++ cfe/trunk/test/Driver/print-multi-directory.c (removed)
@@ -1,28 +0,0 @@
-// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
-// RUN: -target i386-none-linux \
-// RUN: -print-multi-directory \
-// RUN: | FileCheck
[PATCH] D51354: Fix the -print-multi-directory flag to print the selected multilib.
timshen added a comment. > The test fails on my system like so: I also observed the same failure. Bots also fail: http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules/builds/19509/steps/check-all/logs/FAIL%3A%20Clang%3A%3Aprint-multi-directory.c I'm going to revert this patch. Repository: rC Clang https://reviews.llvm.org/D51354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r341373 - Fix the -print-multi-directory flag to print the selected multilib.
This is breaking buildbots:
http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules/builds/19509
Can you take a look? Thanks!
On Tue, 4 Sep 2018 at 08:36, Christian Bruel via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: chrib
> Date: Tue Sep 4 08:22:13 2018
> New Revision: 341373
>
> URL: http://llvm.org/viewvc/llvm-project?rev=341373=rev
> Log:
> Fix the -print-multi-directory flag to print the selected multilib.
>
> Summary: Fix -print-multi-directory to print the selected multilib
>
> Reviewers: jroelofs
>
> Reviewed By: jroelofs
>
> Subscribers: srhines, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D51354
>
> Added:
> cfe/trunk/test/Driver/print-multi-directory.c
> Modified:
> cfe/trunk/include/clang/Driver/ToolChain.h
> cfe/trunk/lib/Driver/Driver.cpp
> cfe/trunk/lib/Driver/ToolChains/Linux.cpp
>
> Modified: cfe/trunk/include/clang/Driver/ToolChain.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=341373=341372=341373=diff
>
> ==
> --- cfe/trunk/include/clang/Driver/ToolChain.h (original)
> +++ cfe/trunk/include/clang/Driver/ToolChain.h Tue Sep 4 08:22:13 2018
> @@ -149,6 +149,7 @@ private:
>
> protected:
>MultilibSet Multilibs;
> + Multilib SelectedMultilib;
>
>ToolChain(const Driver , const llvm::Triple ,
> const llvm::opt::ArgList );
> @@ -227,6 +228,8 @@ public:
>
>const MultilibSet () const { return Multilibs; }
>
> + const Multilib () const { return SelectedMultilib; }
> +
>const SanitizerArgs& getSanitizerArgs() const;
>
>const XRayArgs& getXRayArgs() const;
>
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=341373=341372=341373=diff
>
> ==
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Tue Sep 4 08:22:13 2018
> @@ -1661,14 +1661,13 @@ bool Driver::HandleImmediateArgs(const C
>}
>
>if (C.getArgs().hasArg(options::OPT_print_multi_directory)) {
> -for (const Multilib : TC.getMultilibs()) {
> - if (Multilib.gccSuffix().empty())
> -llvm::outs() << ".\n";
> - else {
> -StringRef Suffix(Multilib.gccSuffix());
> -assert(Suffix.front() == '/');
> -llvm::outs() << Suffix.substr(1) << "\n";
> - }
> +const Multilib = TC.getMultilib();
> +if (Multilib.gccSuffix().empty())
> + llvm::outs() << ".\n";
> +else {
> + StringRef Suffix(Multilib.gccSuffix());
> + assert(Suffix.front() == '/');
> + llvm::outs() << Suffix.substr(1) << "\n";
> }
> return false;
>}
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=341373=341372=341373=diff
>
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Tue Sep 4 08:22:13 2018
> @@ -210,6 +210,7 @@ Linux::Linux(const Driver , const llvm
> : Generic_ELF(D, Triple, Args) {
>GCCInstallation.init(Triple, Args);
>Multilibs = GCCInstallation.getMultilibs();
> + SelectedMultilib = GCCInstallation.getMultilib();
>llvm::Triple::ArchType Arch = Triple.getArch();
>std::string SysRoot = computeSysRoot();
>
> @@ -299,16 +300,14 @@ Linux::Linux(const Driver , const llvm
>if (GCCInstallation.isValid()) {
> const llvm::Triple = GCCInstallation.getTriple();
> const std::string = GCCInstallation.getParentLibPath();
> -const Multilib = GCCInstallation.getMultilib();
> -const MultilibSet = GCCInstallation.getMultilibs();
>
> // Add toolchain / multilib specific file paths.
> -addMultilibsFilePaths(D, Multilibs, Multilib,
> +addMultilibsFilePaths(D, Multilibs, SelectedMultilib,
>GCCInstallation.getInstallPath(), Paths);
>
> // Sourcery CodeBench MIPS toolchain holds some libraries under
> // a biarch-like suffix of the GCC installation.
> -addPathIfExists(D, GCCInstallation.getInstallPath() +
> Multilib.gccSuffix(),
> +addPathIfExists(D, GCCInstallation.getInstallPath() +
> SelectedMultilib.gccSuffix(),
> Paths);
>
> // GCC cross compiling toolchains will install target libraries which
> ship
> @@ -330,7 +329,7 @@ Linux::Linux(const Driver , const llvm
> // Note that this matches the GCC behavior. See the below comment for
> where
> // Clang diverges from GCC's behavior.
> addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib/../" +
> - OSLibDir + Multilib.osSuffix(),
> + OSLibDir + SelectedMultilib.osSuffix(),
> Paths);
>
>
[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)
Wawha marked an inline comment as done.
Wawha added inline comments.
Comment at: lib/Format/ContinuationIndenter.cpp:1307
+ (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr &&
+Current.Next->is(TT_LambdaLSquare)));
State.Stack.back().IsInsideObjCArrayLiteral =
klimek wrote:
> Wawha wrote:
> > klimek wrote:
> > > klimek wrote:
> > > > I think I misunderstood some of this the first time around (and thanks
> > > > for bearing with me :) - iiuc we want to break for Allman even when
> > > > there's only one nested block. I think I'll need to take another look
> > > > when I'm back from vacation, unless Daniel or somebody else finds time
> > > > before then (will be back in 1.5 weeks)
> > > So, HasMultipleNestedBlocks should only be true if there are multiple
> > > nested blocks.
> > > What tests break if you remove this change to HasMultipleNestedBlocks?
> > All cases with only one lambda in parameter are break. The Lambda body with
> > be put on the same line as the function and aligned with [] instead of
> > putting the body [] on another line with just one simple indentation.
> >
> > So if restore the line to :
> >
> > ```
> > State.Stack.back().HasMultipleNestedBlocks = Current.BlockParameterCount >
> > 1;
> > ```
> >
> > Here some cases.
> > FormatTest.cpp, ligne 11412:
> >
> > ```
> > // With my modification
> > FunctionWithOneParam(
> > []()
> > {
> > // A cool function...
> > return 43;
> > });
> >
> > // Without my modification
> > FunctionWithOneParam([]()
> > {
> >// A cool function...
> >return 43;
> > });
> > ```
> > The "{" block of the lambda will be aligned on the "[" depending of the
> > function name length.
> >
> >
> > You have the same case with multiple lambdas (FormatTest.cpp, ligne 11433):
> >
> > ```
> > // With my modification
> > TwoNestedLambdas(
> > []()
> > {
> > return Call(
> > []()
> > {
> > return 17;
> > });
> > });
> >
> > // Without my modification
> > TwoNestedLambdas([]()
> > {
> >return Call([]()
> >{
> > return 17;
> >});
> > });
> > ```
> Do we want to always break before lambdas in Allman style?
Perhaps not always. I make that current implementation, because, I have some
difficulty to make the other. I alreay tried to modified the code to have that :
```
TwoNestedLambdas([]()
{
return Call(
[]()
{
return 17;
});
});
```
With just a tabulation after the line return. But with no success for the
moment.
I could try again to implement it, and why not, add an option to tell if we
break or not before the lambda. And set it to false by default.
Repository:
rC Clang
https://reviews.llvm.org/D44609
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51657: [CMake] Link to compiler-rt if LIBUNWIND_USE_COMPILER_RT is ON.
cdavis5x created this revision.
cdavis5x added a reviewer: beanz.
Herald added subscribers: cfe-commits, chrib, christof, mgorny, dberris.
If `-nodefaultlibs` is given, we weren't actually linking to it. This
was true irrespective of passing `-rtlib=compiler-rt` (see previous
patch). Now we explicitly link it to handle that case.
I wonder if we should be linking these libraries only if we're using
`-nodefaultlibs`...
Repository:
rUNW libunwind
https://reviews.llvm.org/D51657
Files:
src/CMakeLists.txt
Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -53,7 +53,12 @@
# Generate library list.
set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
-append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
+if (LIBUNWIND_USE_COMPILER_RT)
+ list(APPEND libraries "${LIBUNWIND_BUILTINS_LIBRARY}")
+else()
+ append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
+ list(APPEND libraries gcc)
+endif()
append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
if (LIBUNWIND_ENABLE_THREADS)
append_if(libraries LIBUNWIND_HAS_PTHREAD_LIB pthread)
Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -53,7 +53,12 @@
# Generate library list.
set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
-append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
+if (LIBUNWIND_USE_COMPILER_RT)
+ list(APPEND libraries "${LIBUNWIND_BUILTINS_LIBRARY}")
+else()
+ append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
+ list(APPEND libraries gcc)
+endif()
append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
if (LIBUNWIND_ENABLE_THREADS)
append_if(libraries LIBUNWIND_HAS_PTHREAD_LIB pthread)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51645: [CMake] Don't use -rtlib=compiler-rt with -nodefaultlibs.
This revision was automatically updated to reflect the committed changes.
Closed by commit rUNW341404: [CMake] Dont use -rtlib=compiler-rt with
-nodefaultlibs. (authored by cdavis, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51645?vs=163864=163907#toc
Repository:
rUNW libunwind
https://reviews.llvm.org/D51645
Files:
CMakeLists.txt
cmake/config-ix.cmake
Index: cmake/config-ix.cmake
===
--- cmake/config-ix.cmake
+++ cmake/config-ix.cmake
@@ -23,7 +23,6 @@
list(APPEND CMAKE_REQUIRED_LIBRARIES c)
endif ()
if (LIBUNWIND_USE_COMPILER_RT)
-list(APPEND CMAKE_REQUIRED_FLAGS -rtlib=compiler-rt)
find_compiler_rt_library(builtins LIBUNWIND_BUILTINS_LIBRARY)
list(APPEND CMAKE_REQUIRED_LIBRARIES "${LIBUNWIND_BUILTINS_LIBRARY}")
elseif (LIBUNWIND_HAS_GCC_S_LIB)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -234,7 +234,7 @@
# Configure compiler.
include(config-ix)
-if (LIBUNWIND_USE_COMPILER_RT)
+if (LIBUNWIND_USE_COMPILER_RT AND NOT LIBUNWIND_HAS_NODEFAULTLIBS_FLAG)
list(APPEND LIBUNWIND_LINK_FLAGS "-rtlib=compiler-rt")
endif()
Index: cmake/config-ix.cmake
===
--- cmake/config-ix.cmake
+++ cmake/config-ix.cmake
@@ -23,7 +23,6 @@
list(APPEND CMAKE_REQUIRED_LIBRARIES c)
endif ()
if (LIBUNWIND_USE_COMPILER_RT)
-list(APPEND CMAKE_REQUIRED_FLAGS -rtlib=compiler-rt)
find_compiler_rt_library(builtins LIBUNWIND_BUILTINS_LIBRARY)
list(APPEND CMAKE_REQUIRED_LIBRARIES "${LIBUNWIND_BUILTINS_LIBRARY}")
elseif (LIBUNWIND_HAS_GCC_S_LIB)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -234,7 +234,7 @@
# Configure compiler.
include(config-ix)
-if (LIBUNWIND_USE_COMPILER_RT)
+if (LIBUNWIND_USE_COMPILER_RT AND NOT LIBUNWIND_HAS_NODEFAULTLIBS_FLAG)
list(APPEND LIBUNWIND_LINK_FLAGS "-rtlib=compiler-rt")
endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r341404 - [CMake] Don't use -rtlib=compiler-rt with -nodefaultlibs.
Author: cdavis
Date: Tue Sep 4 13:57:50 2018
New Revision: 341404
URL: http://llvm.org/viewvc/llvm-project?rev=341404=rev
Log:
[CMake] Don't use -rtlib=compiler-rt with -nodefaultlibs.
Summary:
This switch only has an effect at link time. It changes the default
compiler support library to `compiler-rt`. With `-nodefaultlibs`, this
library won't get linked anyway; Clang actually warns about that.
Reviewers: mstorsjo, rnk
Subscribers: dberris, mgorny, christof, cfe-commits
Differential Revision: https://reviews.llvm.org/D51645
Modified:
libunwind/trunk/CMakeLists.txt
libunwind/trunk/cmake/config-ix.cmake
Modified: libunwind/trunk/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=341404=341403=341404=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Tue Sep 4 13:57:50 2018
@@ -234,7 +234,7 @@ endif()
# Configure compiler.
include(config-ix)
-if (LIBUNWIND_USE_COMPILER_RT)
+if (LIBUNWIND_USE_COMPILER_RT AND NOT LIBUNWIND_HAS_NODEFAULTLIBS_FLAG)
list(APPEND LIBUNWIND_LINK_FLAGS "-rtlib=compiler-rt")
endif()
Modified: libunwind/trunk/cmake/config-ix.cmake
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/cmake/config-ix.cmake?rev=341404=341403=341404=diff
==
--- libunwind/trunk/cmake/config-ix.cmake (original)
+++ libunwind/trunk/cmake/config-ix.cmake Tue Sep 4 13:57:50 2018
@@ -23,7 +23,6 @@ if (LIBUNWIND_HAS_NODEFAULTLIBS_FLAG)
list(APPEND CMAKE_REQUIRED_LIBRARIES c)
endif ()
if (LIBUNWIND_USE_COMPILER_RT)
-list(APPEND CMAKE_REQUIRED_FLAGS -rtlib=compiler-rt)
find_compiler_rt_library(builtins LIBUNWIND_BUILTINS_LIBRARY)
list(APPEND CMAKE_REQUIRED_LIBRARIES "${LIBUNWIND_BUILTINS_LIBRARY}")
elseif (LIBUNWIND_HAS_GCC_S_LIB)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51645: [CMake] Don't use -rtlib=compiler-rt with -nodefaultlibs.
beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. LGTM. Repository: rUNW libunwind https://reviews.llvm.org/D51645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51507: Allow all supportable attributes to be used with #pragma clang attribute.
arphaman accepted this revision. arphaman added a comment. LGTM, Thank you! Repository: rC Clang https://reviews.llvm.org/D51507 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good to me & the answers to others various questions seem well addressed. Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51507: Allow all supportable attributes to be used with #pragma clang attribute.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thank you for pointing out that the new form of type attributes aren't automatically opted in from this change. With that (and the two problematic attributes opted out), this LGTM. Repository: rC Clang https://reviews.llvm.org/D51507 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51200: Introduce per-callsite inline intrinsics
kuhar added a comment. Thank you for the comments, Richard! In https://reviews.llvm.org/D51200#1223745, @rsmith wrote: > Have you considered whether the builtin should apply to `new` expressions? > (There are potentially three different top-level calls implied by a `new` > expression -- an `operator new`, a constructor, and an `operator delete` -- > so it's not completely obvious what effect this would have there.) And > likewise for `delete`. I haven't thought about it, but to be honest I'm not sure what should happen. Intuitively, I think these 3 options would make most sense, listed from the most to the least reasonable in my opionion: 1. apply the corresponding attribute to all of the implied calls 2. apply to constructor/destructor only 1. don't handle it at all -- emit an error How would you suggest to handle it? https://reviews.llvm.org/D51200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
aprantl added a comment. In https://reviews.llvm.org/D51576#1223562, @labath wrote: > The interactions here are a bit weird, but the short answer is no, this will > not affect apple tables in any way. Then I have no problem with this patch :-) Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51200: Introduce per-callsite inline intrinsics
kuhar added a comment. In https://reviews.llvm.org/D51200#1223752, @rsmith wrote: > +rjmccall for CodeGen changes @rsmith @rjmccall I have one high-level question about the CodeGen part that I wasn't able to figure out: is it possible to bail out of custom CodeGen in CGBuiltin and somehow say: emit whatever is inside (IE treat the builtin call node as a no-op)? https://reviews.llvm.org/D51200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51655: [analyzer] Remove traces of ubigraph visualization
george.karpenkov created this revision.
george.karpenkov added reviewers: dcoughlin, NoQ.
Herald added subscribers: Szelethus, mikhail.ramalho, a.sidorin, szepet,
baloghadamsoftware, xazax.hun, whisperity.
Ubigraph project has been dead since about 2008, and to the best of my
knowledge, no one was using it.
Previously, I wasn't able to launch the existing binary at all.
https://reviews.llvm.org/D51655
Files:
clang/include/clang/Driver/CC1Options.td
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/test/Analysis/ubigraph-viz.cpp
clang/tools/scan-build-py/libscanbuild/analyze.py
clang/tools/scan-build/libexec/ccc-analyzer
clang/utils/analyzer/ubiviz
Index: clang/utils/analyzer/ubiviz
===
--- clang/utils/analyzer/ubiviz
+++ /dev/null
@@ -1,76 +0,0 @@
-#!/usr/bin/env python
-#
-# The LLVM Compiler Infrastructure
-#
-# This file is distributed under the University of Illinois Open Source
-# License. See LICENSE.TXT for details.
-#
-##======##
-#
-# This script reads visualization data emitted by the static analyzer for
-# display in Ubigraph.
-#
-##======##
-
-import xmlrpclib
-import sys
-
-
-def Error(message):
-print >> sys.stderr, 'ubiviz: ' + message
-sys.exit(1)
-
-
-def StreamData(filename):
-file = open(filename)
-for ln in file:
-yield eval(ln)
-file.close()
-
-
-def Display(G, data):
-action = data[0]
-if action == 'vertex':
-vertex = data[1]
-G.new_vertex_w_id(vertex)
-for attribute in data[2:]:
-G.set_vertex_attribute(vertex, attribute[0], attribute[1])
-elif action == 'edge':
-src = data[1]
-dst = data[2]
-edge = G.new_edge(src, dst)
-for attribute in data[3:]:
-G.set_edge_attribute(edge, attribute[0], attribute[1])
-elif action == "vertex_style":
-style_id = data[1]
-parent_id = data[2]
-G.new_vertex_style_w_id(style_id, parent_id)
-for attribute in data[3:]:
-G.set_vertex_style_attribute(style_id, attribute[0], attribute[1])
-elif action == "vertex_style_attribute":
-style_id = data[1]
-for attribute in data[2:]:
-G.set_vertex_style_attribute(style_id, attribute[0], attribute[1])
-elif action == "change_vertex_style":
-vertex_id = data[1]
-style_id = data[2]
-G.change_vertex_style(vertex_id, style_id)
-
-
-def main(args):
-if len(args) == 0:
-Error('no input files')
-
-server = xmlrpclib.Server('http://127.0.0.1:20738/RPC2')
-G = server.ubigraph
-
-for arg in args:
-G.clear()
-for x in StreamData(arg):
-Display(G, x)
-
-sys.exit(0)
-
-
-if __name__ == '__main__':
-main(sys.argv[1:])
Index: clang/tools/scan-build/libexec/ccc-analyzer
===
--- clang/tools/scan-build/libexec/ccc-analyzer
+++ clang/tools/scan-build/libexec/ccc-analyzer
@@ -243,11 +243,6 @@
push @Args, "-Xclang", $arg;
}
-# Display Ubiviz graph?
-if (defined $ENV{'CCC_UBI'}) {
- push @Args, "-Xclang", "-analyzer-viz-egraph-ubigraph";
-}
-
if (defined $AnalyzerTarget) {
push @Args, "-target", $AnalyzerTarget;
}
Index: clang/tools/scan-build-py/libscanbuild/analyze.py
===
--- clang/tools/scan-build-py/libscanbuild/analyze.py
+++ clang/tools/scan-build-py/libscanbuild/analyze.py
@@ -395,8 +395,6 @@
if args.disable_checker:
checkers = ','.join(args.disable_checker)
result.extend(['-analyzer-disable-checker', checkers])
-if os.getenv('UBIVIZ'):
-result.append('-analyzer-viz-egraph-ubigraph')
return prefix_with('-Xclang', result)
Index: clang/test/Analysis/ubigraph-viz.cpp
===
--- clang/test/Analysis/ubigraph-viz.cpp
+++ /dev/null
@@ -1,7 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API -analyzer-viz-egraph-ubigraph -verify %s
-// expected-no-diagnostics
-
-int f(int x) {
- return x < 0 ? 0 : 42;
-}
-
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -50,8 +50,6 @@
#define DEBUG_TYPE "AnalysisConsumer"
-static
[PATCH] D51200: Introduce per-callsite inline intrinsics
rsmith added a comment. +rjmccall for CodeGen changes https://reviews.llvm.org/D51200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51200: Introduce per-callsite inline intrinsics
rsmith added a comment.
> Support for constructor calls (`CXXTemporaryExpr`) should also be possible,
> but is not the part of this patch.
(You should handle the base class (`CXXConstructExpr`) that describes the
semantics, rather than the derived class (`CXXTemporaryObjectExpr`) that
describes a particular syntactic form for said semantics.)
Have you considered whether the builtin should apply to `new` expressions?
(There are potentially three different top-level calls implied by a `new`
expression -- an `operator new`, a constructor, and an `operator delete` -- so
it's not completely obvious what effect this would have there.) And likewise
for `delete`.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8236
+def err_argument_to_inline_intrinsic_builtin_call : Error<
+ "argument to %0 must not be a builtin call">;
+def err_argument_to_inline_intrinsic_pdotr_call : Error<
kuhar wrote:
> Quuxplusone wrote:
> > I'd expect to be able to write
> >
> > __builtin_always_inline(sqrt(x))
> > __builtin_no_inline(sqrt(x))
> >
> > without caring whether `sqrt` was a real function or just a macro around
> > `__builtin_sqrt`. How important is it that calls to builtin functions be
> > errors, instead of just being ignored for this purpose?
> Very good point. Initially I thought this should be an error, but I think
> that since compiler can reason about intrinsics anyway, it makes sense to
> treat inline intrinsics as NoOps in this case.
I think it would be surprising if `__builtin_no_inline(sqrt(x))` would use a
target-specific `sqrt` instruction rather than making a library call. But given
that these builtins are best-effort anyway, maybe it's acceptable.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8238
+def err_argument_to_inline_intrinsic_pdtor_call : Error<
+ "argument to %0 must not be a pseudo-destructor call">;
+def err_argument_to_inline_intrinsic_block_call : Error<
Nit: we generally use "cannot" rather than "must not" in diagnostics.
Comment at: lib/CodeGen/CGBuiltin.cpp:3008-3013
+if (const CXXOperatorCallExpr *OperatorCall =
+dyn_cast(Arg))
+ if (const CXXMethodDecl *MD =
+ dyn_cast_or_null(OperatorCall->getCalleeDecl()))
+return EmitCXXOperatorMemberCallExpr(OperatorCall, MD, ReturnValue,
+ CIK);
Nit: please add braces around the outer `if`.
Comment at: lib/Sema/SemaChecking.cpp:338-345
+ const auto CallStmtClass = Call->getStmtClass();
+ if (CallStmtClass != Stmt::CallExprClass &&
+ CallStmtClass != Stmt::CXXMemberCallExprClass &&
+ CallStmtClass != Stmt::CXXOperatorCallExprClass) {
+S.Diag(BuiltinLoc, diag::err_argument_to_inline_intrinsic_not_call)
+<< Builtin << Call->getSourceRange();
+return true;
Use `isa` rather than checking the exact class here; you should
accept statements derived from these kinds too (eg, `UserDefinedLiteralExpr`).
(You should explicitly disallow `CUDAKernelCallExpr`, though, since the
builtins don't make any sense for a host -> device call.)
Comment at: lib/Sema/SemaChecking.cpp:349
+ const Decl *TargetDecl = CE->getCalleeDecl();
+ if (const auto *FD = dyn_cast_or_null(TargetDecl))
+if (FD->getBuiltinID()) {
Nit: braces here.
Comment at: lib/Sema/SemaChecking.cpp:356-360
+ if (CE->getCallee()->getType()->isBlockPointerType()) {
+S.Diag(BuiltinLoc, diag::err_argument_to_inline_intrinsic_block_call)
+<< Builtin << Call->getSourceRange();
+return true;
+ }
Is this a necessary restriction? I would expect calls to blocks to be
inlineable when the callee can be statically determined.
https://reviews.llvm.org/D51200
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions ď“ś
Wizard added inline comments.
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:50
+
+void FunctionNamingCheck::registerMatchers(MatchFinder *Finder) {
+ // This check should only be applied to Objective-C sources.
Can we do some simple check to see if some easy fix can be provided just like
`objc-property-declaration` check?
Something like `static bool isPositive` to `static bool IsPositive` and `static
bool is_upper_camel` to `IsUpperCamel`. Such check can help provide code fix
for a lot of very common mistake at a low cost (i.e. if the naming pattern
cannot be simply recognized, just provide no fix).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51575
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator
xbolva00 added inline comments.
Comment at: llvm/include/llvm/Support/Allocator.h:295
+int64_t TotalSlabOffset = 0;
+for (size_t Idx = 0; Idx < Slabs.size(); Idx++) {
+ const char *S = reinterpret_cast(Slabs[Idx]);
for (size_t Idx = 0, e = Slabs.size(); Idx < e; Idx++) {
https://reviews.llvm.org/D51393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator
NoQ added inline comments.
Comment at: llvm/include/llvm/Support/Allocator.h:304
+// Use negative index to denote custom sized slabs.
+int64_t CustomSlabOffset = 0;
+for (size_t Idx = 0; Idx < CustomSizedSlabs.size(); Idx++) {
We should start with -1 because otherwise the first standard-slab object and
the first custom-slab object would both have id 0.
https://reviews.llvm.org/D51393
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions ď“ś
hokein added inline comments. Comment at: clang-tidy/google/FunctionNamingCheck.cpp:57 + functionDecl( + isDefinition(), + unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)), stephanemoore wrote: > hokein wrote: > > any reason why we restrict to definitions only? I think we can consider > > declarations too. > I restricted the check to function definitions because function declarations > are sometimes associated with functions outside the control of the author. I > have personally observed unfortunate cases where functions declared in the > iOS SDK had incorrect—or seemingly incorrect—availability attributes that > caused fatal dyld assertions during application startup. The check currently > intends to avoid flagging function declarations because of the rare > circumstances where an inflexible function declaration without a > corresponding function definition is required. > > I have added a comment explaining why only function definitions are flagged. > > I am still open to discussion though. Thanks for the explanations. I have a concern about the heuristic using here, it seems fragile -- if there is an inline function defined in a base header, the check will still give a warning to it if the source file `.m` #includes this header; it also limits the scope of the check, I think this check is flagged mostly on file-local functions (e.g. static functions, functions defined in anonymous namespace). Flagging on function declaration doesn't seem too problematic (we already have a similar check `objc-property-declaration` does the same thing) -- our internal review system shows check warnings on changed code, if user's code includes a common header which violates the check, warnings on the header will not be shown; for violations in iOS SDK, we can use some filtering matchers (`isExpansionInSystemHeader` maybe work) to ignore all functions from these files. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51575 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51645: [CMake] Don't use -rtlib=compiler-rt with -nodefaultlibs.
mstorsjo added a subscriber: beanz. mstorsjo added a comment. Sounds sensible to me, although it might be good with a second opinion I think. @beanz? Repository: rUNW libunwind https://reviews.llvm.org/D51645 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50958: [clangd] Implement findReferences function
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Thanks, looks good from my side.
Comment at: clangd/XRefs.cpp:328
+ : AST(AST) {
+for (const Decl *D : TargetDecls)
+ Targets.insert(D);
nit: we can initialize TargetDecls like `Targets(TargetDecls.begin(),
TargetDecls.end())`.
Comment at: clangd/XRefs.cpp:676
+ auto MainFileRefs = findRefs(Symbols.Decls, AST);
+ for (auto : MainFileRefs) {
+Location Result;
nit: use `const auto &` indicating the variable is immutable.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50958
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51567: CMake: Consolidate gtest detection code
tstellar added a comment. In https://reviews.llvm.org/D51567#1222704, @chandlerc wrote: > I mean, sure. > > I really don't know that supporting this ever expanding diversity of build > strategies is worth its cost, but I don't see a specific reason to not take > this patch I actually agree with you that we don't want to be supporting every possible strategy in trunk. I usually try to keep Fedora specific stuff like this out of trunk, but in this case it looked like a useful code simplification that might be generally helpful. Repository: rC Clang https://reviews.llvm.org/D51567 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51650: Implement target_clones multiversioning
erichkeane created this revision.
erichkeane added reviewers: thiagomacieira, aaron.ballman.
As discussed here: https://lwn.net/Articles/691932/
GCC6.0 adds target_clones multiversioning. This functionality is
an odd cross between the cpu_dispatch and 'target' MV, but is compatible
with neither.
This attribute allows you to list all options, then emits a separately
optimized version of each function per-option (similar to the
cpu_specific attribute). It automatically generates a resolver, just
like the other two.
The mangling however, is... ODD to say the least. The mangling format is:
...
However, the 'option ordinal' is where it gets strange. When parsing the
list, 'default' is moved to the end, so "foo,default,bar", foo is 0, bar is 1,
and default is 2.
Otherwise, emission rules should be the same as 'target'.
https://reviews.llvm.org/D51650
Files:
include/clang/AST/Decl.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/AST/Decl.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
test/CodeGen/attr-cpuspecific.c
test/CodeGen/attr-target-clones.c
test/Misc/pragma-attribute-supported-attributes-list.test
test/Sema/attr-target-clones.c
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -917,6 +917,19 @@
}
}
+static void AppendTargetClonesMangling(const CodeGenModule ,
+ const TargetClonesAttr *Attr,
+ raw_ostream ) {
+ Out << '.';
+ StringRef FeatureStr = Attr->getCurFeatureStr();
+ if (FeatureStr.startswith("arch="))
+Out << "arch_" << FeatureStr.substr(sizeof("arch=") - 1);
+ else
+Out << FeatureStr;
+
+ Out << '.' << Attr->ActiveArgIndex;
+}
+
static std::string getMangledNameImpl(const CodeGenModule , GlobalDecl GD,
const NamedDecl *ND,
bool OmitMultiVersionMangling = false) {
@@ -950,6 +963,8 @@
if (FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion())
AppendCPUSpecificCPUDispatchMangling(
CGM, FD->getAttr(), Out);
+ else if (FD->isTargetClonesMultiVersion())
+AppendTargetClonesMangling(CGM, FD->getAttr(), Out);
else
AppendTargetMangling(CGM, FD->getAttr(), Out);
}
@@ -1013,12 +1028,19 @@
// Since CPUSpecific can require multiple emits per decl, store the manglings
// separately.
if (FD &&
- (FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion())) {
+ (FD->isCPUDispatchMultiVersion() || FD->isCPUSpecificMultiVersion() ||
+ FD->isTargetClonesMultiVersion())) {
const auto *SD = FD->getAttr();
+const auto *TC = FD->getAttr();
-std::pair SpecCanonicalGD{
-CanonicalGD,
-SD ? SD->ActiveArgIndex : std::numeric_limits::max()};
+unsigned VersionID = std::numeric_limits::max();
+
+if (SD)
+ VersionID = SD->ActiveArgIndex;
+else if (TC)
+ VersionID = TC->ActiveArgIndex;
+
+std::pair SpecCanonicalGD{CanonicalGD, VersionID};
auto FoundName = CPUSpecificMangledDeclNames.find(SpecCanonicalGD);
if (FoundName != CPUSpecificMangledDeclNames.end())
@@ -1376,9 +1398,10 @@
const auto *FD = dyn_cast_or_null(D);
FD = FD ? FD->getMostRecentDecl() : FD;
const auto *TD = FD ? FD->getAttr() : nullptr;
- const auto *SD = FD ? FD->getAttr() : nullptr;
bool AddedAttr = false;
- if (TD || SD) {
+
+ if (FD && (TD || FD->hasAttr() ||
+ FD->hasAttr())) {
llvm::StringMap FeatureMap;
getFunctionFeatureMap(FeatureMap, FD);
@@ -2111,6 +2134,9 @@
if (Global->hasAttr())
return emitCPUDispatchDefinition(GD);
+ if (Global->hasAttr() || Global->hasAttr())
+return EmitGlobalFunctionDefinition(GD, nullptr);
+
// If this is CUDA, be selective about which declarations we emit.
if (LangOpts.CUDA) {
if (LangOpts.CUDAIsDevice) {
@@ -2365,6 +2391,7 @@
if (getFunctionLinkage(GD) != llvm::Function::AvailableExternallyLinkage)
return true;
const auto *F = cast(GD.getDecl());
+
if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr())
return false;
@@ -2526,6 +2553,51 @@
CGF.EmitCPUDispatchMultiVersionResolver(ResolverFunc, Options);
}
+void CodeGenModule::EmitTargetClonesResolver(GlobalDecl GD) {
+ const auto *FD = cast(GD.getDecl());
+ assert(FD && "Not a FunctionDecl?");
+ auto *ClonesAttr = FD->getAttr();
+ assert(ClonesAttr && "Not a target_clones Function?");
+ llvm::Type *DeclTy = getTypes().ConvertTypeForMem(FD->getType());
+
+ // Force emission of the IFunc.
+ GetOrCreateMultiVersionIFunc(GD, DeclTy, FD);
+
+ StringRef MangledName
[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
labath added a comment. In https://reviews.llvm.org/D51576#1223596, @clayborg wrote: > We want to ensure that both Apple and DWARF5 tables never get generated > though. That would waste a lot of space. I would say if DWARF5 tables are > enabled, then we need ensure we disable Apple tables. That's already taken care of. The back end will always generate at most one kind of accel tables. Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
clayborg added a comment. Actually, we might need to still emit some Apple tables as the objective C and namespace tables might still be needed even with the DWARF5 tables. Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
clayborg added a comment. We want to ensure that both Apple and DWARF5 tables never get generated though. That would waste a lot of space. I would say if DWARF5 tables are enabled, then we need ensure we disable Apple tables. Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51416: [RTTI] Align rtti types to prevent over-alignment
efriedma added a comment. getClassPointerAlignment is not really relevant here; that's the alignment of a pointer to an object with the type of the class. For the LLVM IR structs which don't have a corresponding clang type, you can probably just use DataLayout::getABITypeAlignment(). I think that's just the alignment of a pointer in practice, but the intent is more clear. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:2027 "vbtable with this name already exists: mangling bug?"); + unsigned Align = CGM.getClassPointerAlignment(RD).getQuantity(); llvm::GlobalVariable *GV = This is an array of `int`. https://reviews.llvm.org/D51416 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51649: [Sema] Don't warn about omitting unavailable enum constants in a switch
erik.pilkington created this revision.
erik.pilkington added a reviewer: arphaman.
Herald added a subscriber: dexonsmith.
rdar://42717026
Thanks for taking a look!
Repository:
rC Clang
https://reviews.llvm.org/D51649
Files:
clang/lib/Sema/SemaStmt.cpp
clang/test/Sema/switch-availability.c
Index: clang/test/Sema/switch-availability.c
===
--- /dev/null
+++ clang/test/Sema/switch-availability.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -verify -Wswitch -triple x86_64-apple-macosx10.12 %s
+
+enum SwitchOne {
+ Unavail __attribute__((availability(macos, unavailable))),
+};
+
+void testSwitchOne(enum SwitchOne so) {
+ switch (so) {} // no warning
+}
+
+enum SwitchTwo {
+ Ed __attribute__((availability(macos, deprecated=10.12))),
+ Vim __attribute__((availability(macos, deprecated=10.13))),
+ Emacs,
+};
+
+void testSwitchTwo(enum SwitchTwo st) {
+ switch (st) {} // expected-warning{{enumeration values 'Vim' and 'Emacs' not
handled in switch}}
+}
+
+enum SwitchThree {
+ New __attribute__((availability(macos, introduced=1000))),
+};
+
+void testSwitchThree(enum SwitchThree st) {
+ switch (st) {} // expected-warning{{enumeration value 'New' not handled in
switch}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -1164,7 +1164,21 @@
SmallVector UnhandledNames;
- for (EI = EnumVals.begin(); EI != EIEnd; EI++){
+ for (EI = EnumVals.begin(); EI != EIEnd; EI++) {
+// Don't warn about omitted unavailable EnumConstantDecls.
+switch (EI->second->getAvailability()) {
+case AR_Deprecated:
+ // Omitting a deprecated constant is ok; it should never materialize.
+case AR_Unavailable:
+ continue;
+
+case AR_NotYetIntroduced:
+ // Partially available enum constants should be present. Note that we
+ // suppress -Wunguarded-availability diagnostics for such uses.
+case AR_Available:
+ break;
+}
+
// Drop unneeded case values
while (CI != CaseVals.end() && CI->first < EI->first)
CI++;
Index: clang/test/Sema/switch-availability.c
===
--- /dev/null
+++ clang/test/Sema/switch-availability.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -verify -Wswitch -triple x86_64-apple-macosx10.12 %s
+
+enum SwitchOne {
+ Unavail __attribute__((availability(macos, unavailable))),
+};
+
+void testSwitchOne(enum SwitchOne so) {
+ switch (so) {} // no warning
+}
+
+enum SwitchTwo {
+ Ed __attribute__((availability(macos, deprecated=10.12))),
+ Vim __attribute__((availability(macos, deprecated=10.13))),
+ Emacs,
+};
+
+void testSwitchTwo(enum SwitchTwo st) {
+ switch (st) {} // expected-warning{{enumeration values 'Vim' and 'Emacs' not handled in switch}}
+}
+
+enum SwitchThree {
+ New __attribute__((availability(macos, introduced=1000))),
+};
+
+void testSwitchThree(enum SwitchThree st) {
+ switch (st) {} // expected-warning{{enumeration value 'New' not handled in switch}}
+}
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -1164,7 +1164,21 @@
SmallVector UnhandledNames;
- for (EI = EnumVals.begin(); EI != EIEnd; EI++){
+ for (EI = EnumVals.begin(); EI != EIEnd; EI++) {
+// Don't warn about omitted unavailable EnumConstantDecls.
+switch (EI->second->getAvailability()) {
+case AR_Deprecated:
+ // Omitting a deprecated constant is ok; it should never materialize.
+case AR_Unavailable:
+ continue;
+
+case AR_NotYetIntroduced:
+ // Partially available enum constants should be present. Note that we
+ // suppress -Wunguarded-availability diagnostics for such uses.
+case AR_Available:
+ break;
+}
+
// Drop unneeded case values
while (CI != CaseVals.end() && CI->first < EI->first)
CI++;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible
beanz added a subscriber: dexonsmith. beanz added a comment. Unfortunately I can't make this kind of policy decision about whether or not this would be acceptable for Darwin. That would probably need to be @dexonsmith. Repository: rC Clang https://reviews.llvm.org/D51440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
labath added a comment. In https://reviews.llvm.org/D51576#1223234, @aprantl wrote: > This is DWARF5+ only, right? (We shouldn't change the preference of Apple > accelerator tables for DWARF 4 and earlier). The interactions here are a bit weird, but the short answer is no, this will not affect apple tables in any way. The long answer is: The type of accelerator tables that get generated gets chosen in the back end, based on target triple and dwarf version. If apple tables get chosen, then this patch makes no difference because apple tables ignore the -gpubnames argument. If dwarf tables get chosen, then they will only index the CUs which had the "pubnames" flag set to on (!None). This patch makes it such that we automatically set that flag when tuning for lldb, thereby obtaining the same default behavior as apple tables (indexing all CUs when tuning for lldb). Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements
mgrang added a comment.
In https://reviews.llvm.org/D50488#1207398, @Szelethus wrote:
> In https://reviews.llvm.org/D50488#1204655, @mgrang wrote:
>
> > This is my first time with ASTMatchers and I am not sure how to get the
> > value_type from hasType (I don't see a matcher for value_types in
> > ASTMatchers.h). Would I end up using a visitor for that? If yes, then maybe
> > the entire check for pointer types needs to be done via visitors? Sorry for
> > the newbie questions :)
>
>
> I played around for a bit, my `clang-query` is a little out of date, but
> here's what I found: You can match `typedef`s with `typedefDecl()`, and
> `using` by `typeAliasDecl()`, and then add `hasName("value_type")` as a
> parameter. As to how to check whether a type has any of these, I'm a little
> unsure myself, but you could use `hasDescendant`, and narrow down the matches.
>
> I'm not sure whether this checks inherited type declarations, and it sure
> doesn't check template specializations of `std::iterator_traits`, but it is a
> good start :) I'll revisit this when I have a little more time on my hand.
I played around a lot with clang-query in the past week but I haven't been able
to match typedefDecl with hasName("value_type"). Here are some matchers I tried:
match callExpr(allOf (callee(functionDecl(hasName("std::sort"))),
hasArgument(0,
stmt(hasDescendant(expr(hasType(typedefType(hasDeclaration(typedefNameDecl(hasName("value_type")))
match callExpr(allOf (callee(functionDecl(hasName("std::sort"))),
hasArgument(0,
hasDescendant(expr(hasType(typedefType(hasDeclaration(typedefDecl(matchesName(".*value.*"))
match callExpr(allOf (callee(functionDecl(hasName("std::sort"))),
hasArgument(0,
hasDescendant(declRefExpr(to(fieldDecl(hasName("value_type"))
Here's the AST dump for the IntPointerArray example:
FunctionDecl 0x639e958 <../../tst/s.cpp:5:1, line:10:1> line:5:5 main 'int ()'
`-CompoundStmt 0x639f1d0
|-DeclStmt 0x639ead0
| `-VarDecl 0x639ea70 col:8 used IntPointerArray 'int *[20]'
|-CallExpr 0x639ebd0 'void'
| |-ImplicitCastExpr 0x639ebb8 'void (*)(int **)'
| | `-DeclRefExpr 0x639eb68 'void (int **)' lvalue Function
0x639e890 'fill' 'void (int **)'
| `-ImplicitCastExpr 0x639ec00 'int **'
| `-DeclRefExpr 0x639eb40 'int *[20]' lvalue Var 0x639ea70
'IntPointerArray' 'int *[20]'
`-CallExpr 0x639f180 'void'
|-ImplicitCastExpr 0x639f168 'void (*)(int **, int **)'
| `-DeclRefExpr 0x639f0d0 'void (int **, int **)' lvalue
Function 0x639efd0 'sort' 'void (int **, int **)' (FunctionTemplate 0x638dd18
'sort')
|-ImplicitCastExpr 0x639f1b8 'int **'
| `-DeclRefExpr 0x639ec98 'int *[20]' lvalue Var 0x639ea70
'IntPointerArray' 'int *[20]'
`-BinaryOperator 0x639ed20 'int **' '+'
|-ImplicitCastExpr 0x639ed08 'int **'
| `-DeclRefExpr 0x639ecc0 'int *[20]' lvalue Var 0x639ea70
'IntPointerArray' 'int *[20]'
`-IntegerLiteral 0x639ece8 'int' 20
Repository:
rC Clang
https://reviews.llvm.org/D50488
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements
mgrang added a comment. In https://reviews.llvm.org/D50488#1222837, @whisperity wrote: > > I'm not sure if we discussed, has this checker been tried on some in-the-wild > examples? To see if there are some real findings or crashes? > There is a good selection of projects here in this tool: > https://github.com/Xazax-hun/csa-testbench. Thanks @whisperity for pointing me to the Static Analyzer tests. I will run the checker on it and report my findings here. Repository: rC Clang https://reviews.llvm.org/D50488 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys
mgrang updated this revision to Diff 163876.
mgrang added a comment.
Addressed comments.
https://reviews.llvm.org/D50488
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
test/Analysis/ptr-sort.cpp
Index: test/Analysis/ptr-sort.cpp
===
--- /dev/null
+++ test/Analysis/ptr-sort.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.nondeterminism.PointerSorting %s -analyzer-output=text -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+namespace std {
+ template
+ bool is_sorted(ForwardIt first, ForwardIt last);
+
+ template
+ void nth_element(RandomIt first, RandomIt nth, RandomIt last);
+
+ template
+ void partial_sort(RandomIt first, RandomIt middle, RandomIt last);
+
+ template
+ void sort (RandomIt first, RandomIt last);
+
+ template
+ void stable_sort(RandomIt first, RandomIt last);
+
+ template
+ BidirIt partition(BidirIt first, BidirIt last, UnaryPredicate p);
+
+ template
+ BidirIt stable_partition(BidirIt first, BidirIt last, UnaryPredicate p);
+}
+
+bool f (int x) { return true; }
+bool g (int *x) { return true; }
+
+void PointerSorting() {
+ int a = 1, b = 2, c = 3;
+ std::vector V1 = {a, b};
+ std::vector V2 = {, };
+
+ std::is_sorted(V1.begin(), V1.end());// no-warning
+ std::nth_element(V1.begin(), V1.begin() + 1, V1.end()); // no-warning
+ std::partial_sort(V1.begin(), V1.begin() + 1, V1.end()); // no-warning
+ std::sort(V1.begin(), V1.end()); // no-warning
+ std::stable_sort(V1.begin(), V1.end()); // no-warning
+ std::partition(V1.begin(), V1.end(), f); // no-warning
+ std::stable_partition(V1.begin(), V1.end(), g); // no-warning
+
+ std::is_sorted(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ std::nth_element(V2.begin(), V2.begin() + 1, V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ std::partial_sort(V2.begin(), V2.begin() + 1, V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ std::sort(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ std::stable_sort(V2.begin(), V2.end()); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ std::partition(V2.begin(), V2.end(), f); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ std::stable_partition(V2.begin(), V2.end(), g); // expected-warning {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+ // expected-note@-1 {{Sorting pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerSorting]
+}
Index: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
@@ -0,0 +1,110 @@
+//===-- PointerSortingChecker.cpp -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines PointerSortingChecker which checks for non-determinism
+// caused due to sorting containers with pointer-like elements.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
[PATCH] D45898: [SemaCXX] Mark destructor as referenced
ahatanak updated this revision to Diff 163871.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
Point the diagnostic at either the relevant init list element or at the close
brace.
Repository:
rC Clang
https://reviews.llvm.org/D45898
Files:
lib/Sema/SemaInit.cpp
test/CodeGenObjCXX/arc-list-init-destruct.mm
test/SemaCXX/aggregate-initialization.cpp
Index: test/SemaCXX/aggregate-initialization.cpp
===
--- test/SemaCXX/aggregate-initialization.cpp
+++ test/SemaCXX/aggregate-initialization.cpp
@@ -186,3 +186,51 @@
// amount of time.
struct A { int n; int arr[1000 * 1000 * 1000]; } a = {1, {2}};
}
+
+namespace ElementDestructor {
+ // The destructor for each element of class type is potentially invoked
+ // (15.4 [class.dtor]) from the context where the aggregate initialization
+ // occurs. Produce a diagnostic if an element's destructor isn't accessible.
+
+ class X { int f; ~X(); }; // expected-note {{implicitly declared private here}}
+ struct Y { X x; };
+
+ void test0() {
+auto *y = new Y {}; // expected-error {{temporary of type 'ElementDestructor::X' has private destructor}}
+ }
+
+ struct S0 { int f; ~S0() = delete; }; // expected-note 3 {{'~S0' has been explicitly marked deleted here}}
+ struct S1 { S0 s0; int f; };
+
+ S1 test1() {
+auto *t = new S1 { .f = 1 }; // expected-error {{attempt to use a deleted function}}
+return {2}; // expected-error {{attempt to use a deleted function}}
+ }
+
+ // Check if the type of an array element has a destructor.
+ struct S2 { S0 a[4]; };
+
+ void test2() {
+auto *t = new S2 {1,2,3,4}; // expected-error {{attempt to use a deleted function}}
+ }
+
+#if __cplusplus >= 201703L
+ namespace BaseDestructor {
+ struct S0 { int f; ~S0() = delete; }; // expected-note {{'~S0' has been explicitly marked deleted here}}
+
+// Check destructor of base class.
+struct S3 : S0 {};
+
+void test3() {
+ S3 s3 = {1}; // expected-error {{attempt to use a deleted function}}
+}
+ }
+#endif
+
+ // A's destructor doesn't have to be accessible from the context of C's
+ // initialization.
+ struct A { friend struct B; private: ~A(); };
+ struct B { B(); A a; };
+ struct C { B b; };
+ C c = { B() };
+}
Index: test/CodeGenObjCXX/arc-list-init-destruct.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/arc-list-init-destruct.mm
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13.0 -std=c++1z -fobjc-arc -fobjc-exceptions -fcxx-exceptions -fexceptions -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[V0:.*]] = type opaque
+// CHECK: %[[STRUCT_CLASS1:.*]] = type { %[[V0]]* }
+
+@interface Class0;
+@end
+
+struct Class1 {
+ Class0 *f;
+};
+
+struct Container {
+ Class1 a;
+ bool b;
+};
+
+bool getBool() {
+ return false;
+}
+
+Class0 *g;
+
+// CHECK: define {{.*}} @_Z4testv()
+// CHECK: invoke zeroext i1 @_Z7getBoolv()
+// CHECK: landingpad { i8*, i32 }
+// CHECK: call void @_ZN6Class1D1Ev(%[[STRUCT_CLASS1]]* %{{.*}})
+// CHECK: br label
+
+// CHECK: define linkonce_odr void @_ZN6Class1D1Ev(
+
+Container test() {
+ return {{g}, getBool()};
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -1818,6 +1818,30 @@
return FlexArrayDiag != diag::ext_flexible_array_init;
}
+/// Check if the type of a class element has an accessible destructor.
+///
+/// Aggregate initialization requires a class element's destructor be
+/// accessible per 11.6.1 [dcl.init.aggr]:
+///
+/// The destructor for each element of class type is potentially invoked
+/// (15.4 [class.dtor]) from the context where the aggregate initialization
+/// occurs.
+static bool hasAccessibleDestructor(QualType ElementType, SourceLocation Loc,
+Sema ) {
+ auto *CXXRD = ElementType->getAsCXXRecordDecl();
+ if (!CXXRD)
+return false;
+
+ CXXDestructorDecl *Destructor = SemaRef.LookupDestructor(CXXRD);
+ SemaRef.CheckDestructorAccess(Loc, Destructor,
+SemaRef.PDiag(diag::err_access_dtor_temp)
+<< ElementType);
+ SemaRef.MarkFunctionReferenced(Loc, Destructor);
+ if (SemaRef.DiagnoseUseOfDecl(Destructor, Loc))
+return true;
+ return false;
+}
+
void InitListChecker::CheckStructUnionTypes(
const InitializedEntity , InitListExpr *IList, QualType DeclType,
CXXRecordDecl::base_class_range Bases, RecordDecl::field_iterator Field,
@@ -1838,6 +1862,15 @@
if (DeclType->isUnionType() && IList->getNumInits() == 0) {
RecordDecl *RD = DeclType->getAs()->getDecl();
+if (!VerifyOnly)
+ for (FieldDecl *FD : RD->fields()) {
+QualType ET = SemaRef.Context.getBaseElementType(FD->getType());
+if (hasAccessibleDestructor(ET, IList->getEndLoc(), SemaRef))
[PATCH] D45898: [SemaCXX] Mark destructor as referenced
ahatanak added inline comments.
Comment at: lib/Sema/SemaInit.cpp:1827-1829
+ if (SemaRef.DiagnoseUseOfDecl(Destructor, Loc))
+return false;
+ return true;
rsmith wrote:
> Usual Clang convention is to return `true` on error.
I also renamed the function to hasAccessibleDestructor to make it clearer what
the function does.
Comment at: lib/Sema/SemaInit.cpp:1856
+if (auto *CXXRD = DeclType->getAsCXXRecordDecl()) {
+ SourceLocation Loc = IList->getBeginLoc();
+ for (auto : Bases)
rsmith wrote:
> It's a minor thing, but I think it'd be preferable to point the diagnostic at
> the relevant init list element, or at the close brace if the initializer was
> omitted.
The function that checks the destructor (hasAccessibleDestructor) is called in
five different places now.
Repository:
rC Clang
https://reviews.llvm.org/D45898
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51354: Fix the -print-multi-directory flag to print the selected multilib.
thakis added a comment. The test fails on my system like so: -- FAIL: Clang :: Driver/print-multi-directory.c (4696 of 13174) TEST 'Clang :: Driver/print-multi-directory.c' FAILED Script: -- : 'RUN: at line 1'; /usr/local/google/home/thakis/src/llvm-build-goma/bin/clang -no-canonical-prefixes /usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/print-multi-directory.c -### -o /usr/local/google/home/thakis/src/llvm-build-goma/tools/clang/test/Driver/Output/print-multi-directory.c.tmp.o 2>&1 -target i386-none-linux -print-multi-directory| /usr/local/google/home/thakis/src/llvm-build-goma/bin/FileCheck --check-prefix=CHECK-X86-MULTILIBS /usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/print-multi-directory.c : 'RUN: at line 10'; /usr/local/google/home/thakis/src/llvm-build-goma/bin/clang -no-canonical-prefixes /usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/print-multi-directory.c -### -o /usr/local/google/home/thakis/src/llvm-build-goma/tools/clang/test/Driver/Output/print-multi-directory.c.tmp.o 2>&1 -target i386-none-linux -m64 -print-multi-directory| /usr/local/google/home/thakis/src/llvm-build-goma/bin/FileCheck --check-prefix=CHECK-X86_64-MULTILIBS /usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/print-multi-directory.c : 'RUN: at line 19'; /usr/local/google/home/thakis/src/llvm-build-goma/bin/clang -no-canonical-prefixes /usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/print-multi-directory.c -### -o /usr/local/google/home/thakis/src/llvm-build-goma/tools/clang/test/Driver/Output/print-multi-directory.c.tmp.o 2>&1 -target arm-linux-androideabi21 -stdlib=libstdc++ -mthumb -B/usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/Inputs/basic_android_ndk_tree --sysroot=/usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/Inputs/basic_android_ndk_tree/sysroot -print-multi-directory| /usr/local/google/home/thakis/src/llvm-build-goma/bin/FileCheck --check-prefix=CHECK-ARM-MULTILIBS /usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/print-multi-directory.c -- Exit Code: 1 Command Output (stderr): -- /usr/local/google/home/thakis/src/llvm-rw/tools/clang/test/Driver/print-multi-directory.c:6:25: error: CHECK-X86-MULTILIBS: expected string not found in input // CHECK-X86-MULTILIBS: 32 ^ :1:1: note: scanning from here clang version 8.0.0 (trunk 341389) ^ :1:28: note: possible intended match here clang version 8.0.0 (trunk 341389) ^ -- Repository: rC Clang https://reviews.llvm.org/D51354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51635: clang-cl: Pass /Brepro to linker if it was passed to the compiler
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341390: clang-cl: Pass /Brepro to linker if it was passed to
the compiler (authored by nico, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51635?vs=163829=163870#toc
Repository:
rC Clang
https://reviews.llvm.org/D51635
Files:
lib/Driver/ToolChains/MSVC.cpp
test/Driver/msvc-link.c
Index: test/Driver/msvc-link.c
===
--- test/Driver/msvc-link.c
+++ test/Driver/msvc-link.c
@@ -3,6 +3,7 @@
// BASIC: "-out:a.exe"
// BASIC: "-defaultlib:libcmt"
// BASIC: "-nologo"
+// BASIC-NOT: "-Brepro"
// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 |
FileCheck --check-prefix=DLL %s
// DLL: link.exe"
@@ -16,3 +17,14 @@
// LIBPATH: "-libpath:/usr/lib"
// LIBPATH: "-nologo"
+// RUN: %clang_cl /Brepro -### %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// REPRO: link.exe"
+// REPRO: "-out:msvc-link.exe"
+// REPRO: "-nologo"
+// REPRO: "-Brepro"
+
+// RUN: %clang_cl /Brepro- -### %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// NOREPRO: link.exe"
+// NOREPRO: "-out:msvc-link.exe"
+// NOREPRO: "-nologo"
+// NOREPRO-NOT: "-Brepro"
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -355,6 +355,15 @@
options::OPT__SLASH_Zd))
CmdArgs.push_back("-debug");
+ // Pass on /Brepro if it was passed to the compiler.
+ // Note that /Brepro maps to -mno-incremental-linker-compatible.
+ bool DefaultIncrementalLinkerCompatible =
+ C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
+ if (!Args.hasFlag(options::OPT_mincremental_linker_compatible,
+options::OPT_mno_incremental_linker_compatible,
+DefaultIncrementalLinkerCompatible))
+CmdArgs.push_back("-Brepro");
+
bool DLL = Args.hasArg(options::OPT__SLASH_LD, options::OPT__SLASH_LDd,
options::OPT_shared);
if (DLL) {
Index: test/Driver/msvc-link.c
===
--- test/Driver/msvc-link.c
+++ test/Driver/msvc-link.c
@@ -3,6 +3,7 @@
// BASIC: "-out:a.exe"
// BASIC: "-defaultlib:libcmt"
// BASIC: "-nologo"
+// BASIC-NOT: "-Brepro"
// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 | FileCheck --check-prefix=DLL %s
// DLL: link.exe"
@@ -16,3 +17,14 @@
// LIBPATH: "-libpath:/usr/lib"
// LIBPATH: "-nologo"
+// RUN: %clang_cl /Brepro -### %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// REPRO: link.exe"
+// REPRO: "-out:msvc-link.exe"
+// REPRO: "-nologo"
+// REPRO: "-Brepro"
+
+// RUN: %clang_cl /Brepro- -### %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// NOREPRO: link.exe"
+// NOREPRO: "-out:msvc-link.exe"
+// NOREPRO: "-nologo"
+// NOREPRO-NOT: "-Brepro"
Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -355,6 +355,15 @@
options::OPT__SLASH_Zd))
CmdArgs.push_back("-debug");
+ // Pass on /Brepro if it was passed to the compiler.
+ // Note that /Brepro maps to -mno-incremental-linker-compatible.
+ bool DefaultIncrementalLinkerCompatible =
+ C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
+ if (!Args.hasFlag(options::OPT_mincremental_linker_compatible,
+options::OPT_mno_incremental_linker_compatible,
+DefaultIncrementalLinkerCompatible))
+CmdArgs.push_back("-Brepro");
+
bool DLL = Args.hasArg(options::OPT__SLASH_LD, options::OPT__SLASH_LDd,
options::OPT_shared);
if (DLL) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341390 - clang-cl: Pass /Brepro to linker if it was passed to the compiler
Author: nico
Date: Tue Sep 4 11:00:14 2018
New Revision: 341390
URL: http://llvm.org/viewvc/llvm-project?rev=341390=rev
Log:
clang-cl: Pass /Brepro to linker if it was passed to the compiler
Differential Revision: https://reviews.llvm.org/D51635
Modified:
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
cfe/trunk/test/Driver/msvc-link.c
Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=341390=341389=341390=diff
==
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Tue Sep 4 11:00:14 2018
@@ -355,6 +355,15 @@ void visualstudio::Linker::ConstructJob(
options::OPT__SLASH_Zd))
CmdArgs.push_back("-debug");
+ // Pass on /Brepro if it was passed to the compiler.
+ // Note that /Brepro maps to -mno-incremental-linker-compatible.
+ bool DefaultIncrementalLinkerCompatible =
+ C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
+ if (!Args.hasFlag(options::OPT_mincremental_linker_compatible,
+options::OPT_mno_incremental_linker_compatible,
+DefaultIncrementalLinkerCompatible))
+CmdArgs.push_back("-Brepro");
+
bool DLL = Args.hasArg(options::OPT__SLASH_LD, options::OPT__SLASH_LDd,
options::OPT_shared);
if (DLL) {
Modified: cfe/trunk/test/Driver/msvc-link.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/msvc-link.c?rev=341390=341389=341390=diff
==
--- cfe/trunk/test/Driver/msvc-link.c (original)
+++ cfe/trunk/test/Driver/msvc-link.c Tue Sep 4 11:00:14 2018
@@ -3,6 +3,7 @@
// BASIC: "-out:a.exe"
// BASIC: "-defaultlib:libcmt"
// BASIC: "-nologo"
+// BASIC-NOT: "-Brepro"
// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 |
FileCheck --check-prefix=DLL %s
// DLL: link.exe"
@@ -16,3 +17,14 @@
// LIBPATH: "-libpath:/usr/lib"
// LIBPATH: "-nologo"
+// RUN: %clang_cl /Brepro -### %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// REPRO: link.exe"
+// REPRO: "-out:msvc-link.exe"
+// REPRO: "-nologo"
+// REPRO: "-Brepro"
+
+// RUN: %clang_cl /Brepro- -### %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// NOREPRO: link.exe"
+// NOREPRO: "-out:msvc-link.exe"
+// NOREPRO: "-nologo"
+// NOREPRO-NOT: "-Brepro"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50229: +fp16fml feature for ARM and AArch64
efriedma added a comment. Do you expect that the regression tests will be affected by the TargetParser fixes? If not, I guess this is okay... but please add clear comments explaining which bits of code you expect to go away after the TargetParser rewrite. Repository: rC Clang https://reviews.llvm.org/D50229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51645: [CMake] Don't use -rtlib=compiler-rt with -nodefaultlibs.
cdavis5x created this revision.
cdavis5x added reviewers: mstorsjo, rnk.
Herald added subscribers: cfe-commits, christof, mgorny, dberris.
This switch only has an effect at link time. It changes the default
compiler support library to `compiler-rt`. With `-nodefaultlibs`, this
library won't get linked anyway; Clang actually warns about that.
Repository:
rUNW libunwind
https://reviews.llvm.org/D51645
Files:
CMakeLists.txt
cmake/config-ix.cmake
Index: cmake/config-ix.cmake
===
--- cmake/config-ix.cmake
+++ cmake/config-ix.cmake
@@ -23,7 +23,6 @@
list(APPEND CMAKE_REQUIRED_LIBRARIES c)
endif ()
if (LIBUNWIND_USE_COMPILER_RT)
-list(APPEND CMAKE_REQUIRED_FLAGS -rtlib=compiler-rt)
find_compiler_rt_library(builtins LIBUNWIND_BUILTINS_LIBRARY)
list(APPEND CMAKE_REQUIRED_LIBRARIES "${LIBUNWIND_BUILTINS_LIBRARY}")
elseif (LIBUNWIND_HAS_GCC_S_LIB)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -234,7 +234,7 @@
# Configure compiler.
include(config-ix)
-if (LIBUNWIND_USE_COMPILER_RT)
+if (LIBUNWIND_USE_COMPILER_RT AND NOT LIBUNWIND_HAS_NODEFAULTLIBS_FLAG)
list(APPEND LIBUNWIND_LINK_FLAGS "-rtlib=compiler-rt")
endif()
Index: cmake/config-ix.cmake
===
--- cmake/config-ix.cmake
+++ cmake/config-ix.cmake
@@ -23,7 +23,6 @@
list(APPEND CMAKE_REQUIRED_LIBRARIES c)
endif ()
if (LIBUNWIND_USE_COMPILER_RT)
-list(APPEND CMAKE_REQUIRED_FLAGS -rtlib=compiler-rt)
find_compiler_rt_library(builtins LIBUNWIND_BUILTINS_LIBRARY)
list(APPEND CMAKE_REQUIRED_LIBRARIES "${LIBUNWIND_BUILTINS_LIBRARY}")
elseif (LIBUNWIND_HAS_GCC_S_LIB)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -234,7 +234,7 @@
# Configure compiler.
include(config-ix)
-if (LIBUNWIND_USE_COMPILER_RT)
+if (LIBUNWIND_USE_COMPILER_RT AND NOT LIBUNWIND_HAS_NODEFAULTLIBS_FLAG)
list(APPEND LIBUNWIND_LINK_FLAGS "-rtlib=compiler-rt")
endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51644: [CMake] Remove variable reference that isn't used.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341388: [CMake] Remove variable reference that isnt
used. (authored by cdavis, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D51644
Files:
libunwind/trunk/src/CMakeLists.txt
Index: libunwind/trunk/src/CMakeLists.txt
===
--- libunwind/trunk/src/CMakeLists.txt
+++ libunwind/trunk/src/CMakeLists.txt
@@ -51,7 +51,7 @@
${LIBUNWIND_ASM_SOURCES})
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
Index: libunwind/trunk/src/CMakeLists.txt
===
--- libunwind/trunk/src/CMakeLists.txt
+++ libunwind/trunk/src/CMakeLists.txt
@@ -51,7 +51,7 @@
${LIBUNWIND_ASM_SOURCES})
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r341388 - [CMake] Remove variable reference that isn't used.
Author: cdavis
Date: Tue Sep 4 10:40:26 2018
New Revision: 341388
URL: http://llvm.org/viewvc/llvm-project?rev=341388=rev
Log:
[CMake] Remove variable reference that isn't used.
Summary:
This variable is never defined, so its value is always empty. Since
`libunwind` is needed to build the C++ ABI library in the first place,
it should never be linked to the C++ ABI library anyway.
Reviewers: mstorsjo, rnk
Subscribers: mgorny, christof, cfe-commits
Differential Revision: https://reviews.llvm.org/D51644
Modified:
libunwind/trunk/src/CMakeLists.txt
Modified: libunwind/trunk/src/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=341388=341387=341388=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Tue Sep 4 10:40:26 2018
@@ -51,7 +51,7 @@ set(LIBUNWIND_SOURCES
${LIBUNWIND_ASM_SOURCES})
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal
NoQ added a comment. In https://reviews.llvm.org/D51300#1220537, @Szelethus wrote: > Would you be comfortable me commiting this without that assert Yup sure. In https://reviews.llvm.org/D51300#1223042, @Szelethus wrote: > I'm not even sure how this is possible -- and unfortunately I've been unable > to create a minimal (not) working example for this, and I wasn't even able to > recreate the error locally. Sounds like a test case would be great to have. Consider extracting a preprocessed file and running it under //creduce//, that's a great generic method for obtaining small reproducers for crashes and regressions (but not for false positives). As far as i understand, it's a crash on llvm codebase, which should be easy to re-analyze locally, even just one file, because llvm is built with cmake and dumps compilation databases, so just use clang-check on a single file, or simply append --analyze to the compilation database run-line. This specific problem sounds elusive because it's a problem with pointer casts, and pointer casts are currently a mess. I cannot state for sure that typed this-region type must always be a record, but it's definitely a bad smell when it is't. So i recommend a quick investigation of whether the region in question is (1) well-formed and (2) correctly reflects the semantics of the program. https://reviews.llvm.org/D51300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51644: [CMake] Remove variable reference that isn't used.
mstorsjo accepted this revision.
mstorsjo added inline comments.
This revision is now accepted and ready to land.
Comment at: src/CMakeLists.txt:54
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
cdavis5x wrote:
> mstorsjo wrote:
> > Is there any point in this line at all now, or can it be removed altogether?
> I think `list(APPEND)` can fail if the variable doesn't exist.
Okay - after reading the cmake docs, I agree that this seems to be the correct
solution.
Repository:
rUNW libunwind
https://reviews.llvm.org/D51644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51641: [VFS] Cache the current working directory for the real FS.
ioeric added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:275 return EC; - return Dir.str().str(); + CWDCache = Dir.str(); + return CWDCache; sammccall wrote: > Doesn't this need to be guarded by a lock? I know the current version is > thread-hostile in principle but this seems likely to lead to corruption in > multithreaded programs. > (I suspect clangd tests would fail under TSan for example) Done. Thanks for the catch! Repository: rC Clang https://reviews.llvm.org/D51641 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51641: [VFS] Cache the current working directory for the real FS.
ioeric updated this revision to Diff 163858.
ioeric marked an inline comment as done.
ioeric added a comment.
- Guard CWDCache with mutex.
Repository:
rC Clang
https://reviews.llvm.org/D51641
Files:
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -25,9 +25,9 @@
#include "llvm/ADT/Twine.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Config/llvm-config.h"
-#include "llvm/Support/Compiler.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Chrono.h"
+#include "llvm/Support/Compiler.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/ErrorHandling.h"
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -243,6 +244,10 @@
std::error_code setCurrentWorkingDirectory(const Twine ) override;
std::error_code getRealPath(const Twine ,
SmallVectorImpl ) const override;
+private:
+ // Make sure `CWDCache` update is thread safe in
`getCurrentWorkingDirectory`.
+ mutable std::mutex Mutex;
+ mutable std::string CWDCache;
};
} // namespace
@@ -265,10 +270,14 @@
}
llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const {
+ std::lock_guard Lock(Mutex);
+ if (!CWDCache.empty())
+return CWDCache;
SmallString<256> Dir;
if (std::error_code EC = llvm::sys::fs::current_path(Dir))
return EC;
- return Dir.str().str();
+ CWDCache = Dir.str();
+ return CWDCache;
}
std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine ) {
@@ -279,7 +288,13 @@
// difference for example on network filesystems, where symlinks might be
// switched during runtime of the tool. Fixing this depends on having a
// file system abstraction that allows openat() style interactions.
- return llvm::sys::fs::set_current_path(Path);
+ if (auto EC = llvm::sys::fs::set_current_path(Path))
+return EC;
+
+ // Invalidate cache.
+ std::lock_guard Lock(Mutex);
+ CWDCache.clear();
+ return std::error_code();
}
std::error_code
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -25,9 +25,9 @@
#include "llvm/ADT/Twine.h"
#include "llvm/ADT/iterator_range.h"
#include "llvm/Config/llvm-config.h"
-#include "llvm/Support/Compiler.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/Chrono.h"
+#include "llvm/Support/Compiler.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/ErrorHandling.h"
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -243,6 +244,10 @@
std::error_code setCurrentWorkingDirectory(const Twine ) override;
std::error_code getRealPath(const Twine ,
SmallVectorImpl ) const override;
+private:
+ // Make sure `CWDCache` update is thread safe in `getCurrentWorkingDirectory`.
+ mutable std::mutex Mutex;
+ mutable std::string CWDCache;
};
} // namespace
@@ -265,10 +270,14 @@
}
llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const {
+ std::lock_guard Lock(Mutex);
+ if (!CWDCache.empty())
+return CWDCache;
SmallString<256> Dir;
if (std::error_code EC = llvm::sys::fs::current_path(Dir))
return EC;
- return Dir.str().str();
+ CWDCache = Dir.str();
+ return CWDCache;
}
std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine ) {
@@ -279,7 +288,13 @@
// difference for example on network filesystems, where symlinks might be
// switched during runtime of the tool. Fixing this depends on having a
// file system abstraction that allows openat() style interactions.
- return llvm::sys::fs::set_current_path(Path);
+ if (auto EC = llvm::sys::fs::set_current_path(Path))
+return EC;
+
+ // Invalidate cache.
+ std::lock_guard Lock(Mutex);
+ CWDCache.clear();
+ return std::error_code();
}
std::error_code
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51644: [CMake] Remove variable reference that isn't used.
cdavis5x added inline comments.
Comment at: src/CMakeLists.txt:54
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
mstorsjo wrote:
> Is there any point in this line at all now, or can it be removed altogether?
I think `list(APPEND)` can fail if the variable doesn't exist.
Repository:
rUNW libunwind
https://reviews.llvm.org/D51644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51644: [CMake] Remove variable reference that isn't used.
mstorsjo added inline comments.
Comment at: src/CMakeLists.txt:54
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
Is there any point in this line at all now, or can it be removed altogether?
Repository:
rUNW libunwind
https://reviews.llvm.org/D51644
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51644: [CMake] Remove variable reference that isn't used.
cdavis5x created this revision.
cdavis5x added reviewers: mstorsjo, rnk.
Herald added subscribers: cfe-commits, christof, mgorny.
This variable is never defined, so its value is always empty. Since
`libunwind` is needed to build the C++ ABI library in the first place,
it should never be linked to the C++ ABI library anyway.
Repository:
rUNW libunwind
https://reviews.llvm.org/D51644
Files:
src/CMakeLists.txt
Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -51,7 +51,7 @@
${LIBUNWIND_ASM_SOURCES})
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -51,7 +51,7 @@
${LIBUNWIND_ASM_SOURCES})
# Generate library list.
-set(libraries ${LIBUNWINDCXX_ABI_LIBRARIES})
+set(libraries)
append_if(libraries LIBUNWIND_HAS_C_LIB c)
append_if(libraries LIBUNWIND_HAS_GCC_S_LIB gcc_s)
append_if(libraries LIBUNWIND_HAS_DL_LIB dl)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51501: [CUDA] Fix CUDA compilation broken by D50845
tra abandoned this revision. tra added a comment. > Not needed anymore after the reverts in https://reviews.llvm.org/rC341115 and > https://reviews.llvm.org/rC341118, right? Correct. https://reviews.llvm.org/D51501 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51641: [VFS] Cache the current working directory for the real FS.
sammccall added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:275 return EC; - return Dir.str().str(); + CWDCache = Dir.str(); + return CWDCache; Doesn't this need to be guarded by a lock? I know the current version is thread-hostile in principle but this seems likely to lead to corruption in multithreaded programs. (I suspect clangd tests would fail under TSan for example) Repository: rC Clang https://reviews.llvm.org/D51641 ___ 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.
martong added a comment. This patch is huge, but we change here almost all implementation functions of `ASTNodeImporter` to return with either `Error` or with `Expected`. We could not really come up with a cohesive but smaller patch because of the recursive nature of the importer. (We are open to ideas about how to cut this patch into smaller parts.) Most of the changes are trivial: we always have to check the return value of any import function and pass on the error to the caller if there was any. In my opinion one great simplification is the introduction of the auxiliary `importSeq` function. It imports each entity one-by-one after each other and in case of any error it returns with that error and does not continue with the subsequent import operations. 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] D51638: [clangd] Load static index asynchronously, add tracing.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341376: [clangd] Load static index asynchronously, add
tracing. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51638?vs=163840=163847#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51638
Files:
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -281,9 +281,15 @@
Opts.BuildDynamicSymbolIndex = EnableIndex;
std::unique_ptr StaticIdx;
if (EnableIndex && !YamlSymbolFile.empty()) {
-StaticIdx = loadIndex(YamlSymbolFile, UseDex);
-Opts.StaticIndex = StaticIdx.get();
+// Load the index asynchronously. Meanwhile SwapIndex returns no results.
+SwapIndex *Placeholder;
+StaticIdx.reset(Placeholder = new
SwapIndex(llvm::make_unique()));
+runAsync([Placeholder] {
+ if (auto Idx = loadIndex(YamlSymbolFile))
+Placeholder->reset(std::move(Idx));
+});
}
+ Opts.StaticIndex = StaticIdx.get();
Opts.AsyncThreadsCount = WorkerThreadsCount;
clangd::CodeCompleteOptions CCOpts;
Index: clang-tools-extra/trunk/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.h
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.h
@@ -44,7 +44,7 @@
// Build an in-memory static index for global symbols from a symbol file.
// The size of global symbols should be relatively small, so that all symbols
// can be managed in memory.
-std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+std::unique_ptr loadIndex(llvm::StringRef SymbolFilename,
bool UseDex = true);
} // namespace clangd
Index: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "SymbolYAML.h"
+#include "../Trace.h"
#include "Index.h"
#include "Serialization.h"
#include "dex/DexIndex.h"
@@ -183,25 +184,31 @@
return OS.str();
}
-std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+std::unique_ptr loadIndex(llvm::StringRef SymbolFilename,
bool UseDex) {
- auto Buffer = llvm::MemoryBuffer::getFile(SymbolFile);
+ trace::Span OverallTracer("LoadIndex");
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFile << "\n";
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
return nullptr;
}
StringRef Data = Buffer->get()->getBuffer();
llvm::Optional Slab;
if (Data.startswith("RIFF")) { // Magic for binary index file.
+trace::Span Tracer("ParseRIFF");
if (auto RIFF = readIndexFile(Data))
Slab = std::move(RIFF->Symbols);
else
llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
} else {
+trace::Span Tracer("ParseYAML");
Slab = symbolsFromYAML(Data);
}
+ if (!Slab)
+return nullptr;
+ trace::Span Tracer("BuildIndex");
return UseDex ? dex::DexIndex::build(std::move(*Slab))
: MemIndex::build(std::move(*Slab), RefSlab());
}
Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -281,9 +281,15 @@
Opts.BuildDynamicSymbolIndex = EnableIndex;
std::unique_ptr StaticIdx;
if (EnableIndex && !YamlSymbolFile.empty()) {
-StaticIdx = loadIndex(YamlSymbolFile, UseDex);
-Opts.StaticIndex = StaticIdx.get();
+// Load the index asynchronously. Meanwhile SwapIndex returns no results.
+SwapIndex *Placeholder;
+StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique()));
+runAsync([Placeholder] {
+ if (auto Idx = loadIndex(YamlSymbolFile))
+Placeholder->reset(std::move(Idx));
+});
}
+ Opts.StaticIndex = StaticIdx.get();
Opts.AsyncThreadsCount = WorkerThreadsCount;
clangd::CodeCompleteOptions CCOpts;
Index: clang-tools-extra/trunk/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.h
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.h
@@ -44,7 +44,7 @@
// Build an in-memory static index for global symbols from a symbol
[PATCH] D51641: [VFS] Cache the current working directory for the real FS.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D51641
Files:
lib/Basic/VirtualFileSystem.cpp
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -243,6 +243,8 @@
std::error_code setCurrentWorkingDirectory(const Twine ) override;
std::error_code getRealPath(const Twine ,
SmallVectorImpl ) const override;
+private:
+ mutable std::string CWDCache;
};
} // namespace
@@ -265,10 +267,13 @@
}
llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const {
+ if (!CWDCache.empty())
+return CWDCache;
SmallString<256> Dir;
if (std::error_code EC = llvm::sys::fs::current_path(Dir))
return EC;
- return Dir.str().str();
+ CWDCache = Dir.str();
+ return CWDCache;
}
std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine ) {
@@ -279,7 +284,14 @@
// difference for example on network filesystems, where symlinks might be
// switched during runtime of the tool. Fixing this depends on having a
// file system abstraction that allows openat() style interactions.
- return llvm::sys::fs::set_current_path(Path);
+ if (auto EC = llvm::sys::fs::set_current_path(Path))
+return EC;
+
+ CWDCache.clear();
+ auto CWD = getCurrentWorkingDirectory();
+ if (CWD)
+CWDCache = CWD.get();
+ return CWD.getError();
}
std::error_code
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -243,6 +243,8 @@
std::error_code setCurrentWorkingDirectory(const Twine ) override;
std::error_code getRealPath(const Twine ,
SmallVectorImpl ) const override;
+private:
+ mutable std::string CWDCache;
};
} // namespace
@@ -265,10 +267,13 @@
}
llvm::ErrorOr RealFileSystem::getCurrentWorkingDirectory() const {
+ if (!CWDCache.empty())
+return CWDCache;
SmallString<256> Dir;
if (std::error_code EC = llvm::sys::fs::current_path(Dir))
return EC;
- return Dir.str().str();
+ CWDCache = Dir.str();
+ return CWDCache;
}
std::error_code RealFileSystem::setCurrentWorkingDirectory(const Twine ) {
@@ -279,7 +284,14 @@
// difference for example on network filesystems, where symlinks might be
// switched during runtime of the tool. Fixing this depends on having a
// file system abstraction that allows openat() style interactions.
- return llvm::sys::fs::set_current_path(Path);
+ if (auto EC = llvm::sys::fs::set_current_path(Path))
+return EC;
+
+ CWDCache.clear();
+ auto CWD = getCurrentWorkingDirectory();
+ if (CWD)
+CWDCache = CWD.get();
+ return CWD.getError();
}
std::error_code
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r341376 - [clangd] Load static index asynchronously, add tracing.
Author: sammccall
Date: Tue Sep 4 09:19:40 2018
New Revision: 341376
URL: http://llvm.org/viewvc/llvm-project?rev=341376=rev
Log:
[clangd] Load static index asynchronously, add tracing.
Summary:
Like D51475 but simplified based on recent patches.
While here, clarify that loadIndex() takes a filename, not file content.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51638
Modified:
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp?rev=341376=341375=341376=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Tue Sep 4 09:19:40 2018
@@ -8,6 +8,7 @@
//===--===//
#include "SymbolYAML.h"
+#include "../Trace.h"
#include "Index.h"
#include "Serialization.h"
#include "dex/DexIndex.h"
@@ -183,25 +184,31 @@ std::string SymbolToYAML(Symbol Sym) {
return OS.str();
}
-std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+std::unique_ptr loadIndex(llvm::StringRef SymbolFilename,
bool UseDex) {
- auto Buffer = llvm::MemoryBuffer::getFile(SymbolFile);
+ trace::Span OverallTracer("LoadIndex");
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFile << "\n";
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
return nullptr;
}
StringRef Data = Buffer->get()->getBuffer();
llvm::Optional Slab;
if (Data.startswith("RIFF")) { // Magic for binary index file.
+trace::Span Tracer("ParseRIFF");
if (auto RIFF = readIndexFile(Data))
Slab = std::move(RIFF->Symbols);
else
llvm::errs() << "Bad RIFF: " << llvm::toString(RIFF.takeError()) << "\n";
} else {
+trace::Span Tracer("ParseYAML");
Slab = symbolsFromYAML(Data);
}
+ if (!Slab)
+return nullptr;
+ trace::Span Tracer("BuildIndex");
return UseDex ? dex::DexIndex::build(std::move(*Slab))
: MemIndex::build(std::move(*Slab), RefSlab());
}
Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.h?rev=341376=341375=341376=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.h Tue Sep 4 09:19:40 2018
@@ -44,7 +44,7 @@ void SymbolsToYAML(const SymbolSlab
// Build an in-memory static index for global symbols from a symbol file.
// The size of global symbols should be relatively small, so that all symbols
// can be managed in memory.
-std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+std::unique_ptr loadIndex(llvm::StringRef SymbolFilename,
bool UseDex = true);
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=341376=341375=341376=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Tue Sep 4 09:19:40 2018
@@ -281,9 +281,15 @@ int main(int argc, char *argv[]) {
Opts.BuildDynamicSymbolIndex = EnableIndex;
std::unique_ptr StaticIdx;
if (EnableIndex && !YamlSymbolFile.empty()) {
-StaticIdx = loadIndex(YamlSymbolFile, UseDex);
-Opts.StaticIndex = StaticIdx.get();
+// Load the index asynchronously. Meanwhile SwapIndex returns no results.
+SwapIndex *Placeholder;
+StaticIdx.reset(Placeholder = new
SwapIndex(llvm::make_unique()));
+runAsync([Placeholder] {
+ if (auto Idx = loadIndex(YamlSymbolFile))
+Placeholder->reset(std::move(Idx));
+});
}
+ Opts.StaticIndex = StaticIdx.get();
Opts.AsyncThreadsCount = WorkerThreadsCount;
clangd::CodeCompleteOptions CCOpts;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341375: [clangd] Define a compact binary serialization fomat
for symbol slab/index. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51585?vs=163836=163845#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51585
Files:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/RIFF.cpp
clang-tools-extra/trunk/clangd/RIFF.h
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.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/Serialization.h
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/RIFFTests.cpp
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/RIFFTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/RIFFTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/RIFFTests.cpp
@@ -0,0 +1,39 @@
+//===-- RIFFTests.cpp - Binary container unit tests ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RIFF.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using namespace llvm;
+using ::testing::ElementsAre;
+
+TEST(RIFFTest, File) {
+ riff::File File{riff::fourCC("test"),
+ {
+ {riff::fourCC("even"), "abcd"},
+ {riff::fourCC("oddd"), "abcde"},
+ }};
+ StringRef Serialized = StringRef("RIFF\x1e\0\0\0test"
+ "even\x04\0\0\0abcd"
+ "oddd\x05\0\0\0abcde\0",
+ 38);
+
+ EXPECT_EQ(llvm::to_string(File), Serialized);
+ auto Parsed = riff::readFile(Serialized);
+ ASSERT_TRUE(bool(Parsed)) << Parsed.takeError();
+ EXPECT_EQ(*Parsed, File);
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
@@ -0,0 +1,138 @@
+//===-- SerializationTests.cpp - Binary and YAML serialization unit tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "index/Serialization.h"
+#include "index/SymbolYAML.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
+namespace clang {
+namespace clangd {
+namespace {
+
+const char *YAML1 = R"(
+---
+ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
+Name: 'Foo1'
+Scope: 'clang::'
+SymInfo:
+ Kind:Function
+ Lang:Cpp
+CanonicalDeclaration:
+ FileURI:file:///path/foo.h
+ Start:
+Line: 1
+Column: 0
+ End:
+Line: 1
+Column: 1
+IsIndexedForCodeCompletion:true
+Documentation:'Foo doc'
+ReturnType:'int'
+IncludeHeaders:
+ - Header:'include1'
+References:7
+ - Header:'include2'
+References:3
+...
+)";
+
+const char *YAML2 = R"(
+---
+ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF858
+Name: 'Foo2'
+Scope: 'clang::'
+SymInfo:
+ Kind:Function
+ Lang:Cpp
+CanonicalDeclaration:
+ FileURI:file:///path/bar.h
+ Start:
+Line: 1
+Column: 0
+ End:
+Line: 1
+Column: 1
+IsIndexedForCodeCompletion:false
+Signature:'-sig'
+CompletionSnippetSuffix:'-snippet'
+...
+)";
+
+MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
+ return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
+}
+
+TEST(SerializationTest, YAMLConversions) {
+ auto Symbols1 = symbolsFromYAML(YAML1);
+ ASSERT_EQ(Symbols1.size(), 1u);
+ const auto = *Symbols1.begin();
+ EXPECT_THAT(Sym1, QName("clang::Foo1"));
+
[clang-tools-extra] r341375 - [clangd] Define a compact binary serialization fomat for symbol slab/index.
Author: sammccall
Date: Tue Sep 4 09:16:50 2018
New Revision: 341375
URL: http://llvm.org/viewvc/llvm-project?rev=341375=rev
Log:
[clangd] Define a compact binary serialization fomat for symbol slab/index.
Summary:
This is intended to replace the current YAML format for general use.
It's ~10x more compact than YAML, and ~40% more compact than gzipped YAML:
llvmidx.riff = 20M, llvmidx.yaml = 272M, llvmidx.yaml.gz = 32M
It's also simpler/faster to read and write.
The format is a RIFF container (chunks of (type, size, data)) with:
- a compressed string table
- simple binary encoding of symbols (with varints for compactness)
It can be extended to include occurrences, Dex posting lists, etc.
There's no rich backwards-compatibility scheme, but a version number is included
so we can detect incompatible files and do ad-hoc back-compat.
Alternatives considered:
- compressed YAML or JSON: bulky and slow to load
- llvm bitstream: confusing model and libraries are hard to use. My attempt
produced slightly larger files, and the code was longer and slower.
- protobuf or similar: would be really nice (esp for back-compat) but the
dependency is a big hassle
- ad-hoc binary format without a container: it seems clear we're going
to add posting lists and occurrences here, and that they will benefit
from sharing a string table. The container makes it easy to debug
these pieces in isolation, and make them optional.
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, mgrang, arphaman,
kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D51585
Added:
clang-tools-extra/trunk/clangd/RIFF.cpp
clang-tools-extra/trunk/clangd/RIFF.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
clang-tools-extra/trunk/clangd/index/Serialization.h
clang-tools-extra/trunk/unittests/clangd/RIFFTests.cpp
clang-tools-extra/trunk/unittests/clangd/SerializationTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=341375=341374=341375=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Tue Sep 4 09:16:50 2018
@@ -29,6 +29,7 @@ add_clang_library(clangDaemon
Protocol.cpp
ProtocolHandlers.cpp
Quality.cpp
+ RIFF.cpp
SourceCode.cpp
Threading.cpp
Trace.cpp
@@ -41,6 +42,7 @@ add_clang_library(clangDaemon
index/Index.cpp
index/MemIndex.cpp
index/Merge.cpp
+ index/Serialization.cpp
index/SymbolCollector.cpp
index/SymbolYAML.cpp
Added: clang-tools-extra/trunk/clangd/RIFF.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/RIFF.cpp?rev=341375=auto
==
--- clang-tools-extra/trunk/clangd/RIFF.cpp (added)
+++ clang-tools-extra/trunk/clangd/RIFF.cpp Tue Sep 4 09:16:50 2018
@@ -0,0 +1,88 @@
+//===--- RIFF.cpp - Binary container file format
--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RIFF.h"
+#include "llvm/Support/Endian.h"
+
+using namespace llvm;
+namespace clang {
+namespace clangd {
+namespace riff {
+
+static Error makeError(const char *Msg) {
+ return createStringError(inconvertibleErrorCode(), Msg);
+}
+
+Expected readChunk(StringRef ) {
+ if (Stream.size() < 8)
+return makeError("incomplete chunk header");
+ Chunk C;
+ std::copy(Stream.begin(), Stream.begin() + 4, C.ID.begin());
+ Stream = Stream.drop_front(4);
+ uint32_t Len = support::endian::read32le(Stream.take_front(4).begin());
+ Stream = Stream.drop_front(4);
+ if (Stream.size() < Len)
+return makeError("truncated chunk");
+ C.Data = Stream.take_front(Len);
+ Stream = Stream.drop_front(Len);
+ if (Len % 2 & !Stream.empty()) { // Skip padding byte.
+if (Stream.front())
+ return makeError("nonzero padding byte");
+Stream = Stream.drop_front();
+ }
+ return C;
+};
+
+raw_ostream <<(raw_ostream , const Chunk ) {
+ OS.write(C.ID.begin(), C.ID.size());
+ char Size[4];
+ llvm::support::endian::write32le(Size, C.Data.size());
+ OS.write(Size,
[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional
martong added a comment. Here is the `Expected` based patch: https://reviews.llvm.org/D51633 Repository: rC Clang https://reviews.llvm.org/D50672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51475: [clangd] Load YAML static index asynchronously.
ioeric abandoned this revision. ioeric added a comment. Dropping this in favor of https://reviews.llvm.org/D51638 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51638: [clangd] Load static index asynchronously, add tracing.
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg. Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51638: [clangd] Load static index asynchronously, add tracing.
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Like https://reviews.llvm.org/D51475 but simplified based on recent patches.
While here, clarify that loadIndex() takes a filename, not file content.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51638
Files:
clangd/index/SymbolYAML.cpp
clangd/index/SymbolYAML.h
clangd/tool/ClangdMain.cpp
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -280,9 +280,15 @@
Opts.BuildDynamicSymbolIndex = EnableIndex;
std::unique_ptr StaticIdx;
if (EnableIndex && !YamlSymbolFile.empty()) {
-StaticIdx = loadIndex(YamlSymbolFile, UseDex);
-Opts.StaticIndex = StaticIdx.get();
+// Load the index asynchronously. Meanwhile SwapIndex returns no results.
+SwapIndex *Placeholder;
+StaticIdx.reset(Placeholder = new
SwapIndex(llvm::make_unique()));
+runAsync([Placeholder] {
+ if (auto Idx = loadIndex(YamlSymbolFile))
+Placeholder->reset(std::move(Idx));
+});
}
+ Opts.StaticIndex = StaticIdx.get();
Opts.AsyncThreadsCount = WorkerThreadsCount;
clangd::CodeCompleteOptions CCOpts;
Index: clangd/index/SymbolYAML.h
===
--- clangd/index/SymbolYAML.h
+++ clangd/index/SymbolYAML.h
@@ -44,7 +44,7 @@
// Build an in-memory static index for global symbols from a symbol file.
// The size of global symbols should be relatively small, so that all symbols
// can be managed in memory.
-std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+std::unique_ptr loadIndex(llvm::StringRef SymbolFilename,
bool UseDex = true);
} // namespace clangd
Index: clangd/index/SymbolYAML.cpp
===
--- clangd/index/SymbolYAML.cpp
+++ clangd/index/SymbolYAML.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "SymbolYAML.h"
+#include "../Trace.h"
#include "Index.h"
#include "dex/DexIndex.h"
#include "llvm/ADT/Optional.h"
@@ -182,17 +183,24 @@
return OS.str();
}
-std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+std::unique_ptr loadIndex(llvm::StringRef SymbolFilename,
bool UseDex) {
- auto Buffer = llvm::MemoryBuffer::getFile(SymbolFile);
+ trace::Span OverallTracer("LoadIndex");
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFilename);
if (!Buffer) {
-llvm::errs() << "Can't open " << SymbolFile << "\n";
+llvm::errs() << "Can't open " << SymbolFilename << "\n";
return nullptr;
}
- auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
+ StringRef Data = Buffer.get()->getBuffer();
+ llvm::Optional SymbolSlab;
+ {
+trace::Span Tracer("ParseYAML");
+auto Slab = symbolsFromYAML(Data);
+ }
- return UseDex ? dex::DexIndex::build(std::move(Slab))
-: MemIndex::build(std::move(Slab), RefSlab());
+ trace::Span Tracer("BuildIndex");
+ return UseDex ? dex::DexIndex::build(std::move(*SymbolSlab))
+: MemIndex::build(std::move(*SymbolSlab), RefSlab());
}
} // namespace clangd
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -280,9 +280,15 @@
Opts.BuildDynamicSymbolIndex = EnableIndex;
std::unique_ptr StaticIdx;
if (EnableIndex && !YamlSymbolFile.empty()) {
-StaticIdx = loadIndex(YamlSymbolFile, UseDex);
-Opts.StaticIndex = StaticIdx.get();
+// Load the index asynchronously. Meanwhile SwapIndex returns no results.
+SwapIndex *Placeholder;
+StaticIdx.reset(Placeholder = new SwapIndex(llvm::make_unique()));
+runAsync([Placeholder] {
+ if (auto Idx = loadIndex(YamlSymbolFile))
+Placeholder->reset(std::move(Idx));
+});
}
+ Opts.StaticIndex = StaticIdx.get();
Opts.AsyncThreadsCount = WorkerThreadsCount;
clangd::CodeCompleteOptions CCOpts;
Index: clangd/index/SymbolYAML.h
===
--- clangd/index/SymbolYAML.h
+++ clangd/index/SymbolYAML.h
@@ -44,7 +44,7 @@
// Build an in-memory static index for global symbols from a symbol file.
// The size of global symbols should be relatively small, so that all symbols
// can be managed in memory.
-std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+std::unique_ptr loadIndex(llvm::StringRef SymbolFilename,
bool UseDex = true);
} // namespace clangd
Index: clangd/index/SymbolYAML.cpp
===
--- clangd/index/SymbolYAML.cpp
+++ clangd/index/SymbolYAML.cpp
@@ -8,6 +8,7 @@
[PATCH] D50147: clang-format: support external styles
Typz added a comment. clang-format does indeed support .clang-format, which is great for *isolated* projects, and which is not affected by this patch. This patch addresses the issue of *centralizing* the definition of styles, e.g. allowing individual projects to reference externally defined styles. This is useful when deploying a coding styles compagny-wide, to avoid copying the configuration in each project (and later having problem issues to maintain the styles or check if the correct style is indeed used). Another way of seeing it is that it allows extending the styles which are hard-coded in clang-format (llvm, google, ...) Repository: rC Clang https://reviews.llvm.org/D50147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51636: [clangd] NFC: Change quality type to float
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341374: [clangd] NFC: Change quality type to float
(authored by omtcyfz, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51636?vs=163833=163837#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51636
Files:
clangd/index/Index.cpp
clangd/index/Index.h
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -59,7 +59,7 @@
return OS << S.Scope << S.Name;
}
-double quality(const Symbol ) {
+float quality(const Symbol ) {
// This avoids a sharp gradient for tail symbols, and also neatly avoids the
// question of whether 0 references means a bad symbol or missing data.
if (S.References < 3)
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -250,7 +250,7 @@
// This currently falls in the range [1, ln(#indexed documents)].
// FIXME: this should probably be split into symbol -> signals
//and signals -> score, so it can be reused for Sema completions.
-double quality(const Symbol );
+float quality(const Symbol );
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -59,7 +59,7 @@
return OS << S.Scope << S.Name;
}
-double quality(const Symbol ) {
+float quality(const Symbol ) {
// This avoids a sharp gradient for tail symbols, and also neatly avoids the
// question of whether 0 references means a bad symbol or missing data.
if (S.References < 3)
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -250,7 +250,7 @@
// This currently falls in the range [1, ln(#indexed documents)].
// FIXME: this should probably be split into symbol -> signals
//and signals -> score, so it can be reused for Sema completions.
-double quality(const Symbol );
+float quality(const Symbol );
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r341374 - [clangd] NFC: Change quality type to float
Author: omtcyfz
Date: Tue Sep 4 08:45:56 2018
New Revision: 341374
URL: http://llvm.org/viewvc/llvm-project?rev=341374=rev
Log:
[clangd] NFC: Change quality type to float
Reviewed by: sammccall
Differential Revision: https://reviews.llvm.org/D51636
Modified:
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
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=341374=341373=341374=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Tue Sep 4 08:45:56 2018
@@ -59,7 +59,7 @@ raw_ostream <<(raw_ostream ,
return OS << S.Scope << S.Name;
}
-double quality(const Symbol ) {
+float quality(const Symbol ) {
// This avoids a sharp gradient for tail symbols, and also neatly avoids the
// question of whether 0 references means a bad symbol or missing data.
if (S.References < 3)
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=341374=341373=341374=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Tue Sep 4 08:45:56 2018
@@ -250,7 +250,7 @@ llvm::raw_ostream <<(llvm::raw_
// This currently falls in the range [1, ln(#indexed documents)].
// FIXME: this should probably be split into symbol -> signals
//and signals -> score, so it can be reused for Sema completions.
-double quality(const Symbol );
+float quality(const Symbol );
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51585 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51636: [clangd] NFC: Change quality type to float
kbobyrev added a comment. In https://reviews.llvm.org/D51636#1223204, @sammccall wrote: > If it's not too expensive, we can use the symbol quality metrics (from > Quality.h) to do a more accurate prescore. Worth a benchmark :-) Yes, I agree. We can investigate that quality/performance trade-off when we have more tools to simplify the process :) https://reviews.llvm.org/D51636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
sammccall added a comment. In https://reviews.llvm.org/D51036#1223230, @melver wrote: > Awaiting remaining reviewer acceptance. > > FYI: I do not have commit commit access -- what is the procedure to commit > once diff is accepted? > > Many thanks! Anyone with commit access can land it for you - I'm happy to do this. @owenpan any concerns? Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.
sammccall updated this revision to Diff 163836.
sammccall marked an inline comment as done.
sammccall added a comment.
address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51585
Files:
clangd/CMakeLists.txt
clangd/RIFF.cpp
clangd/RIFF.h
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Serialization.cpp
clangd/index/Serialization.h
clangd/tool/ClangdMain.cpp
unittests/clangd/CMakeLists.txt
unittests/clangd/RIFFTests.cpp
unittests/clangd/SerializationTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -45,7 +45,6 @@
MATCHER_P(Labeled, Label, "") {
return (arg.Name + arg.Signature).str() == Label;
}
-MATCHER(HasReturnType, "") { return !arg.ReturnType.empty(); }
MATCHER_P(ReturnType, D, "") { return arg.ReturnType == D; }
MATCHER_P(Doc, D, "") { return arg.Documentation == D; }
MATCHER_P(Snippet, S, "") {
@@ -746,84 +745,6 @@
Snippet("ff(${1:int x}, ${2:double y})";
}
-TEST_F(SymbolCollectorTest, YAMLConversions) {
- const std::string YAML1 = R"(
-ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
-Name: 'Foo1'
-Scope: 'clang::'
-SymInfo:
- Kind:Function
- Lang:Cpp
-CanonicalDeclaration:
- FileURI:file:///path/foo.h
- Start:
-Line: 1
-Column: 0
- End:
-Line: 1
-Column: 1
-IsIndexedForCodeCompletion:true
-Documentation:'Foo doc'
-ReturnType:'int'
-IncludeHeaders:
- - Header:'include1'
-References:7
- - Header:'include2'
-References:3
-...
-)";
- const std::string YAML2 = R"(
-ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF858
-Name: 'Foo2'
-Scope: 'clang::'
-SymInfo:
- Kind:Function
- Lang:Cpp
-CanonicalDeclaration:
- FileURI:file:///path/bar.h
- Start:
-Line: 1
-Column: 0
- End:
-Line: 1
-Column: 1
-IsIndexedForCodeCompletion:false
-Signature:'-sig'
-CompletionSnippetSuffix:'-snippet'
-...
-)";
-
- auto Symbols1 = symbolsFromYAML(YAML1);
-
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(AllOf(QName("clang::Foo1"), Labeled("Foo1"),
- Doc("Foo doc"), ReturnType("int"),
- DeclURI("file:///path/foo.h"),
- ForCodeCompletion(true;
- auto = *Symbols1.begin();
- EXPECT_THAT(Sym1.IncludeHeaders,
- UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u),
- IncludeHeaderWithRef("include2", 3u)));
- auto Symbols2 = symbolsFromYAML(YAML2);
- EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(
-QName("clang::Foo2"), Labeled("Foo2-sig"),
-Not(HasReturnType()), DeclURI("file:///path/bar.h"),
-ForCodeCompletion(false;
-
- std::string ConcatenatedYAML;
- {
-llvm::raw_string_ostream OS(ConcatenatedYAML);
-SymbolsToYAML(Symbols1, OS);
-SymbolsToYAML(Symbols2, OS);
- }
- auto ConcatenatedSymbols = symbolsFromYAML(ConcatenatedYAML);
- EXPECT_THAT(ConcatenatedSymbols,
- UnorderedElementsAre(QName("clang::Foo1"),
- QName("clang::Foo2")));
-}
-
TEST_F(SymbolCollectorTest, IncludeHeaderSameAsFileURI) {
CollectorOpts.CollectIncludePath = true;
runSymbolCollector("class Foo {};", /*Main=*/"");
Index: unittests/clangd/SerializationTests.cpp
===
--- /dev/null
+++ unittests/clangd/SerializationTests.cpp
@@ -0,0 +1,138 @@
+//===-- SerializationTests.cpp - Binary and YAML serialization unit tests -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "index/Serialization.h"
+#include "index/SymbolYAML.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
+namespace clang {
+namespace clangd {
+namespace {
+
+const char *YAML1 = R"(
+---
+ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856
+Name: 'Foo1'
+Scope: 'clang::'
+SymInfo:
+ Kind:Function
+ Lang:Cpp
+CanonicalDeclaration:
+ FileURI:file:///path/foo.h
+ Start:
+Line: 1
+Column: 0
+ End:
+Line: 1
+Column: 1
+IsIndexedForCodeCompletion:true
+Documentation:'Foo doc'
+ReturnType:'int'
+IncludeHeaders:
+ - Header:'include1'
+
[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.
sammccall marked 2 inline comments as done.
sammccall added inline comments.
Comment at: clangd/index/Serialization.cpp:50
+
+void writeVar(uint32_t I, raw_ostream ) {
+ constexpr static uint8_t More = 1 << 7;
ioeric wrote:
> This function could use a comment. What's the difference between this and
> `write32`?
Added comment describing varint encoding.
Comment at: clangd/index/Serialization.cpp:96
+ std::vector Sorted;
+ DenseMap, unsigned> Index;
+
ioeric wrote:
> Any reason to use `std::pair` instead of `StringRef`?
It's a performance hack: DenseMap does a lookup by content which
requires hashing the string. We intern the strings as we gather them so there's
no need to hash the string twice. Added a comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51635: clang-cl: Pass /Brepro to linker if it was passed to the compiler
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. I suppose the alternative would be to make /Brepro a non-alias flag, expand it to -mno-incremental-linker-compatible for the cc1 invocation and look for it for the linker invocation. But this is fine too. https://reviews.llvm.org/D51635 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r341373 - Fix the -print-multi-directory flag to print the selected multilib.
Author: chrib
Date: Tue Sep 4 08:22:13 2018
New Revision: 341373
URL: http://llvm.org/viewvc/llvm-project?rev=341373=rev
Log:
Fix the -print-multi-directory flag to print the selected multilib.
Summary: Fix -print-multi-directory to print the selected multilib
Reviewers: jroelofs
Reviewed By: jroelofs
Subscribers: srhines, cfe-commits
Differential Revision: https://reviews.llvm.org/D51354
Added:
cfe/trunk/test/Driver/print-multi-directory.c
Modified:
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Linux.cpp
Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=341373=341372=341373=diff
==
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Tue Sep 4 08:22:13 2018
@@ -149,6 +149,7 @@ private:
protected:
MultilibSet Multilibs;
+ Multilib SelectedMultilib;
ToolChain(const Driver , const llvm::Triple ,
const llvm::opt::ArgList );
@@ -227,6 +228,8 @@ public:
const MultilibSet () const { return Multilibs; }
+ const Multilib () const { return SelectedMultilib; }
+
const SanitizerArgs& getSanitizerArgs() const;
const XRayArgs& getXRayArgs() const;
Modified: cfe/trunk/lib/Driver/Driver.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=341373=341372=341373=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Tue Sep 4 08:22:13 2018
@@ -1661,14 +1661,13 @@ bool Driver::HandleImmediateArgs(const C
}
if (C.getArgs().hasArg(options::OPT_print_multi_directory)) {
-for (const Multilib : TC.getMultilibs()) {
- if (Multilib.gccSuffix().empty())
-llvm::outs() << ".\n";
- else {
-StringRef Suffix(Multilib.gccSuffix());
-assert(Suffix.front() == '/');
-llvm::outs() << Suffix.substr(1) << "\n";
- }
+const Multilib = TC.getMultilib();
+if (Multilib.gccSuffix().empty())
+ llvm::outs() << ".\n";
+else {
+ StringRef Suffix(Multilib.gccSuffix());
+ assert(Suffix.front() == '/');
+ llvm::outs() << Suffix.substr(1) << "\n";
}
return false;
}
Modified: cfe/trunk/lib/Driver/ToolChains/Linux.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Linux.cpp?rev=341373=341372=341373=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Linux.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Linux.cpp Tue Sep 4 08:22:13 2018
@@ -210,6 +210,7 @@ Linux::Linux(const Driver , const llvm
: Generic_ELF(D, Triple, Args) {
GCCInstallation.init(Triple, Args);
Multilibs = GCCInstallation.getMultilibs();
+ SelectedMultilib = GCCInstallation.getMultilib();
llvm::Triple::ArchType Arch = Triple.getArch();
std::string SysRoot = computeSysRoot();
@@ -299,16 +300,14 @@ Linux::Linux(const Driver , const llvm
if (GCCInstallation.isValid()) {
const llvm::Triple = GCCInstallation.getTriple();
const std::string = GCCInstallation.getParentLibPath();
-const Multilib = GCCInstallation.getMultilib();
-const MultilibSet = GCCInstallation.getMultilibs();
// Add toolchain / multilib specific file paths.
-addMultilibsFilePaths(D, Multilibs, Multilib,
+addMultilibsFilePaths(D, Multilibs, SelectedMultilib,
GCCInstallation.getInstallPath(), Paths);
// Sourcery CodeBench MIPS toolchain holds some libraries under
// a biarch-like suffix of the GCC installation.
-addPathIfExists(D, GCCInstallation.getInstallPath() + Multilib.gccSuffix(),
+addPathIfExists(D, GCCInstallation.getInstallPath() +
SelectedMultilib.gccSuffix(),
Paths);
// GCC cross compiling toolchains will install target libraries which ship
@@ -330,7 +329,7 @@ Linux::Linux(const Driver , const llvm
// Note that this matches the GCC behavior. See the below comment for where
// Clang diverges from GCC's behavior.
addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib/../" +
- OSLibDir + Multilib.osSuffix(),
+ OSLibDir + SelectedMultilib.osSuffix(),
Paths);
// If the GCC installation we found is inside of the sysroot, we want to
Added: cfe/trunk/test/Driver/print-multi-directory.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/print-multi-directory.c?rev=341373=auto
==
--- cfe/trunk/test/Driver/print-multi-directory.c (added)
+++ cfe/trunk/test/Driver/print-multi-directory.c Tue Sep 4 08:22:13 2018
@@ -0,0 +1,28 @@
[PATCH] D51576: Enable DWARF accelerator tables by default when tuning for lldb (-glldb => -gpubnames)
aprantl added a comment. This is DWARF5+ only, right? (We shouldn't change the preference of Apple accelerator tables for DWARF 4 and earlier). Repository: rC Clang https://reviews.llvm.org/D51576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51036: clang-format: Fix formatting C++ namespaces with preceding 'inline' or 'export' specifier
melver added a comment. Awaiting remaining reviewer acceptance. FYI: I do not have commit commit access -- what is the procedure to commit once diff is accepted? Many thanks! Repository: rC Clang https://reviews.llvm.org/D51036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50958: [clangd] Implement findReferences function
sammccall added a comment. Oops, and I rebased on head (Occurrences -> Refs) of course. @ilya-biryukov @ioeric, any interest in taking over review here? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50958 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.
ioeric added a comment.
Super cool! Just a few nits.
Comment at: clangd/RIFF.cpp:58
+ if (RIFF->ID != fourCC("RIFF"))
+return makeError("Extra content after RIFF chunk");
+ if (RIFF->Data.size() < 4)
The error message seems wrong?
Comment at: clangd/index/Serialization.cpp:50
+
+void writeVar(uint32_t I, raw_ostream ) {
+ constexpr static uint8_t More = 1 << 7;
This function could use a comment. What's the difference between this and
`write32`?
Comment at: clangd/index/Serialization.cpp:96
+ std::vector Sorted;
+ DenseMap, unsigned> Index;
+
Any reason to use `std::pair` instead of `StringRef`?
Comment at: clangd/index/Serialization.cpp:335
+ std::vector Symbols;
+ for (const auto : *Data.Symbols) {
+Symbols.emplace_back(Sym);
`assert(Data.Symbols)`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51354: Fix the -print-multi-directory flag to print the selected multilib.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341373: Fix the -print-multi-directory flag to print the
selected multilib. (authored by chrib, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51354?vs=162846=163835#toc
Repository:
rC Clang
https://reviews.llvm.org/D51354
Files:
include/clang/Driver/ToolChain.h
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Linux.cpp
test/Driver/print-multi-directory.c
Index: test/Driver/print-multi-directory.c
===
--- test/Driver/print-multi-directory.c
+++ test/Driver/print-multi-directory.c
@@ -0,0 +1,28 @@
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-none-linux \
+// RUN: -print-multi-directory \
+// RUN: | FileCheck --check-prefix=CHECK-X86-MULTILIBS %s
+
+// CHECK-X86-MULTILIBS: 32
+// CHECK-X86-MULTILIBS-NOT: x32
+// CHECK-X86-MULTILIBS-NOT: .
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target i386-none-linux -m64 \
+// RUN: -print-multi-directory \
+// RUN: | FileCheck --check-prefix=CHECK-X86_64-MULTILIBS %s
+
+// CHECK-X86_64-MULTILIBS: .
+// CHECK-X86_64-MULTILIBS-NOT: x32
+// CHECK-X86_64-MULTILIBS-NOT: 32
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: -target arm-linux-androideabi21 -stdlib=libstdc++ \
+// RUN: -mthumb \
+// RUN: -B%S/Inputs/basic_android_ndk_tree \
+// RUN: --sysroot=%S/Inputs/basic_android_ndk_tree/sysroot \
+// RUN: -print-multi-directory \
+// RUN: | FileCheck --check-prefix=CHECK-ARM-MULTILIBS %s
+
+// CHECK-ARM-MULTILIBS: thumb
+// CHECK-ARM-MULTILIBS-NOT: .
Index: lib/Driver/ToolChains/Linux.cpp
===
--- lib/Driver/ToolChains/Linux.cpp
+++ lib/Driver/ToolChains/Linux.cpp
@@ -210,6 +210,7 @@
: Generic_ELF(D, Triple, Args) {
GCCInstallation.init(Triple, Args);
Multilibs = GCCInstallation.getMultilibs();
+ SelectedMultilib = GCCInstallation.getMultilib();
llvm::Triple::ArchType Arch = Triple.getArch();
std::string SysRoot = computeSysRoot();
@@ -299,16 +300,14 @@
if (GCCInstallation.isValid()) {
const llvm::Triple = GCCInstallation.getTriple();
const std::string = GCCInstallation.getParentLibPath();
-const Multilib = GCCInstallation.getMultilib();
-const MultilibSet = GCCInstallation.getMultilibs();
// Add toolchain / multilib specific file paths.
-addMultilibsFilePaths(D, Multilibs, Multilib,
+addMultilibsFilePaths(D, Multilibs, SelectedMultilib,
GCCInstallation.getInstallPath(), Paths);
// Sourcery CodeBench MIPS toolchain holds some libraries under
// a biarch-like suffix of the GCC installation.
-addPathIfExists(D, GCCInstallation.getInstallPath() + Multilib.gccSuffix(),
+addPathIfExists(D, GCCInstallation.getInstallPath() + SelectedMultilib.gccSuffix(),
Paths);
// GCC cross compiling toolchains will install target libraries which ship
@@ -330,7 +329,7 @@
// Note that this matches the GCC behavior. See the below comment for where
// Clang diverges from GCC's behavior.
addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib/../" +
- OSLibDir + Multilib.osSuffix(),
+ OSLibDir + SelectedMultilib.osSuffix(),
Paths);
// If the GCC installation we found is inside of the sysroot, we want to
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1661,14 +1661,13 @@
}
if (C.getArgs().hasArg(options::OPT_print_multi_directory)) {
-for (const Multilib : TC.getMultilibs()) {
- if (Multilib.gccSuffix().empty())
-llvm::outs() << ".\n";
- else {
-StringRef Suffix(Multilib.gccSuffix());
-assert(Suffix.front() == '/');
-llvm::outs() << Suffix.substr(1) << "\n";
- }
+const Multilib = TC.getMultilib();
+if (Multilib.gccSuffix().empty())
+ llvm::outs() << ".\n";
+else {
+ StringRef Suffix(Multilib.gccSuffix());
+ assert(Suffix.front() == '/');
+ llvm::outs() << Suffix.substr(1) << "\n";
}
return false;
}
Index: include/clang/Driver/ToolChain.h
===
--- include/clang/Driver/ToolChain.h
+++ include/clang/Driver/ToolChain.h
@@ -149,6 +149,7 @@
protected:
MultilibSet Multilibs;
+ Multilib SelectedMultilib;
ToolChain(const Driver , const llvm::Triple ,
const llvm::opt::ArgList );
@@ -227,6 +228,8 @@
const MultilibSet () const { return Multilibs; }
+ const Multilib () const { return SelectedMultilib; }
+
const SanitizerArgs& getSanitizerArgs() const;
const XRayArgs&
[PATCH] D50958: [clangd] Implement findReferences function
sammccall updated this revision to Diff 163834.
sammccall added a comment.
Address comments and tighten up highlights/refs reuse in XRefs.cpp a bit.
Still to do:
- test interaction with index, including actual data
- maybe add the ClangdServer/LSP binding and lit test, if it's just boilerplate
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50958
Files:
clangd/XRefs.cpp
clangd/XRefs.h
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1068,6 +1068,81 @@
ElementsAre(Location{FooCppUri, FooWithoutHeader.range()}));
}
+TEST(FindReferences, WithinAST) {
+ const char *Tests[] = {
+ R"cpp(// Local variable
+int main() {
+ int $foo[[foo]];
+ $foo[[^foo]] = 2;
+ int test1 = $foo[[foo]];
+}
+ )cpp",
+
+ R"cpp(// Struct
+namespace ns1 {
+struct $foo[[Foo]] {};
+} // namespace ns1
+int main() {
+ ns1::$foo[[Fo^o]]* Params;
+}
+ )cpp",
+
+ R"cpp(// Function
+int $foo[[foo]](int) {}
+int main() {
+ auto *X = &$foo[[^foo]];
+ $foo[[foo]](42)
+}
+ )cpp",
+
+ R"cpp(// Field
+struct Foo {
+ int $foo[[foo]];
+ Foo() : $foo[[foo]](0) {}
+};
+int main() {
+ Foo f;
+ f.$foo[[f^oo]] = 1;
+}
+ )cpp",
+
+ R"cpp(// Method call
+struct Foo { int [[foo]](); };
+int Foo::[[foo]]() {}
+int main() {
+ Foo f;
+ f.^foo();
+}
+ )cpp",
+
+ R"cpp(// Typedef
+typedef int $foo[[Foo]];
+int main() {
+ $foo[[^Foo]] bar;
+}
+ )cpp",
+
+ R"cpp(// Namespace
+namespace $foo[[ns]] {
+struct Foo {};
+} // namespace ns
+int main() { $foo[[^ns]]::Foo foo; }
+ )cpp",
+ };
+ for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+std::vector> ExpectedLocations;
+for (const auto : T.ranges("foo"))
+ ExpectedLocations.push_back(RangeIs(R));
+EXPECT_THAT(findReferences(AST, T.point()),
+ElementsAreArray(ExpectedLocations))
+<< Test;
+ }
+}
+
+// TODO: add tests that rely on the index, and verify whether it was queried.
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/XRefs.h
===
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -34,6 +34,10 @@
/// Get the hover information when hovering at \p Pos.
llvm::Optional getHover(ParsedAST , Position Pos);
+/// Returns reference locations of the symbol at a specified \p Pos.
+std::vector findReferences(ParsedAST , Position Pos,
+ const SymbolIndex *Index = nullptr);
+
} // namespace clangd
} // namespace clang
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -174,30 +174,27 @@
return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()};
}
-llvm::Optional
-makeLocation(ParsedAST , const SourceRange ) {
+Range getTokenRange(ParsedAST , SourceLocation TokLoc) {
const SourceManager = AST.getASTContext().getSourceManager();
- const LangOptions = AST.getASTContext().getLangOpts();
- SourceLocation LocStart = ValSourceRange.getBegin();
+ SourceLocation LocEnd = Lexer::getLocForEndOfToken(
+ TokLoc, 0, SourceMgr, AST.getASTContext().getLangOpts());
+ return {sourceLocToPosition(SourceMgr, TokLoc),
+ sourceLocToPosition(SourceMgr, LocEnd)};
+}
- const FileEntry *F =
- SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart));
+llvm::Optional makeLocation(ParsedAST , SourceLocation TokLoc) {
+ const SourceManager = AST.getASTContext().getSourceManager();
+ const FileEntry *F = SourceMgr.getFileEntryForID(SourceMgr.getFileID(TokLoc));
if (!F)
return llvm::None;
- SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(), 0,
- SourceMgr, LangOpts);
- Position Begin = sourceLocToPosition(SourceMgr, LocStart);
- Position End = sourceLocToPosition(SourceMgr, LocEnd);
- Range R = {Begin, End};
- Location L;
-
auto FilePath = getRealPath(F, SourceMgr);
if (!FilePath) {
log("failed to get path!");
return llvm::None;
}
+ Location L;
L.uri = URIForFile(*FilePath);
- L.range = R;
+ L.range = getTokenRange(AST, TokLoc);
return L;
}
@@ -223,7 +220,7 @@
for (auto Item : Symbols.Macros) {
auto Loc = Item.Info->getDefinitionLoc();
-auto L = makeLocation(AST, SourceRange(Loc, Loc));
+auto L = makeLocation(AST, Loc);
if (L)
Result.push_back(*L);
}
@@ -266,7
[PATCH] D51636: [clangd] NFC: Change quality type to float
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. If it's not too expensive, we can use the symbol quality metrics (from Quality.h) to do a more accurate prescore. Worth a benchmark :-) https://reviews.llvm.org/D51636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51636: [clangd] NFC: Change quality type to float
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay.
For the sake of consistency, `quality()` should return `float` instead of
`double` since all of the scoring calculation happens to be in `float`s.
https://reviews.llvm.org/D51636
Files:
clang-tools-extra/clangd/index/Index.cpp
clang-tools-extra/clangd/index/Index.h
Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -250,7 +250,7 @@
// This currently falls in the range [1, ln(#indexed documents)].
// FIXME: this should probably be split into symbol -> signals
//and signals -> score, so it can be reused for Sema completions.
-double quality(const Symbol );
+float quality(const Symbol );
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
Index: clang-tools-extra/clangd/index/Index.cpp
===
--- clang-tools-extra/clangd/index/Index.cpp
+++ clang-tools-extra/clangd/index/Index.cpp
@@ -59,7 +59,7 @@
return OS << S.Scope << S.Name;
}
-double quality(const Symbol ) {
+float quality(const Symbol ) {
// This avoids a sharp gradient for tail symbols, and also neatly avoids the
// question of whether 0 references means a bad symbol or missing data.
if (S.References < 3)
Index: clang-tools-extra/clangd/index/Index.h
===
--- clang-tools-extra/clangd/index/Index.h
+++ clang-tools-extra/clangd/index/Index.h
@@ -250,7 +250,7 @@
// This currently falls in the range [1, ln(#indexed documents)].
// FIXME: this should probably be split into symbol -> signals
//and signals -> score, so it can be reused for Sema completions.
-double quality(const Symbol );
+float quality(const Symbol );
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
Index: clang-tools-extra/clangd/index/Index.cpp
===
--- clang-tools-extra/clangd/index/Index.cpp
+++ clang-tools-extra/clangd/index/Index.cpp
@@ -59,7 +59,7 @@
return OS << S.Scope << S.Name;
}
-double quality(const Symbol ) {
+float quality(const Symbol ) {
// This avoids a sharp gradient for tail symbols, and also neatly avoids the
// question of whether 0 references means a bad symbol or missing data.
if (S.References < 3)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51635: clang-cl: Pass /Brepro to linker if it was passed to the compiler
thakis created this revision.
thakis added a reviewer: hans.
/Brepro currently is just an alias for -mno-incremental-linker-compatible and
before this patch only controlled if we write a timestamp into the output obj
file. But cl.exe also passes it on to link.exe (where it controls whether the
final PE image timestamp really is a timestamp or a hash of the output).
It's a bit weird to overload -mno-incremental-linker-compatible to also pass
/Brepro to the linker, but all alternatives are a bit weird too. Open for
suggestions though :-)
https://reviews.llvm.org/D51635
Files:
clang/lib/Driver/ToolChains/MSVC.cpp
clang/test/Driver/msvc-link.c
Index: clang/test/Driver/msvc-link.c
===
--- clang/test/Driver/msvc-link.c
+++ clang/test/Driver/msvc-link.c
@@ -3,6 +3,7 @@
// BASIC: "-out:a.exe"
// BASIC: "-defaultlib:libcmt"
// BASIC: "-nologo"
+// BASIC-NOT: "-Brepro"
// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 |
FileCheck --check-prefix=DLL %s
// DLL: link.exe"
@@ -16,3 +17,14 @@
// LIBPATH: "-libpath:/usr/lib"
// LIBPATH: "-nologo"
+// RUN: %clang_cl /Brepro -### %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// REPRO: link.exe"
+// REPRO: "-out:msvc-link.exe"
+// REPRO: "-nologo"
+// REPRO: "-Brepro"
+
+// RUN: %clang_cl /Brepro- -### %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// NOREPRO: link.exe"
+// NOREPRO: "-out:msvc-link.exe"
+// NOREPRO: "-nologo"
+// NOREPRO-NOT: "-Brepro"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -355,6 +355,15 @@
options::OPT__SLASH_Zd))
CmdArgs.push_back("-debug");
+ // Pass on /Brepro if it was passed to the compiler.
+ // Note that /Brepro maps to -mno-incremental-linker-compatible.
+ bool DefaultIncrementalLinkerCompatible =
+ C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
+ if (!Args.hasFlag(options::OPT_mincremental_linker_compatible,
+options::OPT_mno_incremental_linker_compatible,
+DefaultIncrementalLinkerCompatible))
+CmdArgs.push_back("-Brepro");
+
bool DLL = Args.hasArg(options::OPT__SLASH_LD, options::OPT__SLASH_LDd,
options::OPT_shared);
if (DLL) {
Index: clang/test/Driver/msvc-link.c
===
--- clang/test/Driver/msvc-link.c
+++ clang/test/Driver/msvc-link.c
@@ -3,6 +3,7 @@
// BASIC: "-out:a.exe"
// BASIC: "-defaultlib:libcmt"
// BASIC: "-nologo"
+// BASIC-NOT: "-Brepro"
// RUN: %clang -target i686-pc-windows-msvc -shared -o a.dll -### %s 2>&1 | FileCheck --check-prefix=DLL %s
// DLL: link.exe"
@@ -16,3 +17,14 @@
// LIBPATH: "-libpath:/usr/lib"
// LIBPATH: "-nologo"
+// RUN: %clang_cl /Brepro -### %s 2>&1 | FileCheck --check-prefix=REPRO %s
+// REPRO: link.exe"
+// REPRO: "-out:msvc-link.exe"
+// REPRO: "-nologo"
+// REPRO: "-Brepro"
+
+// RUN: %clang_cl /Brepro- -### %s 2>&1 | FileCheck --check-prefix=NOREPRO %s
+// NOREPRO: link.exe"
+// NOREPRO: "-out:msvc-link.exe"
+// NOREPRO: "-nologo"
+// NOREPRO-NOT: "-Brepro"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -355,6 +355,15 @@
options::OPT__SLASH_Zd))
CmdArgs.push_back("-debug");
+ // Pass on /Brepro if it was passed to the compiler.
+ // Note that /Brepro maps to -mno-incremental-linker-compatible.
+ bool DefaultIncrementalLinkerCompatible =
+ C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment();
+ if (!Args.hasFlag(options::OPT_mincremental_linker_compatible,
+options::OPT_mno_incremental_linker_compatible,
+DefaultIncrementalLinkerCompatible))
+CmdArgs.push_back("-Brepro");
+
bool DLL = Args.hasArg(options::OPT__SLASH_LD, options::OPT__SLASH_LDd,
options::OPT_shared);
if (DLL) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51626: [clangd] Move buildStaticIndex() to SymbolYAML
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341369: [clangd] Move buildStaticIndex() to SymbolYAML
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51626?vs=163823=163827#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51626
Files:
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -39,24 +39,6 @@
enum class PCHStorageFlag { Disk, Memory };
-// Build an in-memory static index for global symbols from a YAML-format file.
-// The size of global symbols should be relatively small, so that all symbols
-// can be managed in memory.
-std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
- auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
- if (!Buffer) {
-llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
-return nullptr;
- }
- auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
- SymbolSlab::Builder SymsBuilder;
- for (auto Sym : Slab)
-SymsBuilder.insert(Sym);
-
- return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-: MemIndex::build(std::move(SymsBuilder).build(), RefSlab());
-}
-
} // namespace
static llvm::cl::opt CompileCommandsDir(
@@ -298,7 +280,7 @@
Opts.BuildDynamicSymbolIndex = EnableIndex;
std::unique_ptr StaticIdx;
if (EnableIndex && !YamlSymbolFile.empty()) {
-StaticIdx = buildStaticIndex(YamlSymbolFile);
+StaticIdx = loadIndex(YamlSymbolFile, UseDex);
Opts.StaticIndex = StaticIdx.get();
}
Opts.AsyncThreadsCount = WorkerThreadsCount;
Index: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
@@ -9,6 +9,7 @@
#include "SymbolYAML.h"
#include "Index.h"
+#include "dex/DexIndex.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Errc.h"
@@ -25,18 +26,18 @@
using clang::clangd::SymbolID;
using clang::clangd::SymbolLocation;
using clang::index::SymbolInfo;
-using clang::index::SymbolLanguage;
using clang::index::SymbolKind;
+using clang::index::SymbolLanguage;
// Helper to (de)serialize the SymbolID. We serialize it as a hex string.
struct NormalizedSymbolID {
NormalizedSymbolID(IO &) {}
- NormalizedSymbolID(IO &, const SymbolID& ID) {
+ NormalizedSymbolID(IO &, const SymbolID ) {
llvm::raw_string_ostream OS(HexString);
OS << ID;
}
- SymbolID denormalize(IO&) {
+ SymbolID denormalize(IO &) {
SymbolID ID;
HexString >> ID;
return ID;
@@ -167,7 +168,7 @@
return S;
}
-void SymbolsToYAML(const SymbolSlab& Symbols, llvm::raw_ostream ) {
+void SymbolsToYAML(const SymbolSlab , llvm::raw_ostream ) {
llvm::yaml::Output Yout(OS);
for (Symbol S : Symbols) // copy: Yout<< requires mutability.
Yout << S;
@@ -181,5 +182,18 @@
return OS.str();
}
+std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+ bool UseDex) {
+ auto Buffer = llvm::MemoryBuffer::getFile(SymbolFile);
+ if (!Buffer) {
+llvm::errs() << "Can't open " << SymbolFile << "\n";
+return nullptr;
+ }
+ auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
+
+ return UseDex ? dex::DexIndex::build(std::move(Slab))
+: MemIndex::build(std::move(Slab), RefSlab());
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/clangd/index/SymbolYAML.h
===
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.h
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.h
@@ -41,6 +41,12 @@
// The YAML result is safe to concatenate if you have multiple symbol slabs.
void SymbolsToYAML(const SymbolSlab , llvm::raw_ostream );
+// Build an in-memory static index for global symbols from a symbol file.
+// The size of global symbols should be relatively small, so that all symbols
+// can be managed in memory.
+std::unique_ptr loadIndex(llvm::StringRef SymbolFile,
+ bool UseDex = true);
+
} // namespace clangd
} // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
