[PATCH] D41039: Add support for attribute "trivial_abi"
ahatanak updated this revision to Diff 129038.
ahatanak added a comment.
Add more tests for template instantiation of "trivial_abi" classes.
https://reviews.llvm.org/D41039
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Decl.h
include/clang/AST/DeclCXX.h
include/clang/AST/Type.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/AST/ASTContext.cpp
lib/AST/DeclCXX.cpp
lib/AST/Type.cpp
lib/CodeGen/CGCall.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaTemplateInstantiate.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
lib/Serialization/ASTWriterDecl.cpp
test/CodeGenCXX/trivial_abi.cpp
test/CodeGenObjCXX/trivial_abi.mm
test/Misc/pragma-attribute-supported-attributes-list.test
test/SemaObjCXX/attr-trivial-abi.mm
Index: test/SemaObjCXX/attr-trivial-abi.mm
===
--- /dev/null
+++ test/SemaObjCXX/attr-trivial-abi.mm
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -std=c++11 -fobjc-runtime-has-weak -fobjc-weak -fobjc-arc -fsyntax-only -verify %s
+
+void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}}
+
+struct [[clang::trivial_abi]] S0 {
+ int a;
+};
+
+struct __attribute__((trivial_abi)) S1 {
+ int a;
+};
+
+struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}}
+ __weak id a;
+};
+
+struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}}
+ virtual void m();
+};
+
+struct S4 {
+ int a;
+};
+
+struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}}
+};
+
+struct __attribute__((trivial_abi)) S9 : public S4 {
+};
+
+struct S6 {
+ __weak id a;
+};
+
+struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}}
+ __weak id a;
+};
+
+struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}}
+ __weak id a[2];
+};
+
+struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}}
+ S6 a;
+};
+
+struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}}
+ S6 a[2];
+};
+
+struct __attribute__((trivial_abi(1))) S8 { // expected-error {{'trivial_abi' attribute takes no arguments}}
+ int a;
+};
+
+// Do not warn when 'trivial_abi' is used to annotate a template class.
+template
+struct __attribute__((trivial_abi)) S10 {
+ T p;
+};
+
+S10 p1;
+S10<__weak id> p2;
+
+template<>
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10'}}
+ __weak id a;
+};
+
+template
+struct S14 {
+ T a;
+ __weak id b;
+};
+
+template
+struct __attribute__((trivial_abi)) S15 : S14 {
+};
+
+S15 s15;
+
+template
+struct __attribute__((trivial_abi)) S16 {
+ S14 a;
+};
+
+S16 s16;
+
+template
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}}
+ __weak id a;
+};
+
+S17 s17;
Index: test/Misc/pragma-attribute-supported-attributes-list.test
===
--- test/Misc/pragma-attribute-supported-attributes-list.test
+++ test/Misc/pragma-attribute-supported-attributes-list.test
@@ -2,7 +2,7 @@
// The number of supported attributes should never go down!
-// CHECK: #pragma clang attribute supports 66 attributes:
+// CHECK: #pragma clang attribute supports 67 attributes:
// CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
@@ -66,6 +66,7 @@
// CHECK-NEXT: TLSModel (SubjectMatchRule_variable_is_thread_local)
// CHECK-NEXT: Target (SubjectMatchRule_function)
// CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member)
+// CHECK-NEXT: TrivialABI (SubjectMatchRule_record)
// CHECK-NEXT: WarnUnusedResult (SubjectMatchRule_objc_method, SubjectMatchRule_enum, SubjectMatchRule_record, SubjectMatchRule_hasType_functionType)
// CHECK-NEXT: XRayInstrument (SubjectMatchRule_function, SubjectMatchRule_objc_method)
// CHECK-NEXT: XRayLogArgs (SubjectMatchRule_function, SubjectMatchRule_objc_method)
Index: test/CodeGenObjCXX/trivial_abi.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/trivial_abi.mm
@@ -0,0 +1,103 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -std=c++11 -fobjc-arc -fobjc-weak -fobjc-runtime-has-weak -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: %[[STRUCT_STRONGWEAK:.*]] = type { i8*, i8* }
+// CHECK:
[PATCH] D41039: Add support for attribute "trivial_abi"
ahatanak marked 7 inline comments as done.
ahatanak added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:11700
+}
+ }
+
rsmith wrote:
> rjmccall wrote:
> > I think it's correct not to call CheckDestructorAccess and
> > DiagnoseUseOfDecl here, since according to the standard destructor access
> > is always supposed to be checked at the call-site, but please leave a
> > comment explaining that.
> The corresponding code for `areArgsDestroyedLeftToRightInCallee` is in
> `SemaChecking`. This should be done in the same place.
>
> More generally, we have at least three different places where we check
> `CXXABI::areArgsDestroyedLeftToRightInCallee() ||
> Type->hasTrivialABIOverride()`. It would make sense to factor that out into a
> `isParamDestroyedInCallee` function (probably on `ASTContext`, since we don't
> have anywhere better for ABI-specific checks at a layering level that can
> talk about types).
I defined function ASTContext::isParamDestroyedInCallee and used it in two
places. I didn't use it in CodeGenFunction::EmitParmDecl because the destructor
cleanup is pushed only when the argument is passed indirectly in MSVC's case,
whereas it is always pushed when the class is HasTrivialABIOverride.
Comment at: lib/Sema/SemaDeclCXX.cpp:7580
+static void checkIllFormedTrivialABIStruct(CXXRecordDecl &RD, Sema &S) {
+ auto PrintDiagAndRemoveAttr = [&]() {
rsmith wrote:
> Either "ill-formed" is the wrong word here, or this needs to produce `ext_`
> diagnostics that `-pedantic-errors` turns into hard errors.
I made the warning derive from ExtWarn.
https://reviews.llvm.org/D41039
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41500: ananas: Add shared library support
ed accepted this revision. ed added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D41500#965157, @zhmu wrote: > @ed I tried to keep things as much in line as the other > ::linker::Construct() functions do. Do you wish to stray from that path? > > I actually prefer to keep it like this as it's quite readable already, but > YMMV of course :-) Agreed. I'll commit this patch later today as is! Repository: rC Clang https://reviews.llvm.org/D41500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41730: [clangd] Use ToolExecutor to write the global-symbol-builder tool.
sammccall added a comment. This makes `SymbolCollector` (production code) depend on YAML (explicitly experimental), as well as on Tooling interfaces. At least we should invert this by making the thing passed to SymbolCollector a `function` or so. Moreover, it's unfortunate we'd need to change SymbolCollector at all. Currently it's a nice abstraction that does one thing (AST -> `SymbolSlab`), now we're making it do two things. It can't be for scalability reasons - there's no problem fitting all the symbols from a TU in memory. So why do we need to do this? I don't really understand the `ToolExecutor` interfaces well, but ISTM overriding `SymbolIndexActionFactory::runInvocation` to flush the symbols in the slab out to the `ExecutionContext` would allow you to use `SymbolCollector` unchanged? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40478: Added control flow architecture protection Flag
This revision was automatically updated to reflect the committed changes.
Closed by commit rC322063: Added Control Flow Protection Flag (authored by
orenb, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D40478?vs=128601&id=129041#toc
Repository:
rC Clang
https://reviews.llvm.org/D40478
Files:
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/TargetInfo.h
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/Basic/Targets/X86.cpp
lib/Basic/Targets/X86.h
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenModule.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/x86-cf-protection.c
test/Driver/clang_f_opts.c
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1042,6 +1042,11 @@
HelpText<"Like -finstrument-functions, but insert the calls after inlining">;
def finstrument_function_entry_bare : Flag<["-"], "finstrument-function-entry-bare">, Group, Flags<[CC1Option]>,
HelpText<"Instrument function entry only, after inlining, without arguments to the instrumentation call">;
+def fcf_protection_EQ : Joined<["-"], "fcf-protection=">, Flags<[CoreOption, CC1Option]>, Group,
+ HelpText<"Instrument control-flow architecture protection. Options: return, branch, full, none.">, Values<"return,branch,full,none">;
+def fcf_protection : Flag<["-"], "fcf-protection">, Group, Flags<[CoreOption, CC1Option]>,
+ Alias, AliasArgs<["full"]>,
+ HelpText<"Enable cf-protection in 'full' mode">;
def fxray_instrument : Flag<["-"], "fxray-instrument">, Group,
Flags<[CC1Option]>,
Index: include/clang/Basic/DiagnosticCommonKinds.td
===
--- include/clang/Basic/DiagnosticCommonKinds.td
+++ include/clang/Basic/DiagnosticCommonKinds.td
@@ -203,6 +203,8 @@
"execute only is not supported for the %0 sub-architecture">;
def err_opt_not_valid_with_opt : Error<
"option '%0' cannot be specified with '%1'">;
+def err_opt_not_valid_without_opt : Error<
+ "option '%0' cannot be specified without '%1'">;
// Source manager
def err_cannot_open_file : Error<"cannot open file '%0': %1">, DefaultFatal;
Index: include/clang/Basic/TargetInfo.h
===
--- include/clang/Basic/TargetInfo.h
+++ include/clang/Basic/TargetInfo.h
@@ -1051,6 +1051,18 @@
return false;
}
+ /// Check if the target supports CFProtection branch.
+ virtual bool
+ checkCFProtectionBranchSupported(DiagnosticsEngine &Diags) const {
+return false;
+ }
+
+ /// Check if the target supports CFProtection branch.
+ virtual bool
+ checkCFProtectionReturnSupported(DiagnosticsEngine &Diags) const {
+return false;
+ }
+
/// \brief Whether target allows to overalign ABI-specified preferred alignment
virtual bool allowsLargerPreferedTypeAlignment() const { return true; }
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -80,7 +80,10 @@
///< -finstrument-functions-after-inlining is enabled.
CODEGENOPT(InstrumentFunctionEntryBare , 1, 0) ///< Set when
///< -finstrument-function-entry-bare is enabled.
-
+CODEGENOPT(CFProtectionReturn , 1, 0) ///< if -fcf-protection is
+ ///< set to full or return.
+CODEGENOPT(CFProtectionBranch , 1, 0) ///< if -fcf-protection is
+ ///< set to full or branch.
CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is
///< enabled.
CODEGENOPT(StackSizeSection , 1, 0) ///< Set when -fstack-size-section is enabled.
Index: test/CodeGen/x86-cf-protection.c
===
--- test/CodeGen/x86-cf-protection.c
+++ test/CodeGen/x86-cf-protection.c
@@ -0,0 +1,6 @@
+// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN
+// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH
+
+// RETURN: error: option 'cf-protection=return' cannot be specified without '-mshstk'
+// BRANCH: error: option 'cf-protection=branch' cannot be specified without '-mibt'
+void foo() {}
Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -503,3 +503,17 @@
// CHECK-WINDOWS-ISO10646: "-fwchar-type=int"
// CHECK-WINDOWS-ISO10646: "-fsigned-wchar"
+// RUN: %clang -### -S -fcf-
r322063 - Added Control Flow Protection Flag
Author: orenb
Date: Tue Jan 9 00:53:59 2018
New Revision: 322063
URL: http://llvm.org/viewvc/llvm-project?rev=322063&view=rev
Log:
Added Control Flow Protection Flag
Cf-protection is a target independent flag that instructs the back-end to
instrument control flow mechanisms like: Branch, Return, etc.
For example in X86 this flag will be used to instrument Indirect Branch
Tracking instructions.
Differential Revision: https://reviews.llvm.org/D40478
Change-Id: I5126e766c0e6b84118cae0ee8a20fe78cc373dea
Added:
cfe/trunk/test/CodeGen/x86-cf-protection.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/clang_f_opts.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td?rev=322063&r1=322062&r2=322063&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td Tue Jan 9 00:53:59
2018
@@ -203,6 +203,8 @@ def err_target_unsupported_execute_only
"execute only is not supported for the %0 sub-architecture">;
def err_opt_not_valid_with_opt : Error<
"option '%0' cannot be specified with '%1'">;
+def err_opt_not_valid_without_opt : Error<
+ "option '%0' cannot be specified without '%1'">;
// Source manager
def err_cannot_open_file : Error<"cannot open file '%0': %1">, DefaultFatal;
Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=322063&r1=322062&r2=322063&view=diff
==
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Tue Jan 9 00:53:59 2018
@@ -1051,6 +1051,18 @@ public:
return false;
}
+ /// Check if the target supports CFProtection branch.
+ virtual bool
+ checkCFProtectionBranchSupported(DiagnosticsEngine &Diags) const {
+return false;
+ }
+
+ /// Check if the target supports CFProtection branch.
+ virtual bool
+ checkCFProtectionReturnSupported(DiagnosticsEngine &Diags) const {
+return false;
+ }
+
/// \brief Whether target allows to overalign ABI-specified preferred
alignment
virtual bool allowsLargerPreferedTypeAlignment() const { return true; }
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=322063&r1=322062&r2=322063&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Jan 9 00:53:59 2018
@@ -1042,6 +1042,11 @@ def finstrument_functions_after_inlining
HelpText<"Like -finstrument-functions, but insert the calls after inlining">;
def finstrument_function_entry_bare : Flag<["-"],
"finstrument-function-entry-bare">, Group, Flags<[CC1Option]>,
HelpText<"Instrument function entry only, after inlining, without arguments
to the instrumentation call">;
+def fcf_protection_EQ : Joined<["-"], "fcf-protection=">, Flags<[CoreOption,
CC1Option]>, Group,
+ HelpText<"Instrument control-flow architecture protection. Options: return,
branch, full, none.">, Values<"return,branch,full,none">;
+def fcf_protection : Flag<["-"], "fcf-protection">, Group,
Flags<[CoreOption, CC1Option]>,
+ Alias, AliasArgs<["full"]>,
+ HelpText<"Enable cf-protection in 'full' mode">;
def fxray_instrument : Flag<["-"], "fxray-instrument">, Group,
Flags<[CC1Option]>,
Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=322063&r1=322062&r2=322063&view=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Tue Jan 9 00:53:59 2018
@@ -80,7 +80,10 @@ CODEGENOPT(InstrumentFunctionsAfterInlin
///< -finstrument-functions-after-inlining is
enabled.
CODEGENOPT(InstrumentFunctionEntryBare , 1, 0) ///< Set when
///< -finstrument-function-entry-bare is
enabled.
-
+CODEGENOPT(CFProtectionReturn , 1, 0) ///< if -fcf-protection is
+ ///< set to full or return.
+CODEGENOP
[PATCH] D41852: [clang-tidy] Don't generate fix for argument constructed from std::initializer_list.
hokein created this revision.
hokein added reviewers: ilya-biryukov, alexfh.
Herald added subscribers: xazax.hun, klimek.
A follow-up fix of https://reviews.llvm.org/rL311652.
The previous `vector` in our test is different with `std::vector`, so
The check still generates fixes for std::vector (`auto p =
std::unique_ptr(new Foo({1,2,3}))`) in real world, the patch makes the
vector behavior in test align with std::vector (both AST nodes are the same
now).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41852
Files:
clang-tidy/modernize/MakeSmartPtrCheck.cpp
test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
test/clang-tidy/modernize-make-unique.cpp
Index: test/clang-tidy/modernize-make-unique.cpp
===
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -25,6 +25,11 @@
int a, b;
};
+template
+struct MyVector {
+ MyVector(std::initializer_list);
+};
+
struct Empty {};
struct E {
@@ -45,6 +50,7 @@
struct H {
H(std::vector);
+ H(MyVector, int);
};
struct I {
@@ -331,6 +337,13 @@
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
// CHECK-FIXES: PH1.reset(new H({1, 2, 3}));
+ std::unique_ptr PH2 = std::unique_ptr(new H({1, 2, 3}, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr PH2 = std::unique_ptr(new H({1, 2, 3},
1));
+ PH2.reset(new H({1, 2, 3}, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PH2.reset(new H({1, 2, 3}, 1));
+
std::unique_ptr PI1 = std::unique_ptr(new I(G({1, 2, 3})));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
// CHECK-FIXES: std::unique_ptr PI1 = std::make_unique(G({1, 2, 3}));
Index: test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
===
--- test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
+++ test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
@@ -27,5 +27,6 @@
class vector {
public:
vector(initializer_list<_E> init);
+ ~vector();
};
} // namespace std
Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -281,12 +281,25 @@
if (isa(Arg)) {
return false;
}
+// Check whether we construct a class from a std::initializer_list.
+// If so, we won't generate the fixes.
+auto IsStdInitListInitConstructExpr = [](const Expr* E) {
+ assert(E);
+ if (const auto *ImplicitCE = dyn_cast(E)) {
+if (ImplicitCE->isStdInitListInitialization())
+ return true;
+ }
+ return false;
+};
// Check the implicit conversion from the std::initializer_list type to
// a class type.
-if (const auto *ImplicitCE = dyn_cast(Arg)) {
- if (ImplicitCE->isStdInitListInitialization()) {
+if (IsStdInitListInitConstructExpr(Arg))
+ return false;
+// The Arg can be a CXXBindTemporaryExpr, checking its underlying
+// construct expr.
+if (const auto * CTE = dyn_cast(Arg)) {
+ if (IsStdInitListInitConstructExpr(CTE->getSubExpr()))
return false;
- }
}
}
}
Index: test/clang-tidy/modernize-make-unique.cpp
===
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -25,6 +25,11 @@
int a, b;
};
+template
+struct MyVector {
+ MyVector(std::initializer_list);
+};
+
struct Empty {};
struct E {
@@ -45,6 +50,7 @@
struct H {
H(std::vector);
+ H(MyVector, int);
};
struct I {
@@ -331,6 +337,13 @@
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
// CHECK-FIXES: PH1.reset(new H({1, 2, 3}));
+ std::unique_ptr PH2 = std::unique_ptr(new H({1, 2, 3}, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
+ // CHECK-FIXES: std::unique_ptr PH2 = std::unique_ptr(new H({1, 2, 3}, 1));
+ PH2.reset(new H({1, 2, 3}, 1));
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::make_unique instead
+ // CHECK-FIXES: PH2.reset(new H({1, 2, 3}, 1));
+
std::unique_ptr PI1 = std::unique_ptr(new I(G({1, 2, 3})));
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use std::make_unique instead
// CHECK-FIXES: std::unique_ptr PI1 = std::make_unique(G({1, 2, 3}));
Index: test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
===
--- test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
+++ test/clang-tidy/Inputs/modernize-smart-ptr/initializer
[PATCH] D41345: [clangd] Add more symbol information for code completion.
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Just some nits
Comment at: clangd/index/Index.h:122
+
+ llvm::Optional Detail;
+
ioeric wrote:
> sammccall wrote:
> > ioeric wrote:
> > > sammccall wrote:
> > > > I think you probably want a raw pointer rather than optional:
> > > > - reduce the size of the struct when it's absent
> > > > - make it inheritance-friendly so we can hang index-specific info off
> > > > it
> > > > (raw pointer rather than unique_ptr because it's owned by a slab not by
> > > > malloc, but unique_ptr is ok for now)
> > > >
> > > This is not easy for now with `unique_ptr` because of this line :(
> > > https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141).
> > >
> > >
> > > This shouldn't be an issue when we have the optimized symbol slab, where
> > > we store raw pointers. And we would probably want to serialize the whole
> > > slab instead of the individual symbols anyway.
> > >
> > > > reduce the size of the struct when it's absent
> > > `llvm::Optional` doesn't take much more space, so the size should be fine.
> > >
> > > > make it inheritance-friendly so we can hang index-specific info off it
> > > Could you elaborate on `index-specific info`? It's unclear to me how this
> > > would be used.
> > > This is not easy for now with unique_ptr because of this line
> > Oh no, somehow i missed this during review.
> > We shouldn't be relying on symbols being copyable. I'll try to work out how
> > to fix this and delete the copy constructor.
> >
> > > This shouldn't be an issue when we have the optimized symbol slab, where
> > > we store raw pointers.
> > Sure. That's not a big monolithic/mysterous thing though, storing the
> > details in the slab can be done in this patch... If you think it'll be
> > easier once strings are arena-based, then maybe we should delay this patch
> > until that's done, rather than make that work bigger.
> >
> > > And we would probably want to serialize the whole slab instead of the
> > > individual symbols anyway.
> > This i'm less sure about, but I don't think it matters.
> >
> > > llvm::Optional doesn't take much more space, so the size should be fine.
> > Optional takes the same size as the details itself (plus one bool). This is
> > fairly small for now, but I think a major point of Details is to expand it
> > in the future?
> >
> > > Could you elaborate on index-specific info? It's unclear to me how this
> > > would be used.
> > Yeah, this is something we talked about in the meeting with Marc-Andre but
> > it's not really obvious - what's the point of allowing Details to be
> > extended if clangd has to consume it?
> >
> > It sounded like he might have use cases for using index infrastructure
> > outside clangd. We might also have google-internal index features we want
> > (matching generated code to proto fields?). I'm not really sure how
> > compelling this argument is.
> Thanks for the allocator change!
>
> `Details` now contains just stringrefs. I wonder how much we want it to be
> inherit-friendly at this point, as the size is relatively small now. If you
> think this is a better way to go, I'll make the structure contain strings and
> store the whole structure in arena. This would require some tweaks for yamls
> tho but shouldn't be hard.
So this needs to be at least optional I think - at the moment, how does an API
user know whether to fetch the rest of the details?
FWIW, the size difference is already significant: symbol is 136 bytes if this
is a unique_ptr, and 164 bytes if it's optional (+20%) - StringRefs are still
pretty big.
But I don't feel really strongly about this.
Comment at: clangd/index/SymbolCollector.cpp:71
+SymbolCollector::SymbolCollector()
+: CompletionAllocator(std::make_shared()),
+ CompletionTUInfo(CompletionAllocator) {}
you shouldn't initialize these both here and in `initialize()`, right?
Comment at: clangd/index/SymbolCollector.cpp:140
+S.CompletionPlainInsertText = PlainInsertText;
+if (PlainInsertText != SnippetInsertText)
+ S.CompletionSnippetInsertText = SnippetInsertText;
why conditional? note we're interning the strings anyway, and an empty
stringref is the same size as a full one
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322064 - ananas: Add shared library support
Author: ed
Date: Tue Jan 9 01:18:14 2018
New Revision: 322064
URL: http://llvm.org/viewvc/llvm-project?rev=322064&view=rev
Log:
ananas: Add shared library support
The Ananas Operating System (https://github.com/zhmu/ananas) has shared
library support as of commit 57739c0b6ece56dd4872aedf30264ed4b9412c77.
This change adds the necessary settings to clang so that shared
executables and libraries can be build correctly.
Submitted by: Rink Springer
Differential Revision: https://reviews.llvm.org/D41500
Modified:
cfe/trunk/lib/Driver/ToolChains/Ananas.cpp
cfe/trunk/test/Driver/ananas.c
Modified: cfe/trunk/lib/Driver/ToolChains/Ananas.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Ananas.cpp?rev=322064&r1=322063&r2=322064&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Ananas.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Ananas.cpp Tue Jan 9 01:18:14 2018
@@ -64,8 +64,19 @@ void ananas::Linker::ConstructJob(Compil
if (!D.SysRoot.empty())
CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
- // Ananas only supports static linkage for now.
- CmdArgs.push_back("-Bstatic");
+ if (Args.hasArg(options::OPT_static)) {
+CmdArgs.push_back("-Bstatic");
+ } else {
+if (Args.hasArg(options::OPT_rdynamic))
+ CmdArgs.push_back("-export-dynamic");
+if (Args.hasArg(options::OPT_shared)) {
+ CmdArgs.push_back("-Bshareable");
+} else {
+ Args.AddAllArgs(CmdArgs, options::OPT_pie);
+ CmdArgs.push_back("-dynamic-linker");
+ CmdArgs.push_back("/lib/ld-ananas.so");
+}
+ }
if (Output.isFilename()) {
CmdArgs.push_back("-o");
@@ -75,9 +86,15 @@ void ananas::Linker::ConstructJob(Compil
}
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+if (!Args.hasArg(options::OPT_shared)) {
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+}
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crti.o")));
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbegin.o")));
+if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie)) {
+
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbeginS.o")));
+} else {
+
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbegin.o")));
+}
}
Args.AddAllArgs(CmdArgs, options::OPT_L);
@@ -97,7 +114,10 @@ void ananas::Linker::ConstructJob(Compil
CmdArgs.push_back("-lc");
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtend.o")));
+if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
+
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
+else
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtend.o")));
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
}
Modified: cfe/trunk/test/Driver/ananas.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ananas.c?rev=322064&r1=322063&r2=322064&view=diff
==
--- cfe/trunk/test/Driver/ananas.c (original)
+++ cfe/trunk/test/Driver/ananas.c Tue Jan 9 01:18:14 2018
@@ -7,3 +7,11 @@
// CHECK-STATIC: crtbegin.o
// CHECK-STATIC: crtend.o
// CHECK-STATIC: crtn.o
+
+// RUN: %clang -no-canonical-prefixes -target x86_64-unknown-ananas -shared %s
\
+// RUN: --sysroot=%S/Inputs/ananas-tree -### 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-SHARED %s
+// CHECK-SHARED: crti.o
+// CHECK-SHARED: crtbeginS.o
+// CHECK-SHARED: crtendS.o
+// CHECK-SHARED: crtn.o
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41500: ananas: Add shared library support
This revision was automatically updated to reflect the committed changes.
Closed by commit rC322064: ananas: Add shared library support (authored by ed,
committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41500?vs=127907&id=129045#toc
Repository:
rC Clang
https://reviews.llvm.org/D41500
Files:
lib/Driver/ToolChains/Ananas.cpp
test/Driver/ananas.c
Index: test/Driver/ananas.c
===
--- test/Driver/ananas.c
+++ test/Driver/ananas.c
@@ -7,3 +7,11 @@
// CHECK-STATIC: crtbegin.o
// CHECK-STATIC: crtend.o
// CHECK-STATIC: crtn.o
+
+// RUN: %clang -no-canonical-prefixes -target x86_64-unknown-ananas -shared %s
\
+// RUN: --sysroot=%S/Inputs/ananas-tree -### 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-SHARED %s
+// CHECK-SHARED: crti.o
+// CHECK-SHARED: crtbeginS.o
+// CHECK-SHARED: crtendS.o
+// CHECK-SHARED: crtn.o
Index: lib/Driver/ToolChains/Ananas.cpp
===
--- lib/Driver/ToolChains/Ananas.cpp
+++ lib/Driver/ToolChains/Ananas.cpp
@@ -64,8 +64,19 @@
if (!D.SysRoot.empty())
CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
- // Ananas only supports static linkage for now.
- CmdArgs.push_back("-Bstatic");
+ if (Args.hasArg(options::OPT_static)) {
+CmdArgs.push_back("-Bstatic");
+ } else {
+if (Args.hasArg(options::OPT_rdynamic))
+ CmdArgs.push_back("-export-dynamic");
+if (Args.hasArg(options::OPT_shared)) {
+ CmdArgs.push_back("-Bshareable");
+} else {
+ Args.AddAllArgs(CmdArgs, options::OPT_pie);
+ CmdArgs.push_back("-dynamic-linker");
+ CmdArgs.push_back("/lib/ld-ananas.so");
+}
+ }
if (Output.isFilename()) {
CmdArgs.push_back("-o");
@@ -75,9 +86,15 @@
}
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+if (!Args.hasArg(options::OPT_shared)) {
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+}
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crti.o")));
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbegin.o")));
+if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie)) {
+
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbeginS.o")));
+} else {
+
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbegin.o")));
+}
}
Args.AddAllArgs(CmdArgs, options::OPT_L);
@@ -97,7 +114,10 @@
CmdArgs.push_back("-lc");
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtend.o")));
+if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
+
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
+else
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtend.o")));
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtn.o")));
}
Index: test/Driver/ananas.c
===
--- test/Driver/ananas.c
+++ test/Driver/ananas.c
@@ -7,3 +7,11 @@
// CHECK-STATIC: crtbegin.o
// CHECK-STATIC: crtend.o
// CHECK-STATIC: crtn.o
+
+// RUN: %clang -no-canonical-prefixes -target x86_64-unknown-ananas -shared %s \
+// RUN: --sysroot=%S/Inputs/ananas-tree -### 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-SHARED %s
+// CHECK-SHARED: crti.o
+// CHECK-SHARED: crtbeginS.o
+// CHECK-SHARED: crtendS.o
+// CHECK-SHARED: crtn.o
Index: lib/Driver/ToolChains/Ananas.cpp
===
--- lib/Driver/ToolChains/Ananas.cpp
+++ lib/Driver/ToolChains/Ananas.cpp
@@ -64,8 +64,19 @@
if (!D.SysRoot.empty())
CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
- // Ananas only supports static linkage for now.
- CmdArgs.push_back("-Bstatic");
+ if (Args.hasArg(options::OPT_static)) {
+CmdArgs.push_back("-Bstatic");
+ } else {
+if (Args.hasArg(options::OPT_rdynamic))
+ CmdArgs.push_back("-export-dynamic");
+if (Args.hasArg(options::OPT_shared)) {
+ CmdArgs.push_back("-Bshareable");
+} else {
+ Args.AddAllArgs(CmdArgs, options::OPT_pie);
+ CmdArgs.push_back("-dynamic-linker");
+ CmdArgs.push_back("/lib/ld-ananas.so");
+}
+ }
if (Output.isFilename()) {
CmdArgs.push_back("-o");
@@ -75,9 +86,15 @@
}
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
-CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+if (!Args.hasArg(options::OPT_shared)) {
+ CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
+}
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crti.o")));
-CmdArgs.push_back(Args.MakeArgString(ToolChain
r322065 - Avoid assumption that lit tests are writable (in a couple more places). NFC
Author: sammccall Date: Tue Jan 9 01:32:53 2018 New Revision: 322065 URL: http://llvm.org/viewvc/llvm-project?rev=322065&view=rev Log: Avoid assumption that lit tests are writable (in a couple more places). NFC Modified: cfe/trunk/test/Modules/modify-module.m cfe/trunk/test/PCH/modified-header-crash.c Modified: cfe/trunk/test/Modules/modify-module.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/modify-module.m?rev=322065&r1=322064&r2=322065&view=diff == --- cfe/trunk/test/Modules/modify-module.m (original) +++ cfe/trunk/test/Modules/modify-module.m Tue Jan 9 01:32:53 2018 @@ -3,9 +3,9 @@ // RUN: rm -rf %t // RUN: mkdir -p %t/include -// RUN: cp %S/Inputs/Modified/A.h %t/include -// RUN: cp %S/Inputs/Modified/B.h %t/include -// RUN: cp %S/Inputs/Modified/module.map %t/include +// RUN: cat %S/Inputs/Modified/A.h > %t/include/A.h +// RUN: cat %S/Inputs/Modified/B.h > %t/include/B.h +// RUN: cat %S/Inputs/Modified/module.map > %t/include/module.map // RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t/cache -fmodules -fimplicit-module-maps -I %t/include %s -verify // RUN: echo '' >> %t/include/B.h // RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t/cache -fmodules -fimplicit-module-maps -I %t/include %s -verify Modified: cfe/trunk/test/PCH/modified-header-crash.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/modified-header-crash.c?rev=322065&r1=322064&r2=322065&view=diff == --- cfe/trunk/test/PCH/modified-header-crash.c (original) +++ cfe/trunk/test/PCH/modified-header-crash.c Tue Jan 9 01:32:53 2018 @@ -1,6 +1,6 @@ // Don't crash. -// RUN: cp %S/modified-header-crash.h %t.h +// RUN: cat %S/modified-header-crash.h > %t.h // RUN: %clang_cc1 -DCAKE -x c-header %t.h -emit-pch -o %t // RUN: echo 'int foobar;' >> %t.h // RUN: not %clang_cc1 %s -include-pch %t -fsyntax-only ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41823: [clangd] Add more filters for collected symbols.
hokein added a comment.
With this patch, the index-based code completion will not provide any
candidates for `ns1::MyClass::^` (as the index-based completion drops all
results from clang's completion engine), we need a way to combine symbols from
3 sources (static index, dynamic index and clang's completion engine).
Comment at: clangd/index/SymbolCollector.cpp:80
+: decl(unless(isExpansionInMainFile())),
+hasDeclContext(anyOf(namespaceDecl(),
translationUnitDecl(),
+*D, *ASTCtx)
These ast matchers seems to be relatively simple, maybe we can use the `Decl`
methods to implement them at the moment. Or will we add more complex filters
in the future?
Comment at: clangd/index/SymbolCollector.cpp:116
- if (const NamedDecl *ND = llvm::dyn_cast(D)) {
-// FIXME: Should we include the internal linkage symbols?
-if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
- return true;
+ if (shouldFilterDecl(D, ASTCtx, Opts))
+return true;
How about moving the this function inside the `if` below? so we don't duplicate
the `dyn_cast` twice.
Comment at: clangd/index/SymbolCollector.h:28
+ struct Options {
+bool IndexMainFiles = false;
+ };
We need documentation for the options.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
hokein updated this revision to Diff 129047.
hokein marked 2 inline comments as done.
hokein added a comment.
Fix remaining comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41668
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/MemIndex.cpp
clangd/index/MemIndex.h
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -453,12 +453,6 @@
std::unique_ptr simpleIndexFromSymbols(
std::vector> Symbols) {
- auto I = llvm::make_unique();
- struct Snapshot {
-SymbolSlab Slab;
-std::vector Pointers;
- };
- auto Snap = std::make_shared();
SymbolSlab::Builder Slab;
for (const auto &Pair : Symbols) {
Symbol Sym;
@@ -475,13 +469,7 @@
Sym.SymInfo.Kind = Pair.second;
Slab.insert(Sym);
}
- Snap->Slab = std::move(Slab).build();
- for (auto &Iter : Snap->Slab)
-Snap->Pointers.push_back(&Iter);
- auto S = std::shared_ptr>(std::move(Snap),
-&Snap->Pointers);
- I->build(std::move(S));
- return std::move(I);
+ return MemIndex::build(std::move(Slab).build());
}
TEST(CompletionTest, NoIndex) {
@@ -496,6 +484,23 @@
EXPECT_THAT(Results.items, Has("No"));
}
+TEST(CompletionTest, StaticAndDynamicIndex) {
+ clangd::CodeCompleteOptions Opts;
+ auto StaticIdx =
+ simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class}});
+ Opts.StaticIndex = StaticIdx.get();
+ auto DynamicIdx =
+ simpleIndexFromSymbols({{"ns::foo", index::SymbolKind::Function}});
+ Opts.Index = DynamicIdx.get();
+
+ auto Results = completions(R"cpp(
+ void f() { ::ns::^ }
+ )cpp",
+ Opts);
+ EXPECT_THAT(Results.items, Contains(Labeled("[G]XYZ")));
+ EXPECT_THAT(Results.items, Contains(Labeled("foo")));
+}
+
TEST(CompletionTest, SimpleIndexBased) {
clangd::CodeCompleteOptions Opts;
auto I = simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class},
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -11,6 +11,7 @@
#include "JSONRPCDispatcher.h"
#include "Path.h"
#include "Trace.h"
+#include "index/SymbolYAML.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
@@ -26,7 +27,24 @@
namespace {
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 = SymbolFromYAML(Buffer.get()->getBuffer());
+ SymbolSlab::Builder SymsBuilder;
+ for (auto Sym : Slab)
+SymsBuilder.insert(Sym);
+
+ return MemIndex::build(std::move(SymsBuilder).build());
}
+} // namespace
static llvm::cl::opt CompileCommandsDir(
"compile-commands-dir",
@@ -97,6 +115,15 @@
"use index built from symbols in opened files"),
llvm::cl::init(false), llvm::cl::Hidden);
+static llvm::cl::opt YamlSymbolFile(
+"yaml-symbol-file",
+llvm::cl::desc(
+"YAML-format global symbol file to build the static index. It is only "
+"available when 'enable-index-based-completion' is enabled.\n"
+"WARNING: This option is experimental only, and will be removed "
+"eventually. Don't rely on it."),
+llvm::cl::init(""), llvm::cl::Hidden);
+
int main(int argc, char *argv[]) {
llvm::cl::ParseCommandLineOptions(argc, argv, "clangd");
@@ -182,13 +209,16 @@
// Change stdin to binary to not lose \r\n on windows.
llvm::sys::ChangeStdinToBinary();
+ std::unique_ptr StaticIdx;
+ if (EnableIndexBasedCompletion && !YamlSymbolFile.empty())
+StaticIdx = BuildStaticIndex(YamlSymbolFile);
clangd::CodeCompleteOptions CCOpts;
CCOpts.EnableSnippets = EnableSnippets;
CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
CCOpts, ResourceDirRef, CompileCommandsDirPath,
-EnableIndexBasedCompletion);
+EnableIndexBasedCompletion, std::move(StaticIdx));
constexpr int NoShutdownRequestErrorCode = 1;
llvm::set_thread_name("clangd.main");
return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErr
[PATCH] D41759: [clangd] Catch more symbols in SymbolCollector.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM (see the review comment about adding a comment in the code too) Comment at: clangd/index/SymbolCollector.cpp:89 +// violations. +if (ND->isInAnonymousNamespace()) return true; hokein wrote: > ilya-biryukov wrote: > > Why don't we include symbols from anonymous namespaces too? > > They are very similar to static symbols. > Yeah, these symbols need a special handling (the qualified name is like > `foobar`), we don't support them well enough. I think it is > fine to ignore them at the moment. Makes sense. Let's add a comment on why we skip them here. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41823: [clangd] Add more filters for collected symbols.
ilya-biryukov added a comment.
In https://reviews.llvm.org/D41823#970725, @hokein wrote:
> With this patch, the index-based code completion will not provide any
> candidates for `ns1::MyClass::^` (as the index-based completion drops all
> results from clang's completion engine), we need a way to combine symbols
> from 3 sources (static index, dynamic index and clang's completion engine).
Given that index-based completion is still experimental I think it's still fine
to address this in a follow-up change. (In order to keep both changes simpler
and more focused).
Comment at: clangd/index/FileIndex.cpp:22
llvm::ArrayRef Decls) {
- auto Collector = std::make_shared();
+ auto CollectorOpts = SymbolCollector::Options();
+ CollectorOpts.IndexMainFiles = true;
NIT: why not `SymbolCollector::Options CollectorOpts`? It's seems more concise.
Or even `SymbolCollector::Options{/*IndexMainFiles=*/true}`;
The last one does not play nicely with moving the fields of the struct around,
though
Comment at: clangd/index/SymbolCollector.cpp:80
+: decl(unless(isExpansionInMainFile())),
+hasDeclContext(anyOf(namespaceDecl(),
translationUnitDecl(),
+*D, *ASTCtx)
hokein wrote:
> These ast matchers seems to be relatively simple, maybe we can use the `Decl`
> methods to implement them at the moment. Or will we add more complex filters
> in the future?
I second this. Also the code would probably be more readable without matchers
in this particular example.
Comment at: clangd/index/SymbolCollector.cpp:86
+// FIXME: Should we include the internal linkage symbols?
+if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
+ return true;
This should probably include changes from @hokein 's patch D41759.
Comment at: clangd/index/SymbolCollector.h:28
+ struct Options {
+bool IndexMainFiles = false;
+ };
hokein wrote:
> We need documentation for the options.
Also, would naming it `OnlyMainFiles` make the intent of the option clearer?
Comment at: unittests/clangd/FileIndexTests.cpp:173
M.update(Ctx, "f1",
- build("f1", "class X { static int m1; int m2;};").getPointer());
+ build("f1", "class X { int m2; static void f(); };").getPointer());
Why do we need to remove `static int m1`?
Perhaps simply adding `void f()` is enough?
Comment at: unittests/clangd/SymbolCollectorTests.cpp:107
const std::string Header = R"(
-class Foo {
- void f();
-};
+class Foo {};
void f1();
Why do we remove `void f()`? Maybe assert that it's not in the list of symbols
instead?
Comment at: unittests/clangd/SymbolCollectorTests.cpp:182
)";
- runSymbolCollector(/*Header=*/"", Main);
+ runSymbolCollector(Header, /*Main=*/"");
EXPECT_THAT(
Why should we change this test?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41759: [clangd] Catch more symbols in SymbolCollector.
hokein updated this revision to Diff 129053.
hokein added a comment.
Add a comment for symbols in anonymous namespace.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41759
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -29,6 +29,7 @@
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
MATCHER_P(QName, Name, "") {
@@ -97,16 +98,53 @@
};
void f1();
inline void f2() {}
+static const int KInt = 2;
+const char* kStr = "123";
)";
const std::string Main = R"(
namespace {
void ff() {} // ignore
}
+
void f1() {}
+
+namespace foo {
+// Type alias
+typedef int int32;
+using int32_t = int32;
+
+// Variable
+int v1;
+
+// Namespace
+namespace bar {
+int v2;
+}
+// Namespace alias
+namespace baz = bar;
+
+// FIXME: using declaration is not supported as the IndexAction will ignore
+// implicit declarations (the implicit using shadow declaration) by
default,
+// and there is no way to customize this behavior at the moment.
+using bar::v2;
+} // namespace foo
)";
runSymbolCollector(Header, Main);
- EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
-QName("f1"), QName("f2")));
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"),
+ QName("Foo::f"),
+ QName("f1"),
+ QName("f2"),
+ QName("KInt"),
+ QName("kStr"),
+ QName("foo"),
+ QName("foo::bar"),
+ QName("foo::int32"),
+ QName("foo::int32_t"),
+ QName("foo::v1"),
+ QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, YAMLConversions) {
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -80,8 +80,15 @@
return true;
if (const NamedDecl *ND = llvm::dyn_cast(D)) {
-// FIXME: Should we include the internal linkage symbols?
-if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
+// FIXME: figure out a way to handle internal linkage symbols (e.g. static
+// variables, function) defined in the .cc files. Also we skip the symbols
+// in anonymous namespace as the qualifier names of these symbols are like
+// `foobar`, which need a special handling.
+// In real world projects, we have a relatively large set of header files
+// that define static variables (like "static const int A = 1;"), we still
+// want to collect these symbols, although they cause potential ODR
+// violations.
+if (ND->isInAnonymousNamespace())
return true;
llvm::SmallString<128> USR;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -29,6 +29,7 @@
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
MATCHER_P(QName, Name, "") {
@@ -97,16 +98,53 @@
};
void f1();
inline void f2() {}
+static const int KInt = 2;
+const char* kStr = "123";
)";
const std::string Main = R"(
namespace {
void ff() {} // ignore
}
+
void f1() {}
+
+namespace foo {
+// Type alias
+typedef int int32;
+using int32_t = int32;
+
+// Variable
+int v1;
+
+// Namespace
+namespace bar {
+int v2;
+}
+// Namespace alias
+namespace baz = bar;
+
+// FIXME: using declaration is not supported as the IndexAction will ignore
+// implicit declarations (the implicit using shadow declaration) by default,
+// and there is no way to customize this behavior at the moment.
+using bar::v2;
+} // namespace foo
)";
runSymbolCollector(Header, Main);
- EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
-QName("f1"), QName("f2")));
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"),
+ QName("Foo::f"),
+ QName("f1"),
+ QName("f2"),
+ QName("KInt"),
+ QName("kStr"),
+ QName("foo"),
+
[clang-tools-extra] r322067 - [clangd] Catch more symbols in SymbolCollector.
Author: hokein
Date: Tue Jan 9 02:44:09 2018
New Revision: 322067
URL: http://llvm.org/viewvc/llvm-project?rev=322067&view=rev
Log:
[clangd] Catch more symbols in SymbolCollector.
Summary:
We currently only collect external-linkage symbols in the collector,
which results in missing some typical symbols (like no-linkage type alias
symbols).
This patch relaxes the constraint a bit to allow collecting more symbols.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D41759
Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=322067&r1=322066&r2=322067&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Tue Jan 9
02:44:09 2018
@@ -80,8 +80,15 @@ bool SymbolCollector::handleDeclOccurenc
return true;
if (const NamedDecl *ND = llvm::dyn_cast(D)) {
-// FIXME: Should we include the internal linkage symbols?
-if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
+// FIXME: figure out a way to handle internal linkage symbols (e.g. static
+// variables, function) defined in the .cc files. Also we skip the symbols
+// in anonymous namespace as the qualifier names of these symbols are like
+// `foobar`, which need a special handling.
+// In real world projects, we have a relatively large set of header files
+// that define static variables (like "static const int A = 1;"), we still
+// want to collect these symbols, although they cause potential ODR
+// violations.
+if (ND->isInAnonymousNamespace())
return true;
llvm::SmallString<128> USR;
Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=322067&r1=322066&r2=322067&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Tue Jan
9 02:44:09 2018
@@ -29,6 +29,7 @@
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
MATCHER_P(QName, Name, "") {
@@ -97,16 +98,53 @@ TEST_F(SymbolCollectorTest, CollectSymbo
};
void f1();
inline void f2() {}
+static const int KInt = 2;
+const char* kStr = "123";
)";
const std::string Main = R"(
namespace {
void ff() {} // ignore
}
+
void f1() {}
+
+namespace foo {
+// Type alias
+typedef int int32;
+using int32_t = int32;
+
+// Variable
+int v1;
+
+// Namespace
+namespace bar {
+int v2;
+}
+// Namespace alias
+namespace baz = bar;
+
+// FIXME: using declaration is not supported as the IndexAction will ignore
+// implicit declarations (the implicit using shadow declaration) by
default,
+// and there is no way to customize this behavior at the moment.
+using bar::v2;
+} // namespace foo
)";
runSymbolCollector(Header, Main);
- EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
-QName("f1"), QName("f2")));
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"),
+ QName("Foo::f"),
+ QName("f1"),
+ QName("f2"),
+ QName("KInt"),
+ QName("kStr"),
+ QName("foo"),
+ QName("foo::bar"),
+ QName("foo::int32"),
+ QName("foo::int32_t"),
+ QName("foo::v1"),
+ QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, YAMLConversions) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41759: [clangd] Catch more symbols in SymbolCollector.
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rCTE322067: [clangd] Catch more symbols in SymbolCollector.
(authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41759?vs=129053&id=129054#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41759
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -80,8 +80,15 @@
return true;
if (const NamedDecl *ND = llvm::dyn_cast(D)) {
-// FIXME: Should we include the internal linkage symbols?
-if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
+// FIXME: figure out a way to handle internal linkage symbols (e.g. static
+// variables, function) defined in the .cc files. Also we skip the symbols
+// in anonymous namespace as the qualifier names of these symbols are like
+// `foobar`, which need a special handling.
+// In real world projects, we have a relatively large set of header files
+// that define static variables (like "static const int A = 1;"), we still
+// want to collect these symbols, although they cause potential ODR
+// violations.
+if (ND->isInAnonymousNamespace())
return true;
llvm::SmallString<128> USR;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -29,6 +29,7 @@
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
MATCHER_P(QName, Name, "") {
@@ -97,16 +98,53 @@
};
void f1();
inline void f2() {}
+static const int KInt = 2;
+const char* kStr = "123";
)";
const std::string Main = R"(
namespace {
void ff() {} // ignore
}
+
void f1() {}
+
+namespace foo {
+// Type alias
+typedef int int32;
+using int32_t = int32;
+
+// Variable
+int v1;
+
+// Namespace
+namespace bar {
+int v2;
+}
+// Namespace alias
+namespace baz = bar;
+
+// FIXME: using declaration is not supported as the IndexAction will ignore
+// implicit declarations (the implicit using shadow declaration) by
default,
+// and there is no way to customize this behavior at the moment.
+using bar::v2;
+} // namespace foo
)";
runSymbolCollector(Header, Main);
- EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Foo::f"),
-QName("f1"), QName("f2")));
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"),
+ QName("Foo::f"),
+ QName("f1"),
+ QName("f2"),
+ QName("KInt"),
+ QName("kStr"),
+ QName("foo"),
+ QName("foo::bar"),
+ QName("foo::int32"),
+ QName("foo::int32_t"),
+ QName("foo::v1"),
+ QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, YAMLConversions) {
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -80,8 +80,15 @@
return true;
if (const NamedDecl *ND = llvm::dyn_cast(D)) {
-// FIXME: Should we include the internal linkage symbols?
-if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
+// FIXME: figure out a way to handle internal linkage symbols (e.g. static
+// variables, function) defined in the .cc files. Also we skip the symbols
+// in anonymous namespace as the qualifier names of these symbols are like
+// `foobar`, which need a special handling.
+// In real world projects, we have a relatively large set of header files
+// that define static variables (like "static const int A = 1;"), we still
+// want to collect these symbols, although they cause potential ODR
+// violations.
+if (ND->isInAnonymousNamespace())
return true;
llvm::SmallString<128> USR;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -29,6 +29,7 @@
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
+using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
MATCHER_P(QName, Name, "") {
@@ -97,16 +98,53 @@
}
[PATCH] D38639: [clangd] #include statements support for Open definition
ilya-biryukov added a comment.
@malaperle, hi! Both new diff and updating this works, so feel free the one
that suits you best. I tend to look over the whole change on each new round of
reviews anyway.
Comment at: clangd/ClangdUnit.cpp:113
+ CppFilePreambleCallbacks(IncludeReferenceMap &IRM)
+ : IRM(IRM) {}
malaperle wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > Let's create a new empty map inside this class and have a
> > > `takeIncludeReferences` method (similar to `TopLevelDeclIDs` and
> > > `takeTopLevelDeclIDs`)
> > This comment is not addressed yet, but marked as done.
> As mentioned below, the idea was to have a single map being appended to,
> without having to merge two separate maps. However, I can change the code so
> that two separate maps are built and merged if you prefer.
>
> But I'm not so clear if that's what you'd prefer:
>
> > You copy the map for preamble and then append to it in
> > CppFilePreambleCallbacks? That also LG, we should not have many references
> > there anyway.
>
> It's not meant to have any copy. The idea was to create a single
> IncludeReferenceMap in CppFile::deferRebuild, populate it with both preamble
> and non-preamble include references and std::move it around for later use
> (stored in ParsedAST).
We can't have a single map because AST is rebuilt more often than the Preamble,
so we have two options:
- Store a map for the preamble separately, copy it when we need to rebuild the
AST and append references from the AST to the new instance of the map.
- Store two maps: one contains references only from the Preamble, the other one
from the AST.
I think both are fine, since the copy of the map will be cheap anyway, as we
only store a list of includes inside the main file.
Comment at: clangd/ClangdUnit.cpp:281
+std::shared_ptr PCHs,
+IntrusiveRefCntPtr VFS, IncludeReferenceMap IRM) {
malaperle wrote:
> ilya-biryukov wrote:
> > Don't we already store the map we need in `PreambleData`? Why do we need an
> > extra `IRM` parameter?
> Since the map will be now stored in ParsedAST and the instance doesn't exist
> yet, we need to keep the IRM parameter. I noticed that it wasn't being
> std::move'd though.
That looks fine. Also see the other comment on why we need to copy the map from
the Preamble, and not `std::move` it.
Comment at: clangd/XRefs.cpp:185
+
+ if ((unsigned)R.start.line ==
+ SourceMgr.getSpellingLineNumber(
malaperle wrote:
> ilya-biryukov wrote:
> > why do we need to convert to unsigned? To slience compiler warnings?
> Yes, "line" from the protocol is signed, whereas getSpellingColumn/lineNumber
> returns unsigned. I'll extract another var for the line number and cast both
> to int instead to have less casts and make the condition smaller.
Can we maybe convert to `clangd::Position` using the helper methods first and
do the comparison of two `clangd::Position`s?
Comparing between `clangd::Position` and clang's line/column numbers is a
common source of off-by-one errors in clangd.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38639
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41487: [clang-format] Adds a FormatStyleSet
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D41487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41816: [analyzer] Model and check unrepresentable left shifts
xazax.hun added a comment. Overall looks good to me, one comment inline. I think it is good to have these checks to prevent the analyzer executing undefined behavior. Maybe this would make it more feasible to run the analyzer with ubsan :) In the future, it would be great to also look for these cases symbolically, but I believe it is perfectly fine to have that in a separate patch. Comment at: lib/StaticAnalyzer/Core/BasicValueFactory.cpp:238 + if (V1.isSigned() && (unsigned) Amt > V1.countLeadingZeros()) + return nullptr; Do you get a warning if you remove the cast above? I am not sure that we actually want to have the cast here. https://reviews.llvm.org/D41816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
sammccall added inline comments.
Comment at: clangd/ClangdLSPServer.h:40
+ bool BuildDynamicSymbolIndex,
+ std::unique_ptr StaticIdx);
is there a reason ClangdServer should own this? I can imagine this complicating
life for embedders. (vs a raw pointer)
Comment at: clangd/CodeComplete.cpp:554
+ const SpecifiedScope &SSInfo,
+ llvm::StringRef IndexSource = "") {
CompletionItem Item;
ioeric wrote:
> Maybe `IndexSourceLabel`?
Actually, can we give this a more generic name like DebuggingLabel?
That will make things less confusing as we store slightly different info here
over time (e.g. this text will definitely change when merging happens). It also
indicates that this isn't functionally important.
If you don't mind, I'd also change the value so you just pass "G" and it gets
formatted into "[G] " by this function. That means we can easily format it
differently (e.g. hide it from the UI actually include it in the JSON as an
extension field) in a way that makes sense.
Comment at: clangd/CodeComplete.cpp:591
- Items->isIncomplete = !Index.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) {
-Items->items.push_back(indexCompletionItem(Sym, Filter, SSInfo));
- });
+ // FIXME: figure out a good algorithm to merge symbols from dynamic index and
+ // static index.
nit: this comment implies that we should do a two-way merge here between
static/dynamic index. I don't think that's the case.
We're also going want to merge with AST based results, and probably want to do
that *before* generating CompletionItems, because scoring should take signals
from all sources into account.
I'd suggest reverting this function to use just one index, and calling it twice.
Then this comment about merging would live near the call site, and cover all
three sources - it'd suggest the fix where we should be applying it.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41668
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D6550: ASTImporter: Fix missing SourceLoc imports
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Great! Thanks! Repository: rC Clang https://reviews.llvm.org/D6550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
ilya-biryukov added inline comments. Comment at: clangd/ClangdLSPServer.h:40 + bool BuildDynamicSymbolIndex, + std::unique_ptr StaticIdx); sammccall wrote: > is there a reason ClangdServer should own this? I can imagine this > complicating life for embedders. (vs a raw pointer) +1, we had a design where `ClangdServer` owns everything before and having a raw reference turned out to be much more flexible! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 129058.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- [clangd] Address review comments; made Detail a pointer.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,23 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -109,6 +120,38 @@
QName("f1"), QName("f2")));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -122,6 +165,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -136,15 +185,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 129059.
ioeric marked an inline comment as done.
ioeric added a comment.
- Minor cleanup.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,23 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -109,6 +120,38 @@
QName("f1"), QName("f2")));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -122,6 +165,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -136,15 +185,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
arg.insertText == Text;
@@ -
[PATCH] D41730: [clangd] Use ToolExecutor to write the global-symbol-builder tool.
ioeric updated this revision to Diff 129063.
ioeric added a comment.
Address review comment: move all ToolExecutor and YAML-specific logics out of
SymbolCollector.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41730
Files:
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/SymbolYAML.cpp
clangd/index/SymbolYAML.h
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -147,7 +147,7 @@
UnorderedElementsAre(QName("clang::Foo2")));
std::string ConcatenatedYAML =
- SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
+ SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);
auto ConcatenatedSymbols = SymbolFromYAML(ConcatenatedYAML);
EXPECT_THAT(ConcatenatedSymbols,
UnorderedElementsAre(QName("clang::Foo1"),
Index: clangd/index/SymbolYAML.h
===
--- clangd/index/SymbolYAML.h
+++ clangd/index/SymbolYAML.h
@@ -27,9 +27,13 @@
// Read symbols from a YAML-format string.
SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent);
+// Convert a single symbol to YAML-format string.
+// The YAML result is safe to concatenate.
+std::string SymbolToYAML(Symbol Sym);
+
// Convert symbols to a YAML-format string.
// The YAML result is safe to concatenate if you have multiple symbol slabs.
-std::string SymbolToYAML(const SymbolSlab& Symbols);
+std::string SymbolsToYAML(const SymbolSlab& Symbols);
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolYAML.cpp
===
--- clangd/index/SymbolYAML.cpp
+++ clangd/index/SymbolYAML.cpp
@@ -133,14 +133,22 @@
return std::move(Syms).build();
}
-std::string SymbolToYAML(const SymbolSlab& Symbols) {
+std::string SymbolsToYAML(const SymbolSlab& Symbols) {
std::string Str;
llvm::raw_string_ostream OS(Str);
llvm::yaml::Output Yout(OS);
for (Symbol S : Symbols) // copy: Yout<< requires mutability.
Yout<< S;
return OS.str();
}
+std::string SymbolToYAML(Symbol Sym) {
+ std::string Str;
+ llvm::raw_string_ostream OS(Str);
+ llvm::yaml::Output Yout(OS);
+ Yout << Sym;
+ return OS.str();
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -17,15 +17,15 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
#include "clang/Frontend/FrontendActions.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
-#include "clang/Tooling/CommonOptionsParser.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Signals.h"
#include "llvm/Support/Path.h"
-#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
#include "llvm/Support/ThreadPool.h"
using namespace llvm;
@@ -37,18 +37,46 @@
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
- SymbolIndexActionFactory() = default;
+ SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
clang::FrontendAction *create() override {
+// Wraps the index action and reports collected symbols to the execution
+// context at the end of each translation unit.
+class WrappedIndexAction : public WrapperFrontendAction {
+public:
+ WrappedIndexAction(std::shared_ptr C,
+ const index::IndexingOptions &Opts,
+ tooling::ExecutionContext *Ctx)
+ : WrapperFrontendAction(
+index::createIndexingAction(C, Opts, nullptr)),
+Ctx(Ctx), Collector(C) {}
+
+ void EndSourceFileAction() override {
+WrapperFrontendAction::EndSourceFileAction();
+
+auto Symbols = Collector->takeSymbols();
+for (const auto &Sym : Symbols) {
+ std::string IDStr;
+ llvm::raw_string_ostream OS(IDStr);
+ OS << Sym.ID;
+ Ctx->reportResult(OS.str(), SymbolToYAML(Sym));
+}
+ }
+
+private:
+ tooling::ExecutionContext *Ctx;
+ std::shared_ptr Collector;
+};
+
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = false;
-Collector = std::make_shared();
-return index::createIndexingAction(Collector, IndexOpts, nullptr).release();
+return new WrappedIn
[PATCH] D41730: [clangd] Use ToolExecutor to write the global-symbol-builder tool.
ioeric added a comment. In https://reviews.llvm.org/D41730#970704, @sammccall wrote: > This makes `SymbolCollector` (production code) depend on YAML (explicitly > experimental), as well as on Tooling interfaces. > At least we should invert this by making the thing passed to SymbolCollector > a `function` or so. Thanks for the catch! I completely missed these perspectives... > Moreover, it's unfortunate we'd need to change SymbolCollector at all. > Currently it's a nice abstraction that does one thing (AST -> `SymbolSlab`), > now we're making it do two things. It can't be for scalability reasons - > there's no problem fitting all the symbols from a TU in memory. So why do we > need to do this? > I don't really understand the `ToolExecutor` interfaces well, but ISTM > overriding `SymbolIndexActionFactory::runInvocation` to flush the symbols in > the slab out to the `ExecutionContext` would allow you to use > `SymbolCollector` unchanged? `WrapperFrontendAction` is helpful here. All changes have been moved into the tool now. PTAL Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface
milianw added a comment. still looks good to me. can someone else please review and commit this? Repository: rC Clang https://reviews.llvm.org/D10833 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37650: [continued] Add a way to get the CXCursor that corresponds to a completion result
milianw requested changes to this revision. milianw added a comment. This revision now requires changes to proceed. I'm pretty sure this is not ABI compatible. You change the size of `CXCompletionResult`, so when anyone is iterating over the array in `CXCompletionResults` it will break. If you don't want to, I can try to find some time to take this over again ;-) Repository: rC Clang https://reviews.llvm.org/D37650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322074 - Track in the AST whether the operand to a UnaryOperator can overflow and then use that logic when evaluating constant expressions and emitting codegen.
Author: aaronballman
Date: Tue Jan 9 05:07:03 2018
New Revision: 322074
URL: http://llvm.org/viewvc/llvm-project?rev=322074&view=rev
Log:
Track in the AST whether the operand to a UnaryOperator can overflow and then
use that logic when evaluating constant expressions and emitting codegen.
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Analysis/BodyFarm.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteModernObjC.cpp
cfe/trunk/lib/Frontend/Rewrite/RewriteObjC.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/SemaPseudoObject.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/test/Misc/ast-dump-stmt.c
Modified: cfe/trunk/include/clang/AST/Expr.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=322074&r1=322073&r2=322074&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Jan 9 05:07:03 2018
@@ -1720,19 +1720,19 @@ public:
private:
unsigned Opc : 5;
+ unsigned CanOverflow : 1;
SourceLocation Loc;
Stmt *Val;
public:
-
- UnaryOperator(Expr *input, Opcode opc, QualType type,
-ExprValueKind VK, ExprObjectKind OK, SourceLocation l)
-: Expr(UnaryOperatorClass, type, VK, OK,
- input->isTypeDependent() || type->isDependentType(),
- input->isValueDependent(),
- (input->isInstantiationDependent() ||
-type->isInstantiationDependentType()),
- input->containsUnexpandedParameterPack()),
- Opc(opc), Loc(l), Val(input) {}
+ UnaryOperator(Expr *input, Opcode opc, QualType type, ExprValueKind VK,
+ExprObjectKind OK, SourceLocation l, bool CanOverflow)
+ : Expr(UnaryOperatorClass, type, VK, OK,
+ input->isTypeDependent() || type->isDependentType(),
+ input->isValueDependent(),
+ (input->isInstantiationDependent() ||
+ type->isInstantiationDependentType()),
+ input->containsUnexpandedParameterPack()),
+Opc(opc), CanOverflow(CanOverflow), Loc(l), Val(input) {}
/// \brief Build an empty unary operator.
explicit UnaryOperator(EmptyShell Empty)
@@ -1748,6 +1748,15 @@ public:
SourceLocation getOperatorLoc() const { return Loc; }
void setOperatorLoc(SourceLocation L) { Loc = L; }
+ /// Returns true if the unary operator can cause an overflow. For instance,
+ /// signed int i = INT_MAX; i++;
+ /// signed char c = CHAR_MAX; c++;
+ /// Due to integer promotions, c++ is promoted to an int before the postfix
+ /// increment, and the result is an int that cannot overflow. However, i++
+ /// can overflow.
+ bool canOverflow() const { return CanOverflow; }
+ void setCanOverflow(bool C) { CanOverflow = C; }
+
/// isPostfix - Return true if this is a postfix operation, like x++.
static bool isPostfix(Opcode Op) {
return Op == UO_PostInc || Op == UO_PostDec;
Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=322074&r1=322073&r2=322074&view=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Tue Jan 9 05:07:03 2018
@@ -2214,12 +2214,14 @@ void ASTDumper::VisitArrayInitIndexExpr(
}
void ASTDumper::VisitUnaryOperator(const UnaryOperator *Node) {
- VisitExpr(Node);
- OS << " " << (Node->isPostfix() ? "postfix" : "prefix")
- << " '" << UnaryOperator::getOpcodeStr(Node->getOpcode()) << "'";
-}
-
-void ASTDumper::VisitUnaryExprOrTypeTraitExpr(
+ VisitExpr(Node);
+ OS << " " << (Node->isPostfix() ? "postfix" : "prefix")
+ << " '" << UnaryOperator::getOpcodeStr(Node->getOpcode()) << "'";
+ if (!Node->canOverflow())
+OS << " cannot overflow";
+}
+
+void ASTDumper::VisitUnaryExprOrTypeTraitExpr(
const UnaryExprOrTypeTraitExpr *Node) {
VisitExpr(Node);
switch(Node->getKind()) {
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=322074&r1=322073&r2=322074&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Jan 9 05:07:03 2018
@@ -5120,14 +5120,13 @@ Expr *ASTNodeImporter::VisitUnaryOperato
if (!SubExpr)
return nullptr;
- return new (Importer.getToContext()) UnaryOperator(SubExpr, E->getOpcode(),
-
[PATCH] D33563: Track whether a unary operation can overflow
aaron.ballman closed this revision. aaron.ballman added a comment. Thanks! Committed in r322074. https://reviews.llvm.org/D33563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41800: [analyzer] Use a custom program point for the check::NewAllocator callback.
xazax.hun accepted this revision.
xazax.hun added a comment.
LG!
Comment at: include/clang/Analysis/ProgramPoint.h:592
+ friend class ProgramPoint;
+ PostAllocatorCall() {}
+ static bool isKind(const ProgramPoint &Location) {
Maybe `= default` is getting more canonical within LLVM? But that would not
match the rest of the file, so I am fine with not touching this.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:2907
+else if (Loc.getAs())
+ Out << "\\lPostAllocatorCall\\l";
I think this is fine for now, but I wonder if in the future it would make more
sense to have a getName or similar method for ProgramPoints.
Repository:
rC Clang
https://reviews.llvm.org/D41800
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41406: [analyzer] Add a new checker callback, check::NewAllocator.
xazax.hun added a comment.
Do you have a plan for the new false negatives when `c++-allocator-inlining` is
on? Should the user mark allocation functions with attributes?
Also, I was wondering if it would make sense to only have the PostCall callback
return the casted memory region in these specific cases instead of introducing
a new callback.
Otherwise looks good.
Comment at: lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp:143
+ /// \param Target The allocated region, casted to the class type.
+ void checkNewAllocator(const CXXNewExpr *NE, SVal Target,
+ CheckerContext &) const {}
Is it possible to have a `NonLoc` Target? If no, would it be better to make it
`Loc` type?
https://reviews.llvm.org/D41406
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41816: [analyzer] Model and check unrepresentable left shifts
rnkovacs updated this revision to Diff 129071.
rnkovacs marked an inline comment as done.
https://reviews.llvm.org/D41816
Files:
lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/BasicValueFactory.cpp
test/Analysis/bitwise-ops.c
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -51,3 +51,9 @@
}
return 0;
}
+
+int testUnrepresentableLeftShift(int a) {
+ if (a == 8)
+return a << 30; // expected-warning{{The result of the left shift is
undefined because it is not representable in the return type}}
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -224,7 +224,6 @@
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
if (V1.isSigned() && V1.isNegative())
return nullptr;
@@ -236,16 +235,17 @@
if (Amt >= V1.getBitWidth())
return nullptr;
+ if (V1.isSigned() && Amt > V1.countLeadingZeros())
+ return nullptr;
+
return &getValue( V1.operator<<( (unsigned) Amt ));
}
case BO_Shr: {
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
-
if (V2.isSigned() && V2.isNegative())
return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -141,6 +141,15 @@
C.isNegative(B->getLHS())) {
OS << "The result of the left shift is undefined because the left "
"operand is negative";
+ } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl) {
+SValBuilder &SB = C.getSValBuilder();
+const llvm::APSInt *LHS =
+SB.getKnownValue(state, C.getSVal(B->getLHS()));
+const llvm::APSInt *RHS =
+SB.getKnownValue(state, C.getSVal(B->getRHS()));
+if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros())
+ OS << "The result of the left shift is undefined because it is not "
+"representable in the return type";
} else {
OS << "The result of the '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -51,3 +51,9 @@
}
return 0;
}
+
+int testUnrepresentableLeftShift(int a) {
+ if (a == 8)
+return a << 30; // expected-warning{{The result of the left shift is undefined because it is not representable in the return type}}
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp
===
--- lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -224,7 +224,6 @@
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
if (V1.isSigned() && V1.isNegative())
return nullptr;
@@ -236,16 +235,17 @@
if (Amt >= V1.getBitWidth())
return nullptr;
+ if (V1.isSigned() && Amt > V1.countLeadingZeros())
+ return nullptr;
+
return &getValue( V1.operator<<( (unsigned) Amt ));
}
case BO_Shr: {
// FIXME: This logic should probably go higher up, where we can
// test these conditions symbolically.
- // FIXME: Expand these checks to include all undefined behavior.
-
if (V2.isSigned() && V2.isNegative())
return nullptr;
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -141,6 +141,15 @@
C.isNegative(B->getLHS())) {
OS << "The result of the left shift is undefined because the left "
"operand is negative";
+ } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl) {
+SValBuilder &SB = C.getSValBuilder();
+const llvm::APSInt *LHS =
+SB.getKnownValue(state, C.getSVal(B->getLHS()));
+const llvm::APSInt *RHS =
+SB.getKnownValue(state, C.getSVal(B->getRHS()));
+if ((unsigned) RHS->getZExtValue() > LHS->countLeadin
[PATCH] D41816: [analyzer] Model and check unrepresentable left shifts
rnkovacs added a comment. In https://reviews.llvm.org/D41816#970845, @xazax.hun wrote: > Overall looks good to me, one comment inline. I think it is good to have > these checks to prevent the analyzer executing undefined behavior. Maybe this > would make it more feasible to run the analyzer with ubsan :) > In the future, it would be great to also look for these cases symbolically, > but I believe it is perfectly fine to have that in a separate patch. I agree, but I thought that making these checks complete first might be a good idea. https://reviews.llvm.org/D41816 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D6550: ASTImporter: Fix missing SourceLoc imports
This revision was automatically updated to reflect the committed changes.
Closed by commit rC322079: [ASTImporter] Fix missing SourceLoc import for
ObjCMethodDecl selectors (authored by a.sidorin, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D6550
Files:
lib/AST/ASTImporter.cpp
test/ASTMerge/interface/Inputs/interface1.m
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2857,9 +2857,13 @@
ToParams[I]->setOwningFunction(ToMethod);
ToMethod->addDeclInternal(ToParams[I]);
}
+
SmallVector SelLocs;
D->getSelectorLocs(SelLocs);
- ToMethod->setMethodParams(Importer.getToContext(), ToParams, SelLocs);
+ for (SourceLocation &Loc : SelLocs)
+Loc = Importer.Import(Loc);
+
+ ToMethod->setMethodParams(Importer.getToContext(), ToParams, SelLocs);
ToMethod->setLexicalDeclContext(LexicalDC);
Importer.Imported(D, ToMethod);
Index: test/ASTMerge/interface/Inputs/interface1.m
===
--- test/ASTMerge/interface/Inputs/interface1.m
+++ test/ASTMerge/interface/Inputs/interface1.m
@@ -100,4 +100,6 @@
@implementation I15 : I12
@end
-
+@interface ImportSelectorSLoc { }
+-(int)addInt:(int)a toInt:(int)b moduloInt:(int)c; // don't crash here
+@end
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2857,9 +2857,13 @@
ToParams[I]->setOwningFunction(ToMethod);
ToMethod->addDeclInternal(ToParams[I]);
}
+
SmallVector SelLocs;
D->getSelectorLocs(SelLocs);
- ToMethod->setMethodParams(Importer.getToContext(), ToParams, SelLocs);
+ for (SourceLocation &Loc : SelLocs)
+Loc = Importer.Import(Loc);
+
+ ToMethod->setMethodParams(Importer.getToContext(), ToParams, SelLocs);
ToMethod->setLexicalDeclContext(LexicalDC);
Importer.Imported(D, ToMethod);
Index: test/ASTMerge/interface/Inputs/interface1.m
===
--- test/ASTMerge/interface/Inputs/interface1.m
+++ test/ASTMerge/interface/Inputs/interface1.m
@@ -100,4 +100,6 @@
@implementation I15 : I12
@end
-
+@interface ImportSelectorSLoc { }
+-(int)addInt:(int)a toInt:(int)b moduloInt:(int)c; // don't crash here
+@end
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322079 - [ASTImporter] Fix missing SourceLoc import for ObjCMethodDecl selectors
Author: a.sidorin
Date: Tue Jan 9 06:25:05 2018
New Revision: 322079
URL: http://llvm.org/viewvc/llvm-project?rev=322079&view=rev
Log:
[ASTImporter] Fix missing SourceLoc import for ObjCMethodDecl selectors
Patch by Nico Rieck, test case by Sean Callanan!
Differential Revision: https://reviews.llvm.org/D6550
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/test/ASTMerge/interface/Inputs/interface1.m
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=322079&r1=322078&r2=322079&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Jan 9 06:25:05 2018
@@ -2857,9 +2857,13 @@ Decl *ASTNodeImporter::VisitObjCMethodDe
ToParams[I]->setOwningFunction(ToMethod);
ToMethod->addDeclInternal(ToParams[I]);
}
+
SmallVector SelLocs;
D->getSelectorLocs(SelLocs);
- ToMethod->setMethodParams(Importer.getToContext(), ToParams, SelLocs);
+ for (SourceLocation &Loc : SelLocs)
+Loc = Importer.Import(Loc);
+
+ ToMethod->setMethodParams(Importer.getToContext(), ToParams, SelLocs);
ToMethod->setLexicalDeclContext(LexicalDC);
Importer.Imported(D, ToMethod);
Modified: cfe/trunk/test/ASTMerge/interface/Inputs/interface1.m
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/interface/Inputs/interface1.m?rev=322079&r1=322078&r2=322079&view=diff
==
--- cfe/trunk/test/ASTMerge/interface/Inputs/interface1.m (original)
+++ cfe/trunk/test/ASTMerge/interface/Inputs/interface1.m Tue Jan 9 06:25:05
2018
@@ -100,4 +100,6 @@
@implementation I15 : I12
@end
-
+@interface ImportSelectorSLoc { }
+-(int)addInt:(int)a toInt:(int)b moduloInt:(int)c; // don't crash here
+@end
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41799: [analyzer] PtrArithChecker: Update to use check::NewAllocator
xazax.hun added a comment. > (@xazax.hun - this is an alpha checker last touched by you, do you still have > plans for it?) I think this check could be very useful. Last I checked it was too noisy (e.g.: around 850 hits on LLVM), and I did plan to look into what are the main sources of false positives, but I did not have time to do so yet. Repository: rC Clang https://reviews.llvm.org/D41799 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40478: Added control flow architecture protection Flag
alexfh added inline comments. Comment at: test/CodeGen/x86-cf-protection.c:1 +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH Any reason this test runs clang with "-S" and "-emit-llvm"? Neither of those seems to be needed for the actual checks being made below. Repository: rC Clang https://reviews.llvm.org/D40478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r322080 - [clangd] Fix a bug in asynchronous code completion
Author: ibiryukov
Date: Tue Jan 9 06:39:27 2018
New Revision: 322080
URL: http://llvm.org/viewvc/llvm-project?rev=322080&view=rev
Log:
[clangd] Fix a bug in asynchronous code completion
A StringRef that goes out of scope was copied and used in a handler
running on a separate thread.
We didn't catch it because clangd does not use the async completion
API yet.
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=322080&r1=322079&r2=322080&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jan 9 06:39:27 2018
@@ -251,10 +251,17 @@ void ClangdServer::codeComplete(
auto CodeCompleteOpts = Opts;
if (FileIdx)
CodeCompleteOpts.Index = FileIdx.get();
+
+ // Copy File, as it is a PathRef that will go out of scope before Task is
+ // executed.
+ Path FileStr = File;
+ // Copy PCHs to avoid accessing this->PCHs concurrently
+ std::shared_ptr PCHs = this->PCHs;
// A task that will be run asynchronously.
auto Task =
// 'mutable' to reassign Preamble variable.
- [=](Context Ctx, CallbackType Callback) mutable {
+ [FileStr, Preamble, Resources, Contents, Pos, CodeCompleteOpts, TaggedFS,
+ PCHs](Context Ctx, CallbackType Callback) mutable {
if (!Preamble) {
// Maybe we built some preamble before processing this request.
Preamble = Resources->getPossiblyStalePreamble();
@@ -263,7 +270,7 @@ void ClangdServer::codeComplete(
// both the old and the new version in case only one of them matches.
CompletionList Result = clangd::codeComplete(
-Ctx, File, Resources->getCompileCommand(),
+Ctx, FileStr, Resources->getCompileCommand(),
Preamble ? &Preamble->Preamble : nullptr, Contents, Pos,
TaggedFS.Value, PCHs, CodeCompleteOpts);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41797: [analyzer] Suppress escape of this-pointer during construction.
xazax.hun added a comment. I am fine with suppressing the escape globally. I did see some code in the wild where the constructors registered the objects with a (global) map. But I think it is still easier to annotate code that does something unconventional than the other way around. Repository: rC Clang https://reviews.llvm.org/D41797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 129075.
ioeric added a comment.
- Merge branch 'master' of http://llvm.org/git/clang-tools-extra into symbol
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,12 +25,24 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -147,6 +158,38 @@
QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -160,6 +203,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -174,15 +223,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric added inline comments. Comment at: clangd/index/Index.h:122 + + llvm::Optional Detail; + sammccall wrote: > ioeric wrote: > > sammccall wrote: > > > ioeric wrote: > > > > sammccall wrote: > > > > > I think you probably want a raw pointer rather than optional: > > > > > - reduce the size of the struct when it's absent > > > > > - make it inheritance-friendly so we can hang index-specific info > > > > > off it > > > > > (raw pointer rather than unique_ptr because it's owned by a slab not > > > > > by malloc, but unique_ptr is ok for now) > > > > > > > > > This is not easy for now with `unique_ptr` because of this line :( > > > > https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141). > > > > > > > > > > > > This shouldn't be an issue when we have the optimized symbol slab, > > > > where we store raw pointers. And we would probably want to serialize > > > > the whole slab instead of the individual symbols anyway. > > > > > > > > > reduce the size of the struct when it's absent > > > > `llvm::Optional` doesn't take much more space, so the size should be > > > > fine. > > > > > > > > > make it inheritance-friendly so we can hang index-specific info off it > > > > Could you elaborate on `index-specific info`? It's unclear to me how > > > > this would be used. > > > > This is not easy for now with unique_ptr because of this line > > > Oh no, somehow i missed this during review. > > > We shouldn't be relying on symbols being copyable. I'll try to work out > > > how to fix this and delete the copy constructor. > > > > > > > This shouldn't be an issue when we have the optimized symbol slab, > > > > where we store raw pointers. > > > Sure. That's not a big monolithic/mysterous thing though, storing the > > > details in the slab can be done in this patch... If you think it'll be > > > easier once strings are arena-based, then maybe we should delay this > > > patch until that's done, rather than make that work bigger. > > > > > > > And we would probably want to serialize the whole slab instead of the > > > > individual symbols anyway. > > > This i'm less sure about, but I don't think it matters. > > > > > > > llvm::Optional doesn't take much more space, so the size should be fine. > > > Optional takes the same size as the details itself (plus one bool). This > > > is fairly small for now, but I think a major point of Details is to > > > expand it in the future? > > > > > > > Could you elaborate on index-specific info? It's unclear to me how this > > > > would be used. > > > Yeah, this is something we talked about in the meeting with Marc-Andre > > > but it's not really obvious - what's the point of allowing Details to be > > > extended if clangd has to consume it? > > > > > > It sounded like he might have use cases for using index infrastructure > > > outside clangd. We might also have google-internal index features we want > > > (matching generated code to proto fields?). I'm not really sure how > > > compelling this argument is. > > Thanks for the allocator change! > > > > `Details` now contains just stringrefs. I wonder how much we want it to be > > inherit-friendly at this point, as the size is relatively small now. If you > > think this is a better way to go, I'll make the structure contain strings > > and store the whole structure in arena. This would require some tweaks for > > yamls tho but shouldn't be hard. > So this needs to be at least optional I think - at the moment, how does an > API user know whether to fetch the rest of the details? > > FWIW, the size difference is already significant: symbol is 136 bytes if this > is a unique_ptr, and 164 bytes if it's optional (+20%) - StringRefs are still > pretty big. > But I don't feel really strongly about this. Done. Made it a raw pointer. PTAL Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41345 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40478: Added control flow architecture protection Flag
oren_ben_simhon added inline comments. Comment at: test/CodeGen/x86-cf-protection.c:1 +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH alexfh wrote: > Any reason this test runs clang with "-S" and "-emit-llvm"? Neither of those > seems to be needed for the actual checks being made below. I agree that -emit-llvm is redundant for the test checks. Without -S the command has no output on stdout or stderr. So the run fails and doesn't execute. Repository: rC Clang https://reviews.llvm.org/D40478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322082 - Explicitly specify output file.
Author: alexfh Date: Tue Jan 9 07:05:13 2018 New Revision: 322082 URL: http://llvm.org/viewvc/llvm-project?rev=322082&view=rev Log: Explicitly specify output file. Otherwise the test fails when LLVM sources are on a read-only partition. Modified: cfe/trunk/test/CodeGen/x86-cf-protection.c Modified: cfe/trunk/test/CodeGen/x86-cf-protection.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/x86-cf-protection.c?rev=322082&r1=322081&r2=322082&view=diff == --- cfe/trunk/test/CodeGen/x86-cf-protection.c (original) +++ cfe/trunk/test/CodeGen/x86-cf-protection.c Tue Jan 9 07:05:13 2018 @@ -1,5 +1,5 @@ -// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN -// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -o %t -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -o %t -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH // RETURN: error: option 'cf-protection=return' cannot be specified without '-mshstk' // BRANCH: error: option 'cf-protection=branch' cannot be specified without '-mibt' ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40478: Added control flow architecture protection Flag
alexfh added inline comments. Comment at: test/CodeGen/x86-cf-protection.c:1 +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH oren_ben_simhon wrote: > alexfh wrote: > > Any reason this test runs clang with "-S" and "-emit-llvm"? Neither of > > those seems to be needed for the actual checks being made below. > I agree that -emit-llvm is redundant for the test checks. Without -S the > command has no output on stdout or stderr. So the run fails and doesn't > execute. The specific problem I've stumbled upon is that the test fails when the source file is on a read-only partition: ``` [3250/3251] Running the Clang regression tests llvm-lit: /src/utils/lit/lit/llvm/config.py:334: note: using clang: /build/bin/clang FAIL: Clang :: CodeGen/x86-cf-protection.c (2485 of 11862) TEST 'Clang :: CodeGen/x86-cf-protection.c' FAILED Script: -- not /build/bin/clang -cc1 -internal-isystem /build/lib/clang/7.0.0/include -nostdsysteminc -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return /src/tools/clang/test/CodeGen/x86-cf-protection.c 2>&1 | /build/bin/FileCheck /src/tools/clang/test/CodeGen/x86 -cf-protection.c --check-prefix=RETURN not /build/bin/clang -cc1 -internal-isystem /build/lib/clang/7.0.0/include -nostdsysteminc -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch /src/tools/clang/test/CodeGen/x86-cf-protection.c 2>&1 | /build/bin/FileCheck /src/tools/clang/test/CodeGen/x86 -cf-protection.c --check-prefix=BRANCH -- Exit Code: 1 Command Output (stderr): -- /src/tools/clang/test/CodeGen/x86-cf-protection.c:4:12: error: expected string not found in input // RETURN: error: option 'cf-protection=return' cannot be specified without '-mshstk' ^ :1:1: note: scanning from here error: unable to open output file '': 'Read-only file system' ^ -- ``` Adding `-o %t` solves the problem (r322082). Repository: rC Clang https://reviews.llvm.org/D40478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41823: [clangd] Add more filters for collected symbols.
ioeric updated this revision to Diff 129078.
ioeric marked 4 inline comments as done.
ioeric added a comment.
- Addressed comments.
- Merged with origin/master
- Merge with diff base.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41823
Files:
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/FileIndex.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/FileIndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -53,20 +53,22 @@
namespace {
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
- SymbolIndexActionFactory() = default;
+ SymbolIndexActionFactory(SymbolCollector::Options COpts)
+ : COpts(std::move(COpts)) {}
clang::FrontendAction *create() override {
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = false;
-Collector = std::make_shared();
+Collector = std::make_shared(COpts);
FrontendAction *Action =
index::createIndexingAction(Collector, IndexOpts, nullptr).release();
return Action;
}
std::shared_ptr Collector;
+ SymbolCollector::Options COpts;
};
class SymbolCollectorTest : public ::testing::Test {
@@ -79,7 +81,7 @@
const std::string FileName = "symbol.cc";
const std::string HeaderName = "symbols.h";
-auto Factory = llvm::make_unique();
+auto Factory = llvm::make_unique(CollectorOpts);
tooling::ToolInvocation Invocation(
{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName},
@@ -100,9 +102,11 @@
protected:
SymbolSlab Symbols;
+ SymbolCollector::Options CollectorOpts;
};
-TEST_F(SymbolCollectorTest, CollectSymbol) {
+TEST_F(SymbolCollectorTest, CollectSymbols) {
+ CollectorOpts.IndexMainFiles = true;
const std::string Header = R"(
class Foo {
void f();
@@ -144,7 +148,6 @@
EXPECT_THAT(Symbols,
UnorderedElementsAreArray(
{QName("Foo"),
- QName("Foo::f"),
QName("f1"),
QName("f2"),
QName("KInt"),
@@ -158,29 +161,84 @@
QName("foo::baz")}));
}
-TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
+ const std::string Header = R"(
+class Foo {};
+void f1();
+inline void f2() {}
+ )";
+ const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void main_f() {} // ignore
+void f1() {}
+ )";
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2")));
+}
+
+TEST_F(SymbolCollectorTest, IncludeSymbolsInMainFile) {
+ CollectorOpts.IndexMainFiles = true;
+ const std::string Header = R"(
+class Foo {};
+void f1();
+inline void f2() {}
+ )";
+ const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void main_f() {}
+void f1() {}
+ )";
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("f1"),
+QName("f2"), QName("main_f")));
+}
+
+TEST_F(SymbolCollectorTest, IgnoreClassMembers) {
+ const std::string Header = R"(
+class Foo {
+ void f() {}
+ void g();
+ static void sf() {}
+ static void ssf();
+ static int x;
+};
+ )";
const std::string Main = R"(
+void Foo::g() {}
+void Foo::ssf() {}
+ )";
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo")));
+}
+
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Header = R"(
namespace nx {
/// Foo comment.
int ff(int x, double y) { return 0; }
}
)";
- runSymbolCollector(/*Header=*/"", Main);
+ runSymbolCollector(Header, /*Main=*/"");
EXPECT_THAT(Symbols,
UnorderedElementsAre(QName("nx"),
AllOf(QName("nx::ff"),
Labeled("ff(int x, double y)"),
Detail("int"), Doc("Foo comment.";
}
TEST_F(SymbolCollectorTest, PlainAndSnippet) {
- const std::string Main = R"(
+ const std::string Header = R"(
namespace nx {
void f() {}
int ff(int x, double y) { return 0; }
}
)";
- runSymbolCollector(/*Header=*/"", Main);
+ runSymbolCollector(Header, /*Main=*/"");
EXPECT_THAT(
Symbols,
UnorderedElementsAre(
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTes
[PATCH] D41823: [clangd] Add more filters for collected symbols.
ioeric added a comment.
Thanks for the reviews!
Comment at: clangd/index/FileIndex.cpp:22
llvm::ArrayRef Decls) {
- auto Collector = std::make_shared();
+ auto CollectorOpts = SymbolCollector::Options();
+ CollectorOpts.IndexMainFiles = true;
ilya-biryukov wrote:
> NIT: why not `SymbolCollector::Options CollectorOpts`? It's seems more
> concise.
>
> Or even `SymbolCollector::Options{/*IndexMainFiles=*/true}`;
> The last one does not play nicely with moving the fields of the struct
> around, though
I think I had too much shared/unique pointers... :P
> The last one does not play nicely with moving the fields of the struct
> around, though
Yeah, this is the reason I didn't use initializer.
Comment at: clangd/index/SymbolCollector.cpp:80
+: decl(unless(isExpansionInMainFile())),
+hasDeclContext(anyOf(namespaceDecl(),
translationUnitDecl(),
+*D, *ASTCtx)
ilya-biryukov wrote:
> hokein wrote:
> > These ast matchers seems to be relatively simple, maybe we can use the
> > `Decl` methods to implement them at the moment. Or will we add more
> > complex filters in the future?
> I second this. Also the code would probably be more readable without matchers
> in this particular example.
Yes, I expect to borrow more matchers from include-fixer's find-all-symbols.
Also, although `isExpansionInMainFile` seems easy to implement, I am inclined
to reuse the existing code if possible.
Comment at: clangd/index/SymbolCollector.h:28
+ struct Options {
+bool IndexMainFiles = false;
+ };
ilya-biryukov wrote:
> hokein wrote:
> > We need documentation for the options.
> Also, would naming it `OnlyMainFiles` make the intent of the option clearer?
Actually, `OnlyMainFiles` is the opposite. We always index header files but
optionally index main files. I've added comment to clarify...
Comment at: unittests/clangd/SymbolCollectorTests.cpp:107
const std::string Header = R"(
-class Foo {
- void f();
-};
+class Foo {};
void f1();
ilya-biryukov wrote:
> Why do we remove `void f()`? Maybe assert that it's not in the list of
> symbols instead?
I was trying to limit the scope of each test case (there is a separate test
case `IgnoreClassMembers`). `UnorderedElementsAre` should be able to capture
unexpected symbols.
Comment at: unittests/clangd/SymbolCollectorTests.cpp:182
)";
- runSymbolCollector(/*Header=*/"", Main);
+ runSymbolCollector(Header, /*Main=*/"");
EXPECT_THAT(
ilya-biryukov wrote:
> Why should we change this test?
By default, main files are not indexed, so I put the code in header.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
hokein updated this revision to Diff 129081.
hokein marked 4 inline comments as done.
hokein added a comment.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41668
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/MemIndex.cpp
clangd/index/MemIndex.h
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -453,12 +453,6 @@
std::unique_ptr simpleIndexFromSymbols(
std::vector> Symbols) {
- auto I = llvm::make_unique();
- struct Snapshot {
-SymbolSlab Slab;
-std::vector Pointers;
- };
- auto Snap = std::make_shared();
SymbolSlab::Builder Slab;
for (const auto &Pair : Symbols) {
Symbol Sym;
@@ -475,13 +469,7 @@
Sym.SymInfo.Kind = Pair.second;
Slab.insert(Sym);
}
- Snap->Slab = std::move(Slab).build();
- for (auto &Iter : Snap->Slab)
-Snap->Pointers.push_back(&Iter);
- auto S = std::shared_ptr>(std::move(Snap),
-&Snap->Pointers);
- I->build(std::move(S));
- return std::move(I);
+ return MemIndex::build(std::move(Slab).build());
}
TEST(CompletionTest, NoIndex) {
@@ -496,6 +484,23 @@
EXPECT_THAT(Results.items, Has("No"));
}
+TEST(CompletionTest, StaticAndDynamicIndex) {
+ clangd::CodeCompleteOptions Opts;
+ auto StaticIdx =
+ simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class}});
+ Opts.StaticIndex = StaticIdx.get();
+ auto DynamicIdx =
+ simpleIndexFromSymbols({{"ns::foo", index::SymbolKind::Function}});
+ Opts.Index = DynamicIdx.get();
+
+ auto Results = completions(R"cpp(
+ void f() { ::ns::^ }
+ )cpp",
+ Opts);
+ EXPECT_THAT(Results.items, Contains(Labeled("[G]XYZ")));
+ EXPECT_THAT(Results.items, Contains(Labeled("foo")));
+}
+
TEST(CompletionTest, SimpleIndexBased) {
clangd::CodeCompleteOptions Opts;
auto I = simpleIndexFromSymbols({{"ns::XYZ", index::SymbolKind::Class},
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -11,6 +11,7 @@
#include "JSONRPCDispatcher.h"
#include "Path.h"
#include "Trace.h"
+#include "index/SymbolYAML.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
@@ -26,7 +27,24 @@
namespace {
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 = SymbolFromYAML(Buffer.get()->getBuffer());
+ SymbolSlab::Builder SymsBuilder;
+ for (auto Sym : Slab)
+SymsBuilder.insert(Sym);
+
+ return MemIndex::build(std::move(SymsBuilder).build());
}
+} // namespace
static llvm::cl::opt CompileCommandsDir(
"compile-commands-dir",
@@ -97,6 +115,15 @@
"use index built from symbols in opened files"),
llvm::cl::init(false), llvm::cl::Hidden);
+static llvm::cl::opt YamlSymbolFile(
+"yaml-symbol-file",
+llvm::cl::desc(
+"YAML-format global symbol file to build the static index. It is only "
+"available when 'enable-index-based-completion' is enabled.\n"
+"WARNING: This option is experimental only, and will be removed "
+"eventually. Don't rely on it."),
+llvm::cl::init(""), llvm::cl::Hidden);
+
int main(int argc, char *argv[]) {
llvm::cl::ParseCommandLineOptions(argc, argv, "clangd");
@@ -182,13 +209,16 @@
// Change stdin to binary to not lose \r\n on windows.
llvm::sys::ChangeStdinToBinary();
+ std::unique_ptr StaticIdx;
+ if (EnableIndexBasedCompletion && !YamlSymbolFile.empty())
+StaticIdx = BuildStaticIndex(YamlSymbolFile);
clangd::CodeCompleteOptions CCOpts;
CCOpts.EnableSnippets = EnableSnippets;
CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(Out, WorkerThreadsCount, StorePreamblesInMemory,
CCOpts, ResourceDirRef, CompileCommandsDirPath,
-EnableIndexBasedCompletion);
+EnableIndexBasedCompletion, StaticIdx.get());
constexpr int NoShutdownRequestErrorCode = 1;
llvm::set_thread_name("clangd.main");
return LSPServer.run(std::cin) ? 0 : NoShutdownRequestErrorCo
[clang-tools-extra] r322084 - [clangd] Use ToolExecutor to write the global-symbol-builder tool.
Author: ioeric
Date: Tue Jan 9 07:21:45 2018
New Revision: 322084
URL: http://llvm.org/viewvc/llvm-project?rev=322084&view=rev
Log:
[clangd] Use ToolExecutor to write the global-symbol-builder tool.
Summary:
This enables more execution modes like standalone and Mapreduce-style execution.
See also D41729
Reviewers: hokein, sammccall
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41730
Modified:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.h
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=322084&r1=322083&r2=322084&view=diff
==
---
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
(original)
+++
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Tue Jan 9 07:21:45 2018
@@ -17,15 +17,15 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
#include "clang/Frontend/FrontendActions.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
-#include "clang/Tooling/CommonOptionsParser.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Signals.h"
#include "llvm/Support/Path.h"
-#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
#include "llvm/Support/ThreadPool.h"
using namespace llvm;
@@ -37,18 +37,46 @@ namespace clangd {
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
- SymbolIndexActionFactory() = default;
+ SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
clang::FrontendAction *create() override {
+// Wraps the index action and reports collected symbols to the execution
+// context at the end of each translation unit.
+class WrappedIndexAction : public WrapperFrontendAction {
+public:
+ WrappedIndexAction(std::shared_ptr C,
+ const index::IndexingOptions &Opts,
+ tooling::ExecutionContext *Ctx)
+ : WrapperFrontendAction(
+index::createIndexingAction(C, Opts, nullptr)),
+Ctx(Ctx), Collector(C) {}
+
+ void EndSourceFileAction() override {
+WrapperFrontendAction::EndSourceFileAction();
+
+auto Symbols = Collector->takeSymbols();
+for (const auto &Sym : Symbols) {
+ std::string IDStr;
+ llvm::raw_string_ostream OS(IDStr);
+ OS << Sym.ID;
+ Ctx->reportResult(OS.str(), SymbolToYAML(Sym));
+}
+ }
+
+private:
+ tooling::ExecutionContext *Ctx;
+ std::shared_ptr Collector;
+};
+
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = false;
-Collector = std::make_shared();
-return index::createIndexingAction(Collector, IndexOpts,
nullptr).release();
+return new WrappedIndexAction(std::make_shared(),
+ IndexOpts, Ctx);
}
- std::shared_ptr Collector;
+ tooling::ExecutionContext *Ctx;
};
} // namespace clangd
@@ -61,50 +89,22 @@ int main(int argc, const char **argv) {
"This is an **experimental** tool to generate YAML-format "
"project-wide symbols for clangd (global code completion). It would be "
"changed and deprecated eventually. Don't use it in production code!";
- CommonOptionsParser OptionsParser(argc, argv, cl::GeneralCategory,
-/*Overview=*/Overview);
+ auto Executor = clang::tooling::createExecutorFromCommandLineArgs(
+ argc, argv, cl::GeneralCategory, Overview);
- // No compilation database found, fallback to single TU analysis, this is
- // mainly for debugging purpose:
- // global-symbol-buidler /tmp/t.cc -- -std=c++11.
- if (OptionsParser.getCompilations().getAllFiles().empty()) {
-llvm::errs() << "No compilation database found, processing individual "
-"files with flags from command-line\n.";
-ClangTool Tool(OptionsParser.getCompilations(),
- OptionsParser.getSourcePathList());
-clang::clangd::SymbolIndexActionFactory IndexAction;
-Tool.run(&IndexAction);
-llvm::outs() << SymbolToYAML(IndexAction.Collector->takeSymbols());
-return 0;
+ if (!Executor) {
+llvm::errs() << l
[PATCH] D41730: [clangd] Use ToolExecutor to write the global-symbol-builder tool.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL322084: [clangd] Use ToolExecutor to write the
global-symbol-builder tool. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D41730
Files:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.h
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -185,7 +185,7 @@
UnorderedElementsAre(QName("clang::Foo2")));
std::string ConcatenatedYAML =
- SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
+ SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);
auto ConcatenatedSymbols = SymbolFromYAML(ConcatenatedYAML);
EXPECT_THAT(ConcatenatedSymbols,
UnorderedElementsAre(QName("clang::Foo1"),
Index: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -17,15 +17,15 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
#include "clang/Frontend/FrontendActions.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Execution.h"
#include "clang/Tooling/Tooling.h"
-#include "clang/Tooling/CommonOptionsParser.h"
+#include "llvm/Support/CommandLine.h"
#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Signals.h"
#include "llvm/Support/Path.h"
-#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Signals.h"
#include "llvm/Support/ThreadPool.h"
using namespace llvm;
@@ -37,18 +37,46 @@
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
- SymbolIndexActionFactory() = default;
+ SymbolIndexActionFactory(tooling::ExecutionContext *Ctx) : Ctx(Ctx) {}
clang::FrontendAction *create() override {
+// Wraps the index action and reports collected symbols to the execution
+// context at the end of each translation unit.
+class WrappedIndexAction : public WrapperFrontendAction {
+public:
+ WrappedIndexAction(std::shared_ptr C,
+ const index::IndexingOptions &Opts,
+ tooling::ExecutionContext *Ctx)
+ : WrapperFrontendAction(
+index::createIndexingAction(C, Opts, nullptr)),
+Ctx(Ctx), Collector(C) {}
+
+ void EndSourceFileAction() override {
+WrapperFrontendAction::EndSourceFileAction();
+
+auto Symbols = Collector->takeSymbols();
+for (const auto &Sym : Symbols) {
+ std::string IDStr;
+ llvm::raw_string_ostream OS(IDStr);
+ OS << Sym.ID;
+ Ctx->reportResult(OS.str(), SymbolToYAML(Sym));
+}
+ }
+
+private:
+ tooling::ExecutionContext *Ctx;
+ std::shared_ptr Collector;
+};
+
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = false;
-Collector = std::make_shared();
-return index::createIndexingAction(Collector, IndexOpts, nullptr).release();
+return new WrappedIndexAction(std::make_shared(),
+ IndexOpts, Ctx);
}
- std::shared_ptr Collector;
+ tooling::ExecutionContext *Ctx;
};
} // namespace clangd
@@ -61,50 +89,22 @@
"This is an **experimental** tool to generate YAML-format "
"project-wide symbols for clangd (global code completion). It would be "
"changed and deprecated eventually. Don't use it in production code!";
- CommonOptionsParser OptionsParser(argc, argv, cl::GeneralCategory,
-/*Overview=*/Overview);
+ auto Executor = clang::tooling::createExecutorFromCommandLineArgs(
+ argc, argv, cl::GeneralCategory, Overview);
- // No compilation database found, fallback to single TU analysis, this is
- // mainly for debugging purpose:
- // global-symbol-buidler /tmp/t.cc -- -std=c++11.
- if (OptionsParser.getCompilations().getAllFiles().empty()) {
-llvm::errs() << "No compilation database found, processing individual "
-"files with flags from command-line\n.";
-ClangTool Tool(OptionsParser.getCompilations(),
- OptionsParser.g
Re: Re: r322018 - [OPENMP] Current status of OpenMP support.
This is is the status of OpenMP support in 6.0. There is nothing new
since branching.
-
Best regards,
Alexey Bataev
08.01.2018 14:09, Jonas Hahnfeld via cfe-commits пишет:
> Can we backport this page to release_60? I think the documented
> support is also valid for 6.0 or did I miss recent commits that added
> support for new directives / clauses?
>
> Am 2018-01-08 20:02, schrieb Alexey Bataev via cfe-commits:
>> Author: abataev
>> Date: Mon Jan 8 11:02:51 2018
>> New Revision: 322018
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=322018&view=rev
>> Log:
>> [OPENMP] Current status of OpenMP support.
>>
>> Summary: Some info about supported features of OpenMP 4.5-5.0.
>>
>> Reviewers: hfinkel, rsmith
>>
>> Subscribers: kkwli0, Hahnfeld, cfe-commits
>>
>> Differential Revision: https://reviews.llvm.org/D39457
>>
>> Added:
>> Â Â Â cfe/trunk/docs/OpenMPSupport.rst
>> Modified:
>> Â Â Â cfe/trunk/docs/index.rst
>>
>> Added: cfe/trunk/docs/OpenMPSupport.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/OpenMPSupport.rst?rev=322018&view=auto
>>
>> ==
>>
>> --- cfe/trunk/docs/OpenMPSupport.rst (added)
>> +++ cfe/trunk/docs/OpenMPSupport.rst Mon Jan 8 11:02:51 2018
>> @@ -0,0 +1,68 @@
>> +.. raw:: html
>> +
>> +Â
>> +Â Â Â .none { background-color: #FF }
>> +Â Â Â .partial { background-color: #99 }
>> +Â Â Â .good { background-color: #CCFF99 }
>> +Â
>> +
>> +.. role:: none
>> +.. role:: partial
>> +.. role:: good
>> +
>> +==
>> +OpenMP Support
>> +==
>> +
>> +Clang fully supports OpenMP 3.1 + some elements of OpenMP 4.5. Clang
>> supports offloading to X86_64, AArch64 and PPC64[LE] devices.
>> +Support for Cuda devices is not ready yet.
>> +The status of major OpenMP 4.5 features support in Clang.
>> +
>> +Standalone directives
>> +=
>> +
>> +* #pragma omp [for] simd: :good:`Complete`.
>> +
>> +* #pragma omp declare simd: :partial:`Partial`. We support
>> parsing/semantic
>> +Â analysis + generation of special attributes for X86 target, but still
>> +Â missing the LLVM pass for vectorization.
>> +
>> +* #pragma omp taskloop [simd]: :good:`Complete`.
>> +
>> +* #pragma omp target [enter|exit] data: :good:`Complete`.
>> +
>> +* #pragma omp target update: :good:`Complete`.
>> +
>> +* #pragma omp target: :partial:`Partial`. No support for the
>> `depend` clauses.
>> +
>> +* #pragma omp declare target: :partial:`Partial`. No full codegen
>> support.
>> +
>> +* #pragma omp teams: :good:`Complete`.
>> +
>> +* #pragma omp distribute [simd]: :good:`Complete`.
>> +
>> +* #pragma omp distribute parallel for [simd]: :good:`Complete`.
>> +
>> +Combined directives
>> +===
>> +
>> +* #pragma omp parallel for simd: :good:`Complete`.
>> +
>> +* #pragma omp target parallel: :partial:`Partial`. No support for
>> the `depend` clauses.
>> +
>> +* #pragma omp target parallel for [simd]: :partial:`Partial`. No
>> support for the `depend` clauses.
>> +
>> +* #pragma omp target simd: :partial:`Partial`. No support for the
>> `depend` clauses.
>> +
>> +* #pragma omp target teams: :partial:`Partial`. No support for the
>> `depend` clauses.
>> +
>> +* #pragma omp teams distribute [simd]: :good:`Complete`.
>> +
>> +* #pragma omp target teams distribute [simd]: :partial:`Partial`. No
>> support for the and `depend` clauses.
>> +
>> +* #pragma omp teams distribute parallel for [simd]: :good:`Complete`.
>> +
>> +* #pragma omp target teams distribute parallel for [simd]:
>> :partial:`Partial`. No full codegen support.
>> +
>> +Clang does not support any constructs/updates from upcoming OpenMP
>> 5.0 except for `reduction`-based clauses in the `task` and
>> `target`-based directives.
>> +
>>
>> Modified: cfe/trunk/docs/index.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/index.rst?rev=322018&r1=322017&r2=322018&view=diff
>>
>> ==
>>
>> --- cfe/trunk/docs/index.rst (original)
>> +++ cfe/trunk/docs/index.rst Mon Jan 8 11:02:51 2018
>> @@ -39,6 +39,7 @@ Using Clang as a Compiler
>> Â Â Â SourceBasedCodeCoverage
>> Â Â Â Modules
>> Â Â Â MSVCCompatibility
>> +Â Â OpenMPSupport
>> Â Â Â ThinLTO
>> Â Â Â CommandGuide/index
>> Â Â Â FAQ
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
signature.asc
Description: OpenPGP digital signature
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322017 - [index] Return when DC is null in handleReference
Author: maskray
Date: Mon Jan 8 10:57:38 2018
New Revision: 322017
URL: http://llvm.org/viewvc/llvm-project?rev=322017&view=rev
Log:
[index] Return when DC is null in handleReference
Summary:
DC may sometimes be NULL and getContainerInfo(DC, Container) will dereference a
null pointer.
Default template arguments (the following example and many test files in
https://github.com/nlohmann/json)
may cause null pointer dereference.
```c++
template
struct actor;
template class Actor = actor>
struct terminal;
```
In tools/libclang/CXIndexDataConsumer.cpp#L203
handleReference(ND, Loc, Cursor,
dyn_cast_or_null(ASTNode.Parent),
ASTNode.ContainerDC, ASTNode.OrigE, Kind);
`dyn_cast_or_null(ASTNode.Parent)` is somehow a null pointer and in
tools/libclang/CXIndexDataConsumer.cpp:935
ContainerInfo Container;
getContainerInfo(DC, Container);
The null DC is casted `ContInfo.cursor = getCursor(cast(DC));` and
SIGSEGV.
```
See discussions in https://github.com/jacobdufault/cquery/issues/219
https://github.com/jacobdufault/cquery/issues/192
Reviewers: akyrtzi, sammccall, yvvan
Reviewed By: sammccall
Subscribers: mehdi_amini, cfe-commits
Differential Revision: https://reviews.llvm.org/D41575
Modified:
cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp
Modified: cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp?rev=322017&r1=322016&r2=322017&view=diff
==
--- cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp (original)
+++ cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp Mon Jan 8 10:57:38 2018
@@ -890,7 +890,7 @@ bool CXIndexDataConsumer::handleReferenc
const DeclContext *DC,
const Expr *E,
CXIdxEntityRefKind Kind) {
- if (!D)
+ if (!D || !DC)
return false;
CXCursor Cursor = E ? MakeCXCursor(E, cast(DC), CXTU)
@@ -907,7 +907,7 @@ bool CXIndexDataConsumer::handleReferenc
if (!CB.indexEntityReference)
return false;
- if (!D)
+ if (!D || !DC)
return false;
if (Loc.isInvalid())
return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40478: Added control flow architecture protection Flag
oren_ben_simhon marked an inline comment as done. oren_ben_simhon added inline comments. Comment at: test/CodeGen/x86-cf-protection.c:1 +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN +// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH alexfh wrote: > oren_ben_simhon wrote: > > alexfh wrote: > > > Any reason this test runs clang with "-S" and "-emit-llvm"? Neither of > > > those seems to be needed for the actual checks being made below. > > I agree that -emit-llvm is redundant for the test checks. Without -S the > > command has no output on stdout or stderr. So the run fails and doesn't > > execute. > The specific problem I've stumbled upon is that the test fails when the > source file is on a read-only partition: > ``` > [3250/3251] Running the Clang regression tests > llvm-lit: /src/utils/lit/lit/llvm/config.py:334: note: using clang: > /build/bin/clang > FAIL: Clang :: CodeGen/x86-cf-protection.c (2485 of 11862) > TEST 'Clang :: CodeGen/x86-cf-protection.c' FAILED > > Script: > -- > not /build/bin/clang -cc1 -internal-isystem /build/lib/clang/7.0.0/include > -nostdsysteminc -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown > -fcf-protection=return /src/tools/clang/test/CodeGen/x86-cf-protection.c 2>&1 > | /build/bin/FileCheck /src/tools/clang/test/CodeGen/x86 > -cf-protection.c --check-prefix=RETURN > not /build/bin/clang -cc1 -internal-isystem /build/lib/clang/7.0.0/include > -nostdsysteminc -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown > -fcf-protection=branch /src/tools/clang/test/CodeGen/x86-cf-protection.c 2>&1 > | /build/bin/FileCheck /src/tools/clang/test/CodeGen/x86 > -cf-protection.c --check-prefix=BRANCH > -- > Exit Code: 1 > > Command Output (stderr): > -- > /src/tools/clang/test/CodeGen/x86-cf-protection.c:4:12: error: expected > string not found in input > // RETURN: error: option 'cf-protection=return' cannot be specified without > '-mshstk' >^ > :1:1: note: scanning from here > error: unable to open output file '': 'Read-only file system' > ^ > > -- > ``` > Adding `-o %t` solves the problem (r322082). I see. Thanks for the fix. Repository: rC Clang https://reviews.llvm.org/D40478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41553: Support parsing double square-bracket attributes in ObjC
aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:239 + bit IncludeC = includeC; +} rjmccall wrote: > aaron.ballman wrote: > > rjmccall wrote: > > > I have no objection to allowing ObjC attributes to be spelled in > > > [[clang::objc_whatever]] style. We can debate giving them a more > > > appropriate standard name in the objc namespace at a later time — or even > > > the primary namespace, if we really want to flex our Somewhat Major > > > Language muscles. > > > > > > I feel like this IncludeC is not the best name for this tablegen > > > property. Perhaps AllowInC? > > > > > > Also, a random positional 1 argument is pretty obscure TableGen design. > > > Do we really expect to be making very many attributes that intentionally > > > disallow C2x? What would these be, just C++-specific attributes without > > > C analogues? It doesn't seem important to prohibit those at the parsing > > > level rather than at the semantic level, since, after all, we are still > > > allowing the GNU-style spelling, and these would still qualified with > > > ``clang::``. > > > > > > I would suggest standardizing in the opposite direction: instead of > > > forcing attributes to opt in to being allowed in C, we should make > > > attributes that we really don't want to allow in the C2x [[clang::]] > > > namespace be explicit about it. If there are a lot of C++-specific > > > attributes like that, we can make a ClangCXXOnly subclass. > > > I have no objection to allowing ObjC attributes to be spelled in > > > [[clang::objc_whatever]] style. We can debate giving them a more > > > appropriate standard name in the objc namespace at a later time — or even > > > the primary namespace, if we really want to flex our Somewhat Major > > > Language muscles. > > > > Thanks -- are you okay with where the attributes are allowed in the syntax? > > I tried to follow the position they're allowed in C and C++ as closely as I > > could, but having confirmation would be nice. > > > > As for putting the attributes in the primary namespace, that would be > > reasonable for using the attributes in ObjC or ObjC++ but not so much for > > using the same attributes in a pure C or C++ compilation. > > > > > I feel like this IncludeC is not the best name for this tablegen > > > property. Perhaps AllowInC? > > > > I'm fine with that name. I'll change it in D41317 when I commit that. > > > > > Also, a random positional 1 argument is pretty obscure TableGen design. > > > Do we really expect to be making very many attributes that intentionally > > > disallow C2x? What would these be, just C++-specific attributes without C > > > analogues? > > > > I agree -- I was toying with using a named entity rather than a numeric > > literal, but I wanted to see how the design shook out in practice once I > > had a better feel for how many attributes are impacted. I'm glad you > > recommend going the opposite direction as that was my ultimate goal. :-) > > Basically, my initial approach is to disallow everything in C and then > > start enabling the attributes that make sense. At some point, I believe > > we'll have more attributes in both language modes than not, and then I plan > > to reverse the meaning of the flag so that an attribute has to opt-out > > rather than opt-in. I don't expect we'll have many attributes that are > > disallowed in C2x. I think they'll fall into two categories: C++ attributes > > that don't make sense in C and attributes that are governed by some other > > body. > > > > > It doesn't seem important to prohibit those at the parsing level rather > > > than at the semantic level, since, after all, we are still allowing the > > > GNU-style spelling, and these would still qualified with `clang::`. > > > > I think it's important for C targets when > > `-fdouble-square-bracket-attributes` is not enabled, as the attributes > > cannot syntactically appear in those positions. > > > > > I would suggest standardizing in the opposite direction > > > > I suspect I will ultimately get there. I chose this approach to be more > > conservative about what we expose. > > Thanks -- are you okay with where the attributes are allowed in the syntax? > > I tried to follow the position they're allowed in C and C++ as closely as I > > could, but having confirmation would be nice. > > I'll leave comments on the various places. For the most part, no, I think > these are the wrong places; where we allow attributes in ObjC is actually > pretty carefully thought-out already, and it's better to follow the places > where we parse GNU attributes than to try to imitate the C grammar. > > > As for putting the attributes in the primary namespace, that would be > > reasonable for using the attributes in ObjC or ObjC++ but not so much for > > using the same attributes in a pure C or C++ compilation. > > Yes, please ignore that. I was just idly thin
[PATCH] D41867: [Frontend] Remove unused FileMgr in pp arg parse
modocache created this revision.
modocache added a reviewer: v.g.vassilev.
A FIXME added 8 years ago (2010) in https://reviews.llvm.org/rL118203
mentioned that a FileManager should not need to be used when parsing
preprocessor arguments. In fact, its only use was removed 6 years ago
(2012), in https://reviews.llvm.org/rL166452. Remove the unused
variable and the obsolete FIXME.
Test Plan: `check-clang`
Repository:
rC Clang
https://reviews.llvm.org/D41867
Files:
lib/Frontend/CompilerInvocation.cpp
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2618,7 +2618,6 @@
}
static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
- FileManager &FileMgr,
DiagnosticsEngine &Diags,
frontend::ActionKind Action) {
using namespace options;
@@ -2840,12 +2839,7 @@
!LangOpts.Sanitize.has(SanitizerKind::Address) &&
!LangOpts.Sanitize.has(SanitizerKind::Memory);
- // FIXME: ParsePreprocessorArgs uses the FileManager to read the contents of
- // PCH file and find the original header name. Remove the need to do that in
- // ParsePreprocessorArgs and remove the FileManager
- // parameters from the function and the "FileManager.h" #include.
- FileManager FileMgr(Res.getFileSystemOpts());
- ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, FileMgr, Diags,
+ ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, Diags,
Res.getFrontendOpts().ProgramAction);
ParsePreprocessorOutputArgs(Res.getPreprocessorOutputOpts(), Args,
Res.getFrontendOpts().ProgramAction);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2618,7 +2618,6 @@
}
static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
- FileManager &FileMgr,
DiagnosticsEngine &Diags,
frontend::ActionKind Action) {
using namespace options;
@@ -2840,12 +2839,7 @@
!LangOpts.Sanitize.has(SanitizerKind::Address) &&
!LangOpts.Sanitize.has(SanitizerKind::Memory);
- // FIXME: ParsePreprocessorArgs uses the FileManager to read the contents of
- // PCH file and find the original header name. Remove the need to do that in
- // ParsePreprocessorArgs and remove the FileManager
- // parameters from the function and the "FileManager.h" #include.
- FileManager FileMgr(Res.getFileSystemOpts());
- ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, FileMgr, Diags,
+ ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, Diags,
Res.getFrontendOpts().ProgramAction);
ParsePreprocessorOutputArgs(Res.getPreprocessorOutputOpts(), Args,
Res.getFrontendOpts().ProgramAction);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value
khuttun updated this revision to Diff 129090.
khuttun added a comment.
The checker is now disabled inside GNU statement expressions
https://reviews.llvm.org/D41655
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/UnusedReturnValueCheck.cpp
clang-tidy/bugprone/UnusedReturnValueCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-unused-return-value.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-unused-return-value-custom.cpp
test/clang-tidy/bugprone-unused-return-value.cpp
Index: test/clang-tidy/bugprone-unused-return-value.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-unused-return-value.cpp
@@ -0,0 +1,212 @@
+// RUN: %check_clang_tidy %s bugprone-unused-return-value %t
+
+namespace std {
+
+using size_t = decltype(sizeof(int));
+
+using max_align_t = long double;
+
+struct future {};
+
+enum class launch {
+ async,
+ deferred
+};
+
+template
+future async(Function &&, Args &&...);
+
+template
+future async(launch, Function &&, Args &&...);
+
+template
+bool empty(const T &);
+
+// the check should be able to match std lib calls even if the functions are
+// declared inside inline namespaces
+inline namespace v1 {
+
+template
+T *launder(T *);
+
+} // namespace v1
+
+template
+struct allocator {
+ using value_type = T;
+ T *allocate(std::size_t);
+ T *allocate(std::size_t, const void *);
+};
+
+template
+struct allocator_traits {
+ using value_type = typename Alloc::value_type;
+ using pointer = value_type *;
+ using size_type = size_t;
+ using const_void_pointer = const void *;
+ static pointer allocate(Alloc &, size_type);
+ static pointer allocate(Alloc &, size_type, const_void_pointer);
+};
+
+template
+struct scoped_allocator_adaptor : public OuterAlloc {
+ using pointer = typename allocator_traits::pointer;
+ using size_type = typename allocator_traits::size_type;
+ using const_void_pointer = typename allocator_traits::const_void_pointer;
+ pointer allocate(size_type);
+ pointer allocate(size_type, const_void_pointer);
+};
+
+template
+struct default_delete {};
+
+template >
+struct unique_ptr {
+ using pointer = T *;
+ pointer release() noexcept;
+};
+
+template >
+struct vector {
+ bool empty() const noexcept;
+};
+
+namespace filesystem {
+
+struct path {
+ bool empty() const noexcept;
+};
+
+} // namespace filesystem
+
+namespace pmr {
+
+struct memory_resource {
+ void *allocate(size_t, size_t = alignof(max_align_t));
+};
+
+template
+struct polymorphic_allocator {
+ T *allocate(size_t);
+};
+
+} // namespace pmr
+
+} // namespace std
+
+struct Foo {
+ void f();
+};
+
+int increment(int i) {
+ return i + 1;
+}
+
+void useFuture(const std::future &fut);
+
+struct FooAlloc {
+ using value_type = Foo;
+};
+
+void warning() {
+ std::async(increment, 42);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::async(std::launch::async, increment, 42);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ Foo F;
+ std::launder(&F);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::unique_ptr UP;
+ UP.release();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::allocator FA;
+ FA.allocate(1);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+ FA.allocate(1, nullptr);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::allocator_traits>::allocate(FA, 1);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+ std::allocator_traits>::allocate(FA, 1, nullptr);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::scoped_allocator_adaptor SAA;
+ SAA.allocate(1);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+ SAA.allocate(1, nullptr);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::pmr::memory_resource MR;
+ MR.allocate(1);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::pmr::polymorphic_allocator PA;
+ PA.allocate(1);
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
+
+ std::vector FV;
+ FV.empty();
+ // CHECK
[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment
alexfh added a comment. A high-level comment: NOLINT category parsing and warning on incorrect NOLINT categories makes it more difficult for code to comply both with clang-tidy and cpplint (https://github.com/cpplint/cpplint), since: 1. the category names in these tools are different, 2. cpplint currently doesn't support more than one category with the `// NOLINT(category)` syntax, 3. cpplint complains on `// NOLINT` directives with an unknown category. So when suppressing a warning that is present in both clang-tidy and cpplint (e.g. google-runtime-int) one has to use `// NOLINT` without categories, which kind of undermines the effort of allowing category names explicitly specified in suppression directives. There are multiple solutions possible. One would be to add a clang-tidy-specific suppression directive and only support specifying check names in it (`// NOCLANGTIDY` seems verbose, but I don't have a better name. Suggestions are welcome.). Leaving support for `// NOLINT` without category names would be nice for suppressing warnings common with cpplint. Another option would be changing cpplint to somehow recognize and ignore clang-tidy check names in the suppressed categories list. Comment at: clang-tidy/ClangTidy.cpp:402 + if (Context.isCheckEnabled(NolintCheckName)) +CheckNames.push_back(NolintCheckName); + clang-format, please Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322091 - [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr
Author: a.sidorin
Date: Tue Jan 9 08:40:40 2018
New Revision: 322091
URL: http://llvm.org/viewvc/llvm-project?rev=322091&view=rev
Log:
[ASTImporter] Support importing CXXUnresolvedConstructExpr and
UnresolvedLookupExpr
* Note: This solution is based on
https://github.com/haoNoQ/clang/blob/summary-ipa-draft/lib/AST/ASTImporter.cpp#L7605.
Patch by Peter Szecsi!
Differential Revision: https://reviews.llvm.org/D38694
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=322091&r1=322090&r2=322091&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Jan 9 08:40:40 2018
@@ -287,6 +287,8 @@ namespace clang {
Expr *VisitCXXConstructExpr(CXXConstructExpr *E);
Expr *VisitCXXMemberCallExpr(CXXMemberCallExpr *E);
Expr *VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E);
+Expr *VisitCXXUnresolvedConstructExpr(CXXUnresolvedConstructExpr *CE);
+Expr *VisitUnresolvedLookupExpr(UnresolvedLookupExpr *E);
Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
Expr *VisitCXXThisExpr(CXXThisExpr *E);
Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
@@ -5887,6 +5889,65 @@ Expr *ASTNodeImporter::VisitCXXDependent
cast_or_null(ToFQ), MemberNameInfo, ResInfo);
}
+Expr *ASTNodeImporter::VisitCXXUnresolvedConstructExpr(
+CXXUnresolvedConstructExpr *CE) {
+
+ unsigned NumArgs = CE->arg_size();
+
+ llvm::SmallVector ToArgs(NumArgs);
+ if (ImportArrayChecked(CE->arg_begin(), CE->arg_end(), ToArgs.begin()))
+return nullptr;
+
+ return CXXUnresolvedConstructExpr::Create(
+ Importer.getToContext(), Importer.Import(CE->getTypeSourceInfo()),
+ Importer.Import(CE->getLParenLoc()), llvm::makeArrayRef(ToArgs),
+ Importer.Import(CE->getRParenLoc()));
+}
+
+Expr *ASTNodeImporter::VisitUnresolvedLookupExpr(UnresolvedLookupExpr *E) {
+ CXXRecordDecl *NamingClass =
+ cast_or_null(Importer.Import(E->getNamingClass()));
+ if (E->getNamingClass() && !NamingClass)
+return nullptr;
+
+ DeclarationName Name = Importer.Import(E->getName());
+ if (E->getName() && !Name)
+return nullptr;
+
+ DeclarationNameInfo NameInfo(Name, Importer.Import(E->getNameLoc()));
+ // Import additional name location/type info.
+ ImportDeclarationNameLoc(E->getNameInfo(), NameInfo);
+
+ UnresolvedSet<8> ToDecls;
+ for (Decl *D : E->decls()) {
+if (NamedDecl *To = cast_or_null(Importer.Import(D)))
+ ToDecls.addDecl(To);
+else
+ return nullptr;
+ }
+
+ TemplateArgumentListInfo ToTAInfo(Importer.Import(E->getLAngleLoc()),
+Importer.Import(E->getRAngleLoc()));
+ TemplateArgumentListInfo *ResInfo = nullptr;
+ if (E->hasExplicitTemplateArgs()) {
+if (ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+ return nullptr;
+ResInfo = &ToTAInfo;
+ }
+
+ if (ResInfo || E->getTemplateKeywordLoc().isValid())
+return UnresolvedLookupExpr::Create(
+Importer.getToContext(), NamingClass,
+Importer.Import(E->getQualifierLoc()),
+Importer.Import(E->getTemplateKeywordLoc()), NameInfo,
E->requiresADL(),
+ResInfo, ToDecls.begin(), ToDecls.end());
+
+ return UnresolvedLookupExpr::Create(
+ Importer.getToContext(), NamingClass,
+ Importer.Import(E->getQualifierLoc()), NameInfo, E->requiresADL(),
+ E->isOverloaded(), ToDecls.begin(), ToDecls.end());
+}
+
Expr *ASTNodeImporter::VisitCallExpr(CallExpr *E) {
QualType T = Importer.Import(E->getType());
if (T.isNull())
Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=322091&r1=322090&r2=322091&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue Jan 9 08:40:40 2018
@@ -656,5 +656,40 @@ TEST(ImportDecl, ImportUsingShadowDecl)
namespaceDecl(has(usingShadowDecl(;
}
+TEST(ImportExpr, ImportUnresolvedLookupExpr) {
+ MatchVerifier Verifier;
+ testImport("template int foo();"
+ "template void declToImport() {"
+ " ::foo;"
+ " ::template foo;"
+ "}"
+ "void instantiate() { declToImport(); }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(has(functionDecl(has(
+ compoundStmt(has(unresolvedLookupExpr(;
+}
+
+TEST(ImportExpr, ImportCXXUnresolvedConstructExpr) {
+ MatchVerifier Verifier;
+ testImport("template class C { T t; };"
+ "template void declToImport() {"
+ " C d;"
+ " d.t = T();"
[PATCH] D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr
This revision was automatically updated to reflect the committed changes.
Closed by commit rC322091: [ASTImporter] Support importing
CXXUnresolvedConstructExpr and… (authored by a.sidorin, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D38694?vs=127283&id=129095#toc
Repository:
rC Clang
https://reviews.llvm.org/D38694
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -287,6 +287,8 @@
Expr *VisitCXXConstructExpr(CXXConstructExpr *E);
Expr *VisitCXXMemberCallExpr(CXXMemberCallExpr *E);
Expr *VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E);
+Expr *VisitCXXUnresolvedConstructExpr(CXXUnresolvedConstructExpr *CE);
+Expr *VisitUnresolvedLookupExpr(UnresolvedLookupExpr *E);
Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
Expr *VisitCXXThisExpr(CXXThisExpr *E);
Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
@@ -5887,6 +5889,65 @@
cast_or_null(ToFQ), MemberNameInfo, ResInfo);
}
+Expr *ASTNodeImporter::VisitCXXUnresolvedConstructExpr(
+CXXUnresolvedConstructExpr *CE) {
+
+ unsigned NumArgs = CE->arg_size();
+
+ llvm::SmallVector ToArgs(NumArgs);
+ if (ImportArrayChecked(CE->arg_begin(), CE->arg_end(), ToArgs.begin()))
+return nullptr;
+
+ return CXXUnresolvedConstructExpr::Create(
+ Importer.getToContext(), Importer.Import(CE->getTypeSourceInfo()),
+ Importer.Import(CE->getLParenLoc()), llvm::makeArrayRef(ToArgs),
+ Importer.Import(CE->getRParenLoc()));
+}
+
+Expr *ASTNodeImporter::VisitUnresolvedLookupExpr(UnresolvedLookupExpr *E) {
+ CXXRecordDecl *NamingClass =
+ cast_or_null(Importer.Import(E->getNamingClass()));
+ if (E->getNamingClass() && !NamingClass)
+return nullptr;
+
+ DeclarationName Name = Importer.Import(E->getName());
+ if (E->getName() && !Name)
+return nullptr;
+
+ DeclarationNameInfo NameInfo(Name, Importer.Import(E->getNameLoc()));
+ // Import additional name location/type info.
+ ImportDeclarationNameLoc(E->getNameInfo(), NameInfo);
+
+ UnresolvedSet<8> ToDecls;
+ for (Decl *D : E->decls()) {
+if (NamedDecl *To = cast_or_null(Importer.Import(D)))
+ ToDecls.addDecl(To);
+else
+ return nullptr;
+ }
+
+ TemplateArgumentListInfo ToTAInfo(Importer.Import(E->getLAngleLoc()),
+Importer.Import(E->getRAngleLoc()));
+ TemplateArgumentListInfo *ResInfo = nullptr;
+ if (E->hasExplicitTemplateArgs()) {
+if (ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+ return nullptr;
+ResInfo = &ToTAInfo;
+ }
+
+ if (ResInfo || E->getTemplateKeywordLoc().isValid())
+return UnresolvedLookupExpr::Create(
+Importer.getToContext(), NamingClass,
+Importer.Import(E->getQualifierLoc()),
+Importer.Import(E->getTemplateKeywordLoc()), NameInfo, E->requiresADL(),
+ResInfo, ToDecls.begin(), ToDecls.end());
+
+ return UnresolvedLookupExpr::Create(
+ Importer.getToContext(), NamingClass,
+ Importer.Import(E->getQualifierLoc()), NameInfo, E->requiresADL(),
+ E->isOverloaded(), ToDecls.begin(), ToDecls.end());
+}
+
Expr *ASTNodeImporter::VisitCallExpr(CallExpr *E) {
QualType T = Importer.Import(E->getType());
if (T.isNull())
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -656,5 +656,40 @@
namespaceDecl(has(usingShadowDecl(;
}
+TEST(ImportExpr, ImportUnresolvedLookupExpr) {
+ MatchVerifier Verifier;
+ testImport("template int foo();"
+ "template void declToImport() {"
+ " ::foo;"
+ " ::template foo;"
+ "}"
+ "void instantiate() { declToImport(); }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(has(functionDecl(has(
+ compoundStmt(has(unresolvedLookupExpr(;
+}
+
+TEST(ImportExpr, ImportCXXUnresolvedConstructExpr) {
+ MatchVerifier Verifier;
+ testImport("template class C { T t; };"
+ "template void declToImport() {"
+ " C d;"
+ " d.t = T();"
+ "}"
+ "void instantiate() { declToImport(); }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(has(functionDecl(has(compoundStmt(has(
+ binaryOperator(has(cxxUnresolvedConstructExpr());
+ testImport("template class C { T t; };"
+ "template void declToImport() {"
+ " C d;"
+ " (&d)->t = T();"
+ "}"
+ "void instantiate() { declToImport(); }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(has(functionDecl(has(compoundStmt(h
[PATCH] D41797: [analyzer] Suppress escape of this-pointer during construction.
a.sidorin added a comment. Hi Artem, I think that global suppression is fine. If one really wants to check such escapes, he can implement a separate callback for this. Repository: rC Clang https://reviews.llvm.org/D41797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41406: [analyzer] Add a new checker callback, check::NewAllocator.
NoQ added a comment. In https://reviews.llvm.org/D41406#970985, @xazax.hun wrote: > Also, I was wondering if it would make sense to only have the PostCall > callback return the casted memory region in these specific cases instead of > introducing a new callback. Yep, i actually think it's a very good idea. Like, it was obvious to me that it's a bad idea because `PostCall` and `CallEvent` should describe the function call just like before - in particular, the moment they capture and the return value should correspond to the actual value returned from the function. And it's kinda obvious by now that there are two different values, both of which may be of interest to the user, and certain amount of modeling (namely, pointer cast) occurs between these two moments. But i guess we could also squeeze both values into `CallEvent`. Like, make a new `CallEvent` sub-class called, say, `CXXNewAllocatorCall`, which would additionally provide a method, say, `.getAllocatedObject()`, which would re-evaluate the cast and return the casted object. The whole reusable casting procedure would in this case live in `SValBuilder` and be accessed from both `ExprEngine` and `CallEvent` (in the latter case through `.getState()->getStateManager()`). Performance-wise, switching to the `CallEvent` approach would trade an additional exploded node or two for a potentially unlimited number of re-evaluations of the cast (in every checker) - both seem lightweight enough for now to never bother. Storing the casted value in `CallEvent` would require making the `CallEvent` object more bulky, which doesn't seem justified. Futureproofness-wise, we may potentially re-introduce the callback when we actually require two program points, or if our cast procedure becomes slow and we'd want to re-use the result, but both is unlikely. https://reviews.llvm.org/D41406 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r322093 - [libunwind][MIPS]: Rename Registers_mips_n64 to Registers_mips_newabi.
Author: jhb
Date: Tue Jan 9 09:07:18 2018
New Revision: 322093
URL: http://llvm.org/viewvc/llvm-project?rev=322093&view=rev
Log:
[libunwind][MIPS]: Rename Registers_mips_n64 to Registers_mips_newabi.
This is in preparation for adding support for N32 unwinding which reuses
the newabi register class.
Reviewed By: compnerd
Differential Revision: https://reviews.llvm.org/D41842
Modified:
libunwind/trunk/include/__libunwind_config.h
libunwind/trunk/src/Registers.hpp
libunwind/trunk/src/UnwindCursor.hpp
libunwind/trunk/src/UnwindRegistersRestore.S
libunwind/trunk/src/libunwind.cpp
Modified: libunwind/trunk/include/__libunwind_config.h
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/include/__libunwind_config.h?rev=322093&r1=322092&r2=322093&view=diff
==
--- libunwind/trunk/include/__libunwind_config.h (original)
+++ libunwind/trunk/include/__libunwind_config.h Tue Jan 9 09:07:18 2018
@@ -76,7 +76,7 @@
#define _LIBUNWIND_CONTEXT_SIZE 18
#define _LIBUNWIND_CURSOR_SIZE 24
# elif defined(_ABI64) && defined(__mips_soft_float)
-#define _LIBUNWIND_TARGET_MIPS_N64 1
+#define _LIBUNWIND_TARGET_MIPS_NEWABI 1
#define _LIBUNWIND_CONTEXT_SIZE 35
#define _LIBUNWIND_CURSOR_SIZE 47
# else
@@ -95,7 +95,7 @@
# define _LIBUNWIND_TARGET_ARM 1
# define _LIBUNWIND_TARGET_OR1K 1
# define _LIBUNWIND_TARGET_MIPS_O32 1
-# define _LIBUNWIND_TARGET_MIPS_N64 1
+# define _LIBUNWIND_TARGET_MIPS_NEWABI 1
# define _LIBUNWIND_CONTEXT_SIZE 136
# define _LIBUNWIND_CURSOR_SIZE 148
# define _LIBUNWIND_HIGHEST_DWARF_REGISTER 287
Modified: libunwind/trunk/src/Registers.hpp
URL:
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Registers.hpp?rev=322093&r1=322092&r2=322093&view=diff
==
--- libunwind/trunk/src/Registers.hpp (original)
+++ libunwind/trunk/src/Registers.hpp Tue Jan 9 09:07:18 2018
@@ -2825,13 +2825,13 @@ inline const char *Registers_mips_o32::g
}
#endif // _LIBUNWIND_TARGET_MIPS_O32
-#if defined(_LIBUNWIND_TARGET_MIPS_N64)
-/// Registers_mips_n64 holds the register state of a thread in a 64-bit MIPS
-/// process.
-class _LIBUNWIND_HIDDEN Registers_mips_n64 {
+#if defined(_LIBUNWIND_TARGET_MIPS_NEWABI)
+/// Registers_mips_newabi holds the register state of a thread in a
+/// MIPS process using NEWABI (the N32 or N64 ABIs).
+class _LIBUNWIND_HIDDEN Registers_mips_newabi {
public:
- Registers_mips_n64();
- Registers_mips_n64(const void *registers);
+ Registers_mips_newabi();
+ Registers_mips_newabi(const void *registers);
boolvalidRegister(int num) const;
uint64_tgetRegister(int num) const;
@@ -2852,28 +2852,28 @@ public:
void setIP(uint64_t value) { _registers.__pc = value; }
private:
- struct mips_n64_thread_state_t {
+ struct mips_newabi_thread_state_t {
uint64_t __r[32];
uint64_t __pc;
uint64_t __hi;
uint64_t __lo;
};
- mips_n64_thread_state_t _registers;
+ mips_newabi_thread_state_t _registers;
};
-inline Registers_mips_n64::Registers_mips_n64(const void *registers) {
- static_assert((check_fit::does_fit),
-"mips_n64 registers do not fit into unw_context_t");
+inline Registers_mips_newabi::Registers_mips_newabi(const void *registers) {
+ static_assert((check_fit::does_fit),
+"mips_newabi registers do not fit into unw_context_t");
memcpy(&_registers, static_cast(registers),
sizeof(_registers));
}
-inline Registers_mips_n64::Registers_mips_n64() {
+inline Registers_mips_newabi::Registers_mips_newabi() {
memset(&_registers, 0, sizeof(_registers));
}
-inline bool Registers_mips_n64::validRegister(int regNum) const {
+inline bool Registers_mips_newabi::validRegister(int regNum) const {
if (regNum == UNW_REG_IP)
return true;
if (regNum == UNW_REG_SP)
@@ -2890,7 +2890,7 @@ inline bool Registers_mips_n64::validReg
return false;
}
-inline uint64_t Registers_mips_n64::getRegister(int regNum) const {
+inline uint64_t Registers_mips_newabi::getRegister(int regNum) const {
if (regNum >= UNW_MIPS_R0 && regNum <= UNW_MIPS_R31)
return _registers.__r[regNum - UNW_MIPS_R0];
@@ -2904,10 +2904,10 @@ inline uint64_t Registers_mips_n64::getR
case UNW_MIPS_LO:
return _registers.__lo;
}
- _LIBUNWIND_ABORT("unsupported mips_n64 register");
+ _LIBUNWIND_ABORT("unsupported mips_newabi register");
}
-inline void Registers_mips_n64::setRegister(int regNum, uint64_t value) {
+inline void Registers_mips_newabi::setRegister(int regNum, uint64_t value) {
if (regNum >= UNW_MIPS_R0 && regNum <= UNW_MIPS_R31) {
_registers.__r[regNum - UNW_MIPS_R0] = value;
return;
@@ -2927,35 +2927,35 @@ inline void Registers_mips_n64::setRegis
_registers.__lo = value;
return;
}
- _LIBUNWIND_ABORT("unsupported mips_n64 register");
+ _LIBUNWIND_ABORT("unsupported
[PATCH] D41842: [libunwind][MIPS]: Rename Registers_mips_n64 to Registers_mips_newabi.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL322093: [libunwind][MIPS]: Rename Registers_mips_n64 to
Registers_mips_newabi. (authored by jhb, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D41842?vs=129000&id=129100#toc
Repository:
rL LLVM
https://reviews.llvm.org/D41842
Files:
libunwind/trunk/include/__libunwind_config.h
libunwind/trunk/src/Registers.hpp
libunwind/trunk/src/UnwindCursor.hpp
libunwind/trunk/src/UnwindRegistersRestore.S
libunwind/trunk/src/libunwind.cpp
Index: libunwind/trunk/src/Registers.hpp
===
--- libunwind/trunk/src/Registers.hpp
+++ libunwind/trunk/src/Registers.hpp
@@ -2825,13 +2825,13 @@
}
#endif // _LIBUNWIND_TARGET_MIPS_O32
-#if defined(_LIBUNWIND_TARGET_MIPS_N64)
-/// Registers_mips_n64 holds the register state of a thread in a 64-bit MIPS
-/// process.
-class _LIBUNWIND_HIDDEN Registers_mips_n64 {
+#if defined(_LIBUNWIND_TARGET_MIPS_NEWABI)
+/// Registers_mips_newabi holds the register state of a thread in a
+/// MIPS process using NEWABI (the N32 or N64 ABIs).
+class _LIBUNWIND_HIDDEN Registers_mips_newabi {
public:
- Registers_mips_n64();
- Registers_mips_n64(const void *registers);
+ Registers_mips_newabi();
+ Registers_mips_newabi(const void *registers);
boolvalidRegister(int num) const;
uint64_tgetRegister(int num) const;
@@ -2852,28 +2852,28 @@
void setIP(uint64_t value) { _registers.__pc = value; }
private:
- struct mips_n64_thread_state_t {
+ struct mips_newabi_thread_state_t {
uint64_t __r[32];
uint64_t __pc;
uint64_t __hi;
uint64_t __lo;
};
- mips_n64_thread_state_t _registers;
+ mips_newabi_thread_state_t _registers;
};
-inline Registers_mips_n64::Registers_mips_n64(const void *registers) {
- static_assert((check_fit::does_fit),
-"mips_n64 registers do not fit into unw_context_t");
+inline Registers_mips_newabi::Registers_mips_newabi(const void *registers) {
+ static_assert((check_fit::does_fit),
+"mips_newabi registers do not fit into unw_context_t");
memcpy(&_registers, static_cast(registers),
sizeof(_registers));
}
-inline Registers_mips_n64::Registers_mips_n64() {
+inline Registers_mips_newabi::Registers_mips_newabi() {
memset(&_registers, 0, sizeof(_registers));
}
-inline bool Registers_mips_n64::validRegister(int regNum) const {
+inline bool Registers_mips_newabi::validRegister(int regNum) const {
if (regNum == UNW_REG_IP)
return true;
if (regNum == UNW_REG_SP)
@@ -2890,7 +2890,7 @@
return false;
}
-inline uint64_t Registers_mips_n64::getRegister(int regNum) const {
+inline uint64_t Registers_mips_newabi::getRegister(int regNum) const {
if (regNum >= UNW_MIPS_R0 && regNum <= UNW_MIPS_R31)
return _registers.__r[regNum - UNW_MIPS_R0];
@@ -2904,10 +2904,10 @@
case UNW_MIPS_LO:
return _registers.__lo;
}
- _LIBUNWIND_ABORT("unsupported mips_n64 register");
+ _LIBUNWIND_ABORT("unsupported mips_newabi register");
}
-inline void Registers_mips_n64::setRegister(int regNum, uint64_t value) {
+inline void Registers_mips_newabi::setRegister(int regNum, uint64_t value) {
if (regNum >= UNW_MIPS_R0 && regNum <= UNW_MIPS_R31) {
_registers.__r[regNum - UNW_MIPS_R0] = value;
return;
@@ -2927,35 +2927,35 @@
_registers.__lo = value;
return;
}
- _LIBUNWIND_ABORT("unsupported mips_n64 register");
+ _LIBUNWIND_ABORT("unsupported mips_newabi register");
}
-inline bool Registers_mips_n64::validFloatRegister(int /* regNum */) const {
+inline bool Registers_mips_newabi::validFloatRegister(int /* regNum */) const {
return false;
}
-inline double Registers_mips_n64::getFloatRegister(int /* regNum */) const {
- _LIBUNWIND_ABORT("mips_n64 float support not implemented");
+inline double Registers_mips_newabi::getFloatRegister(int /* regNum */) const {
+ _LIBUNWIND_ABORT("mips_newabi float support not implemented");
}
-inline void Registers_mips_n64::setFloatRegister(int /* regNum */,
+inline void Registers_mips_newabi::setFloatRegister(int /* regNum */,
double /* value */) {
- _LIBUNWIND_ABORT("mips_n64 float support not implemented");
+ _LIBUNWIND_ABORT("mips_newabi float support not implemented");
}
-inline bool Registers_mips_n64::validVectorRegister(int /* regNum */) const {
+inline bool Registers_mips_newabi::validVectorRegister(int /* regNum */) const {
return false;
}
-inline v128 Registers_mips_n64::getVectorRegister(int /* regNum */) const {
- _LIBUNWIND_ABORT("mips_n64 vector support not implemented");
+inline v128 Registers_mips_newabi::getVectorRegister(int /* regNum */) const {
+ _LIBUNWIND_ABORT("mips_newabi vector support not implemented");
}
-inline void Registers_mips_n64::setVectorRegister(int /* regNum */, v128 /
[PATCH] D41345: [clangd] Add more symbol information for code completion.
sammccall accepted this revision.
sammccall added inline comments.
Comment at: clangd/index/SymbolYAML.cpp:78
+assert(io.getContext());
+Detail = static_cast(io.getContext())
+ ->Allocate();
this removes the Detail object if it's empty - this seems maybe unneccesary and
certainly the wrong layer. It seems enough to do
if (!outputting)
Detail = (allocate)
else if (!Detail)
return;
io.mapOptional("Documentation", Detail->Documentation);
// etc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41668: [clangd] Add static index for the global code completion.
sammccall accepted this revision.
sammccall added a comment.
Ilya's suggestion to have a single index on the CodeCompleteOptions that
encapsulates the merging seems reasonable to me, but not urgent.
So let's get this in as-is and we can make that change whenever.
Comment at: clangd/CodeComplete.cpp:568
+ // FIXME: Find out a better way to show the index source.
+ llvm::raw_string_ostream Label(Item.label);
+ if (!DebuggingLabel.empty()) {
move this inside and only use for nontrivial case?
Comment at: clangd/CodeComplete.cpp:661
Results.items.clear();
+Results.isIncomplete = false;
+// FIXME: figure out a good algorithm to merge symbols from different
Shouldn't it already be false here?
Comment at: clangd/CodeComplete.cpp:666
CompletedName.Filter, &Results);
+if (Opts.StaticIndex) {
+ completeWithIndex(Ctx, *Opts.StaticIndex, Contents,
*CompletedName.SSInfo,
do you only want to use the static index if the dynamic index is also present?
(That logic may exist at a higher level, but I'm not sure it makes sense to
have it here)
I'd suggest to remove the result items empty check and the clear(), and then
just put the two index runs in independent if statements
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41668
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.
bsdjhb updated this revision to Diff 129103.
bsdjhb added a comment.
- Rebase after N64 -> newabi commit.
https://reviews.llvm.org/D39074
Files:
include/__libunwind_config.h
src/AddressSpace.hpp
src/DwarfInstructions.hpp
src/UnwindRegistersRestore.S
src/UnwindRegistersSave.S
src/libunwind.cpp
Index: src/libunwind.cpp
===
--- src/libunwind.cpp
+++ src/libunwind.cpp
@@ -63,7 +63,8 @@
# define REGISTER_KIND Registers_or1k
#elif defined(__mips__) && defined(_ABIO32) && defined(__mips_soft_float)
# define REGISTER_KIND Registers_mips_o32
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABIN32) || defined(_ABI64)) &&\
+defined(__mips_soft_float)
# define REGISTER_KIND Registers_mips_newabi
#elif defined(__mips__)
# warning The MIPS architecture is not supported with this ABI and environment!
Index: src/UnwindRegistersSave.S
===
--- src/UnwindRegistersSave.S
+++ src/UnwindRegistersSave.S
@@ -172,7 +172,8 @@
or$2, $0, $0
.set pop
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&\
+defined(__mips_soft_float)
#
# extern int unw_getcontext(unw_context_t* thread_state)
Index: src/UnwindRegistersRestore.S
===
--- src/UnwindRegistersRestore.S
+++ src/UnwindRegistersRestore.S
@@ -685,7 +685,8 @@
lw$4, (4 * 4)($4)
.set pop
-#elif defined(__mips__) && defined(_ABI64) && defined(__mips_soft_float)
+#elif defined(__mips__) && (defined(_ABI64) || defined(_ABIN32)) &&\
+defined(__mips_soft_float)
//
// void libunwind::Registers_mips_newabi::jumpto()
Index: src/DwarfInstructions.hpp
===
--- src/DwarfInstructions.hpp
+++ src/DwarfInstructions.hpp
@@ -82,10 +82,10 @@
const RegisterLocation &savedReg) {
switch (savedReg.location) {
case CFI_Parser::kRegisterInCFA:
-return addressSpace.getP(cfa + (pint_t)savedReg.value);
+return addressSpace.getRegister(cfa + (pint_t)savedReg.value);
case CFI_Parser::kRegisterAtExpression:
-return addressSpace.getP(
+return addressSpace.getRegister(
evaluateExpression((pint_t)savedReg.value, addressSpace,
registers, cfa));
Index: src/AddressSpace.hpp
===
--- src/AddressSpace.hpp
+++ src/AddressSpace.hpp
@@ -207,6 +207,7 @@
return val;
}
uintptr_t getP(pint_t addr);
+ uint64_tgetRegister(pint_t addr);
static uint64_t getULEB128(pint_t &addr, pint_t end);
static int64_t getSLEB128(pint_t &addr, pint_t end);
@@ -228,6 +229,14 @@
#endif
}
+inline uint64_t LocalAddressSpace::getRegister(pint_t addr) {
+#if __SIZEOF_POINTER__ == 8 || (defined(__mips__) && defined(_ABIN32))
+ return get64(addr);
+#else
+ return get32(addr);
+#endif
+}
+
/// Read a ULEB128 into a 64-bit word.
inline uint64_t LocalAddressSpace::getULEB128(pint_t &addr, pint_t end) {
const uint8_t *p = (uint8_t *)addr;
@@ -600,6 +609,7 @@
uint32_t get32(pint_t addr);
uint64_t get64(pint_t addr);
pint_tgetP(pint_t addr);
+ uint64_t getRegister(pint_t addr);
uint64_t getULEB128(pint_t &addr, pint_t end);
int64_t getSLEB128(pint_t &addr, pint_t end);
pint_tgetEncodedP(pint_t &addr, pint_t end, uint8_t encoding,
@@ -636,7 +646,12 @@
}
template
-uint64_t RemoteAddressSpace::getULEB128(pint_t &addr, pint_t end) {
+typename P::uint_t OtherAddressSpace::getRegister(pint_t addr) {
+ return P::getRegister(*(uint64_t *)localCopy(addr));
+}
+
+template
+uint64_t OtherAddressSpace::getULEB128(pint_t &addr, pint_t end) {
uintptr_t size = (end - addr);
LocalAddressSpace::pint_t laddr = (LocalAddressSpace::pint_t) localCopy(addr);
LocalAddressSpace::pint_t sladdr = laddr;
Index: include/__libunwind_config.h
===
--- include/__libunwind_config.h
+++ include/__libunwind_config.h
@@ -75,6 +75,10 @@
#define _LIBUNWIND_TARGET_MIPS_O32 1
#define _LIBUNWIND_CONTEXT_SIZE 18
#define _LIBUNWIND_CURSOR_SIZE 24
+# elif defined(_ABIN32) && defined(__mips_soft_float)
+#define _LIBUNWIND_TARGET_MIPS_NEWABI 1
+#define _LIBUNWIND_CONTEXT_SIZE 35
+#define _LIBUNWIND_CURSOR_SIZE 42
# elif defined(_ABI64) && defined(__mips_soft_float)
#define _LIBUNWIND_TARGET_MIPS_NEWABI 1
#define _LIBUNWIND_CONTEXT_SIZE 35
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 129106.
ioeric marked an inline comment as done.
ioeric added a comment.
- Addrress review comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,12 +25,24 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -147,6 +158,38 @@
QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -160,6 +203,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -174,15 +223,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
arg.in
[clang-tools-extra] r322097 - [clangd] Add more symbol information for code completion.
Author: ioeric
Date: Tue Jan 9 09:32:00 2018
New Revision: 322097
URL: http://llvm.org/viewvc/llvm-project?rev=322097&view=rev
Log:
[clangd] Add more symbol information for code completion.
Reviewers: hokein, sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41345
Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=322097&r1=322096&r2=322097&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue Jan 9 09:32:00 2018
@@ -316,6 +316,10 @@ const ASTContext &ParsedAST::getASTConte
Preprocessor &ParsedAST::getPreprocessor() { return Clang->getPreprocessor(); }
+std::shared_ptr ParsedAST::getPreprocessorPtr() {
+ return Clang->getPreprocessorPtr();
+}
+
const Preprocessor &ParsedAST::getPreprocessor() const {
return Clang->getPreprocessor();
}
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=322097&r1=322096&r2=322097&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h Tue Jan 9 09:32:00 2018
@@ -79,6 +79,7 @@ public:
const ASTContext &getASTContext() const;
Preprocessor &getPreprocessor();
+ std::shared_ptr getPreprocessorPtr();
const Preprocessor &getPreprocessor() const;
/// This function returns all top-level decls, including those that come
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=322097&r1=322096&r2=322097&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Jan 9 09:32:00 2018
@@ -556,16 +556,19 @@ CompletionItem indexCompletionItem(const
Item.kind = toCompletionItemKind(Sym.SymInfo.Kind);
Item.label = Sym.Name;
// FIXME(ioeric): support inserting/replacing scope qualifiers.
- Item.insertText = Sym.Name;
+
// FIXME(ioeric): support snippets.
+ Item.insertText = Sym.CompletionPlainInsertText;
Item.insertTextFormat = InsertTextFormat::PlainText;
Item.filterText = Sym.Name;
// FIXME(ioeric): sort symbols appropriately.
Item.sortText = "";
- // FIXME(ioeric): use more symbol information (e.g. documentation, label) to
- // populate the completion item.
+ if (Sym.Detail) {
+Item.documentation = Sym.Detail->Documentation;
+Item.detail = Sym.Detail->CompletionDetail;
+ }
return Item;
}
Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=322097&r1=322096&r2=322097&view=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Tue Jan 9 09:32:00 2018
@@ -17,8 +17,10 @@ namespace {
/// Retrieves namespace and class level symbols in \p Decls.
std::unique_ptr indexAST(ASTContext &Ctx,
+ std::shared_ptr PP,
llvm::ArrayRef Decls) {
auto Collector = std::make_shared();
+ Collector->setPreprocessor(std::move(PP));
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -67,7 +69,8 @@ void FileIndex::update(const Context &Ct
if (!AST) {
FSymbols.update(Path, nullptr);
} else {
-auto Slab = indexAST(AST->getASTContext(), AST->getTopLevelDecls());
+auto Slab = indexAST(AST->getASTContext(), AST->getPreprocessorPtr(),
+ AST->getTopLevelDecls());
FSymbols.update(Path, std::move(Slab));
}
auto Symbols = FSymbols.allSymbols();
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clan
[PATCH] D41345: [clangd] Add more symbol information for code completion.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE322097: [clangd] Add more symbol information for code
completion. (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41345?vs=129106&id=129110#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,12 +25,24 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -147,6 +158,38 @@
QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -160,6 +203,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -174,15 +223,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { ret
[PATCH] D41823: [clangd] Add more filters for collected symbols.
ioeric updated this revision to Diff 129112.
ioeric added a comment.
- Rebase on origin/master
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41823
Files:
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/FileIndex.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/FileIndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -53,20 +53,22 @@
namespace {
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
- SymbolIndexActionFactory() = default;
+ SymbolIndexActionFactory(SymbolCollector::Options COpts)
+ : COpts(std::move(COpts)) {}
clang::FrontendAction *create() override {
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = false;
-Collector = std::make_shared();
+Collector = std::make_shared(COpts);
FrontendAction *Action =
index::createIndexingAction(Collector, IndexOpts, nullptr).release();
return Action;
}
std::shared_ptr Collector;
+ SymbolCollector::Options COpts;
};
class SymbolCollectorTest : public ::testing::Test {
@@ -79,7 +81,7 @@
const std::string FileName = "symbol.cc";
const std::string HeaderName = "symbols.h";
-auto Factory = llvm::make_unique();
+auto Factory = llvm::make_unique(CollectorOpts);
tooling::ToolInvocation Invocation(
{"symbol_collector", "-fsyntax-only", "-std=c++11", FileName},
@@ -100,9 +102,11 @@
protected:
SymbolSlab Symbols;
+ SymbolCollector::Options CollectorOpts;
};
-TEST_F(SymbolCollectorTest, CollectSymbol) {
+TEST_F(SymbolCollectorTest, CollectSymbols) {
+ CollectorOpts.IndexMainFiles = true;
const std::string Header = R"(
class Foo {
void f();
@@ -144,7 +148,6 @@
EXPECT_THAT(Symbols,
UnorderedElementsAreArray(
{QName("Foo"),
- QName("Foo::f"),
QName("f1"),
QName("f2"),
QName("KInt"),
@@ -158,29 +161,84 @@
QName("foo::baz")}));
}
-TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
+ const std::string Header = R"(
+class Foo {};
+void f1();
+inline void f2() {}
+ )";
+ const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void main_f() {} // ignore
+void f1() {}
+ )";
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2")));
+}
+
+TEST_F(SymbolCollectorTest, IncludeSymbolsInMainFile) {
+ CollectorOpts.IndexMainFiles = true;
+ const std::string Header = R"(
+class Foo {};
+void f1();
+inline void f2() {}
+ )";
+ const std::string Main = R"(
+namespace {
+void ff() {} // ignore
+}
+void main_f() {}
+void f1() {}
+ )";
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("f1"),
+QName("f2"), QName("main_f")));
+}
+
+TEST_F(SymbolCollectorTest, IgnoreClassMembers) {
+ const std::string Header = R"(
+class Foo {
+ void f() {}
+ void g();
+ static void sf() {}
+ static void ssf();
+ static int x;
+};
+ )";
const std::string Main = R"(
+void Foo::g() {}
+void Foo::ssf() {}
+ )";
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo")));
+}
+
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Header = R"(
namespace nx {
/// Foo comment.
int ff(int x, double y) { return 0; }
}
)";
- runSymbolCollector(/*Header=*/"", Main);
+ runSymbolCollector(Header, /*Main=*/"");
EXPECT_THAT(Symbols,
UnorderedElementsAre(QName("nx"),
AllOf(QName("nx::ff"),
Labeled("ff(int x, double y)"),
Detail("int"), Doc("Foo comment.";
}
TEST_F(SymbolCollectorTest, PlainAndSnippet) {
- const std::string Main = R"(
+ const std::string Header = R"(
namespace nx {
void f() {}
int ff(int x, double y) { return 0; }
}
)";
- runSymbolCollector(/*Header=*/"", Main);
+ runSymbolCollector(Header, /*Main=*/"");
EXPECT_THAT(
Symbols,
UnorderedElementsAre(
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -166,15 +166,16 @@
EXPECT_THAT(match
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
TyanNN updated this revision to Diff 129115.
TyanNN retitled this revision from "[libc++] Fix PR#35780 - make
std::experimental::filesystem::remove return false if the file doesn't exist"
to "[libc++] Fix PR#35780 - make std::experimental::filesystem::remove and
remove_all return false or 0 if the file doesn't exist".
TyanNN edited the summary of this revision.
TyanNN added a comment.
Fix remove_all too. Use std error codes instead of those from C
https://reviews.llvm.org/D41830
Files:
src/experimental/filesystem/operations.cpp
test/libcxx/experimental/filesystem/class.path/path.remove/remove_should_not_throw_on_nonexistant.pass.cpp
Index:
test/libcxx/experimental/filesystem/class.path/path.remove/remove_should_not_throw_on_nonexistant.pass.cpp
===
---
test/libcxx/experimental/filesystem/class.path/path.remove/remove_should_not_throw_on_nonexistant.pass.cpp
+++
test/libcxx/experimental/filesystem/class.path/path.remove/remove_should_not_throw_on_nonexistant.pass.cpp
@@ -0,0 +1,52 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+//
+
+// class path
+
+#define _LIBCPP_DEBUG 0
+#define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : (void)::AssertCount++)
+int AssertCount = 0;
+
+#include
+#include
+#include
+
+namespace fs = std::experimental::filesystem;
+
+int main()
+{
+using namespace fs;
+
+// filesystem::remove test
+
+path nFile("nonexistant.file");
+assert(remove(nFile) == false);
+
+path tempFilePath = temp_directory_path();
+tempFilePath += path("existingFile");
+
+std::ofstream theTempFile(tempFilePath);
+theTempFile.close();
+
+assert(remove(tempFilePath) == true);
+
+// filesystem::remove_all
+
+assert(remove_all(nFile) == 0);
+
+std::ofstream theTempFile2(tempFilePath);
+theTempFile2.close();
+assert(remove_all(tempFilePath) == 1);
+
+return 0;
+}
Index: src/experimental/filesystem/operations.cpp
===
--- src/experimental/filesystem/operations.cpp
+++ src/experimental/filesystem/operations.cpp
@@ -661,8 +661,10 @@
bool __remove(const path& p, std::error_code *ec) {
if (ec) ec->clear();
+
if (::remove(p.c_str()) == -1) {
-set_or_throw(ec, "remove", p);
+if (ec != nullptr && *ec != errc::no_such_file_or_directory)
+set_or_throw(ec, "remove", p);
return false;
}
return true;
@@ -695,8 +697,12 @@
std::error_code mec;
auto count = remove_all_impl(p, mec);
if (mec) {
-set_or_throw(mec, ec, "remove_all", p);
-return static_cast(-1);
+if (mec != errc::no_such_file_or_directory) {
+set_or_throw(mec, ec, "remove_all", p);
+return static_cast(-1);
+} else {
+return static_cast(0);
+}
}
if (ec) ec->clear();
return count;
Index: test/libcxx/experimental/filesystem/class.path/path.remove/remove_should_not_throw_on_nonexistant.pass.cpp
===
--- test/libcxx/experimental/filesystem/class.path/path.remove/remove_should_not_throw_on_nonexistant.pass.cpp
+++ test/libcxx/experimental/filesystem/class.path/path.remove/remove_should_not_throw_on_nonexistant.pass.cpp
@@ -0,0 +1,52 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03
+
+//
+
+// class path
+
+#define _LIBCPP_DEBUG 0
+#define _LIBCPP_ASSERT(x, m) ((x) ? (void)0 : (void)::AssertCount++)
+int AssertCount = 0;
+
+#include
+#include
+#include
+
+namespace fs = std::experimental::filesystem;
+
+int main()
+{
+using namespace fs;
+
+// filesystem::remove test
+
+path nFile("nonexistant.file");
+assert(remove(nFile) == false);
+
+path tempFilePath = temp_directory_path();
+tempFilePath += path("existingFile");
+
+std::ofstream theTempFile(tempFilePath);
+theTempFile.close();
+
+assert(remove(tempFilePath) == true);
+
+// filesystem::remove_all
+
+assert(remove_all(nFile) == 0);
+
+std::ofstream theTempFile2(tempFilePath);
+theTempFile2.close();
+assert(remove_all(tempFilePath) == 1);
+
+return 0;
+}
Index: src/experimental/filesystem/operations.cpp
===
[PATCH] D41553: Support parsing double square-bracket attributes in ObjC
rjmccall added inline comments. Comment at: include/clang/Basic/Attr.td:239 + bit IncludeC = includeC; +} aaron.ballman wrote: > rjmccall wrote: > > aaron.ballman wrote: > > > rjmccall wrote: > > > > I have no objection to allowing ObjC attributes to be spelled in > > > > [[clang::objc_whatever]] style. We can debate giving them a more > > > > appropriate standard name in the objc namespace at a later time — or > > > > even the primary namespace, if we really want to flex our Somewhat > > > > Major Language muscles. > > > > > > > > I feel like this IncludeC is not the best name for this tablegen > > > > property. Perhaps AllowInC? > > > > > > > > Also, a random positional 1 argument is pretty obscure TableGen design. > > > > Do we really expect to be making very many attributes that > > > > intentionally disallow C2x? What would these be, just C++-specific > > > > attributes without C analogues? It doesn't seem important to prohibit > > > > those at the parsing level rather than at the semantic level, since, > > > > after all, we are still allowing the GNU-style spelling, and these > > > > would still qualified with ``clang::``. > > > > > > > > I would suggest standardizing in the opposite direction: instead of > > > > forcing attributes to opt in to being allowed in C, we should make > > > > attributes that we really don't want to allow in the C2x [[clang::]] > > > > namespace be explicit about it. If there are a lot of C++-specific > > > > attributes like that, we can make a ClangCXXOnly subclass. > > > > I have no objection to allowing ObjC attributes to be spelled in > > > > [[clang::objc_whatever]] style. We can debate giving them a more > > > > appropriate standard name in the objc namespace at a later time — or > > > > even the primary namespace, if we really want to flex our Somewhat > > > > Major Language muscles. > > > > > > Thanks -- are you okay with where the attributes are allowed in the > > > syntax? I tried to follow the position they're allowed in C and C++ as > > > closely as I could, but having confirmation would be nice. > > > > > > As for putting the attributes in the primary namespace, that would be > > > reasonable for using the attributes in ObjC or ObjC++ but not so much for > > > using the same attributes in a pure C or C++ compilation. > > > > > > > I feel like this IncludeC is not the best name for this tablegen > > > > property. Perhaps AllowInC? > > > > > > I'm fine with that name. I'll change it in D41317 when I commit that. > > > > > > > Also, a random positional 1 argument is pretty obscure TableGen design. > > > > Do we really expect to be making very many attributes that > > > > intentionally disallow C2x? What would these be, just C++-specific > > > > attributes without C analogues? > > > > > > I agree -- I was toying with using a named entity rather than a numeric > > > literal, but I wanted to see how the design shook out in practice once I > > > had a better feel for how many attributes are impacted. I'm glad you > > > recommend going the opposite direction as that was my ultimate goal. :-) > > > Basically, my initial approach is to disallow everything in C and then > > > start enabling the attributes that make sense. At some point, I believe > > > we'll have more attributes in both language modes than not, and then I > > > plan to reverse the meaning of the flag so that an attribute has to > > > opt-out rather than opt-in. I don't expect we'll have many attributes > > > that are disallowed in C2x. I think they'll fall into two categories: C++ > > > attributes that don't make sense in C and attributes that are governed by > > > some other body. > > > > > > > It doesn't seem important to prohibit those at the parsing level rather > > > > than at the semantic level, since, after all, we are still allowing the > > > > GNU-style spelling, and these would still qualified with `clang::`. > > > > > > I think it's important for C targets when > > > `-fdouble-square-bracket-attributes` is not enabled, as the attributes > > > cannot syntactically appear in those positions. > > > > > > > I would suggest standardizing in the opposite direction > > > > > > I suspect I will ultimately get there. I chose this approach to be more > > > conservative about what we expose. > > > Thanks -- are you okay with where the attributes are allowed in the > > > syntax? I tried to follow the position they're allowed in C and C++ as > > > closely as I could, but having confirmation would be nice. > > > > I'll leave comments on the various places. For the most part, no, I think > > these are the wrong places; where we allow attributes in ObjC is actually > > pretty carefully thought-out already, and it's better to follow the places > > where we parse GNU attributes than to try to imitate the C grammar. > > > > > As for putting the attributes in the primary namespace, that would be > > > re
[PATCH] D41841: [OpenMP] Fix handling of clause on wrong directive
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D41841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41834: [Lex] Fix handling numerical literals ending with ' and signed exponent.
vsapsai updated this revision to Diff 129125.
vsapsai added a comment.
- Don't parse exponent past the end of token, add same test+fix for hexadecimal
numbers.
https://reviews.llvm.org/D41834
Files:
clang/lib/Lex/LiteralSupport.cpp
clang/test/Lexer/cxx1y_digit_separators.cpp
Index: clang/test/Lexer/cxx1y_digit_separators.cpp
===
--- clang/test/Lexer/cxx1y_digit_separators.cpp
+++ clang/test/Lexer/cxx1y_digit_separators.cpp
@@ -51,6 +51,8 @@
float u = 0x.'p1f; // expected-error {{hexadecimal floating literal requires
a significand}}
float v = 0e'f; // expected-error {{exponent has no digits}}
float w = 0x0p'f; // expected-error {{exponent has no digits}}
+ float x = 0'e+1; // expected-error {{digit separator cannot appear at end of
digit sequence}}
+ float y = 0x0'p+1; // expected-error {{digit separator cannot appear at end
of digit sequence}}
}
#line 123'456
Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -738,15 +738,17 @@
s++;
radix = 10;
saw_exponent = true;
-if (*s == '+' || *s == '-') s++; // sign
+if (s != ThisTokEnd && (*s == '+' || *s == '-')) s++; // sign
const char *first_non_digit = SkipDigits(s);
if (containsDigits(s, first_non_digit)) {
checkSeparator(TokLoc, s, CSK_BeforeDigits);
s = first_non_digit;
} else {
- PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
- diag::err_exponent_has_no_digits);
- hadError = true;
+ if (!hadError) {
+PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
+diag::err_exponent_has_no_digits);
+hadError = true;
+ }
return;
}
}
@@ -787,10 +789,12 @@
} else if (Pos == ThisTokEnd)
return;
- if (isDigitSeparator(*Pos))
+ if (isDigitSeparator(*Pos)) {
PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Pos - ThisTokBegin),
diag::err_digit_separator_not_between_digits)
<< IsAfterDigits;
+hadError = true;
+ }
}
/// ParseNumberStartingWithZero - This method is called when the first
character
@@ -840,12 +844,14 @@
const char *Exponent = s;
s++;
saw_exponent = true;
- if (*s == '+' || *s == '-') s++; // sign
+ if (s != ThisTokEnd && (*s == '+' || *s == '-')) s++; // sign
const char *first_non_digit = SkipDigits(s);
if (!containsDigits(s, first_non_digit)) {
-PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
-diag::err_exponent_has_no_digits);
-hadError = true;
+if (!hadError) {
+ PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
+ diag::err_exponent_has_no_digits);
+ hadError = true;
+}
return;
}
checkSeparator(TokLoc, s, CSK_BeforeDigits);
Index: clang/test/Lexer/cxx1y_digit_separators.cpp
===
--- clang/test/Lexer/cxx1y_digit_separators.cpp
+++ clang/test/Lexer/cxx1y_digit_separators.cpp
@@ -51,6 +51,8 @@
float u = 0x.'p1f; // expected-error {{hexadecimal floating literal requires a significand}}
float v = 0e'f; // expected-error {{exponent has no digits}}
float w = 0x0p'f; // expected-error {{exponent has no digits}}
+ float x = 0'e+1; // expected-error {{digit separator cannot appear at end of digit sequence}}
+ float y = 0x0'p+1; // expected-error {{digit separator cannot appear at end of digit sequence}}
}
#line 123'456
Index: clang/lib/Lex/LiteralSupport.cpp
===
--- clang/lib/Lex/LiteralSupport.cpp
+++ clang/lib/Lex/LiteralSupport.cpp
@@ -738,15 +738,17 @@
s++;
radix = 10;
saw_exponent = true;
-if (*s == '+' || *s == '-') s++; // sign
+if (s != ThisTokEnd && (*s == '+' || *s == '-')) s++; // sign
const char *first_non_digit = SkipDigits(s);
if (containsDigits(s, first_non_digit)) {
checkSeparator(TokLoc, s, CSK_BeforeDigits);
s = first_non_digit;
} else {
- PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
- diag::err_exponent_has_no_digits);
- hadError = true;
+ if (!hadError) {
+PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Exponent-ThisTokBegin),
+diag::err_exponent_has_no_digits);
+hadError = true;
+ }
return;
}
}
@@ -787,10 +789,12 @@
} else if (Pos == ThisTokEnd)
return;
- if (isDigitSeparator(*Pos))
+ if (isDigitSeparator(*Pos)) {
PP.Diag(PP.AdvanceToTokenCharacter(TokLoc, Pos - ThisTokBegin),
diag::err_digit_separator_not_between_digits)
<< IsAfterDigits;
+hadError = true;
+ }
}
/// ParseNumberStartingWithZero - This m
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
mclow.lists added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:666
if (::remove(p.c_str()) == -1) {
-set_or_throw(ec, "remove", p);
+if (ec != nullptr && *ec != errc::no_such_file_or_directory)
+set_or_throw(ec, "remove", p);
I don't think that this is correct.
In the case where ec == nullptr AND the error is not "no such file", this will
fail to throw.
I think you just want this test to be:
if (errno != ENOENT)
https://reviews.llvm.org/D41830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41841: [OpenMP] Fix handling of clause on wrong directive
jdenny added a comment. Alexey: Thanks for accepting. I do not have commit privileges. Would you please commit for me? https://reviews.llvm.org/D41841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41841: [OpenMP] Fix handling of clause on wrong directive
ABataev added a comment. In https://reviews.llvm.org/D41841#971375, @jdenny wrote: > Alexey: Thanks for accepting. I do not have commit privileges. Would > you please commit for me? Sure https://reviews.llvm.org/D41841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41830: [libc++] Fix PR#35780 - make std::experimental::filesystem::remove and remove_all return false or 0 if the file doesn't exist
mclow.lists added inline comments.
Comment at: src/experimental/filesystem/operations.cpp:699
auto count = remove_all_impl(p, mec);
if (mec) {
+if (mec != errc::no_such_file_or_directory) {
I don't think that this is quite right, either.
In the case where you call `remove_all("doesNotExist", &ec)`, this code will
return 0, but will fail to clear `ec`.
Maybe this (untested) version instead:
if (ec) ec->clear();
auto count = remove_all_impl(p, mec);
if (mec) {
if (mec == errc::no_such_file_or_directory)
return 0;
else {
set_or_throw(mec, ec, "remove_all", p);
return static_cast(-1);
}
}
return count;
https://reviews.llvm.org/D41830
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322107 - [OpenMP] Fix handling of clause on wrong directive, by Joel. E. Denny
Author: abataev
Date: Tue Jan 9 11:21:04 2018
New Revision: 322107
URL: http://llvm.org/viewvc/llvm-project?rev=322107&view=rev
Log:
[OpenMP] Fix handling of clause on wrong directive, by Joel. E. Denny
Summary:
First, this patch fixes an assert failure when, for example, "omp for"
has num_teams.
Second, this patch prevents duplicate diagnostics when, for example,
"omp for" has uniform.
This patch makes the general assumption (even where it doesn't
necessarily fix an existing bug) that it is worthless to perform sema
for a clause that appears on a directive on which OpenMP does not
permit that clause. However, due to this assumption, this patch
suppresses some diagnostics that were expected in the test suite. I
assert that those diagnostics were likely just distracting to the
user.
Reviewers: ABataev
Reviewed By: ABataev
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D41841
Modified:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/test/OpenMP/distribute_simd_loop_messages.cpp
cfe/trunk/test/OpenMP/for_misc_messages.c
cfe/trunk/test/OpenMP/parallel_messages.cpp
cfe/trunk/test/OpenMP/simd_loop_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_messages.cpp
cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp
Modified: cfe/trunk/include/clang/Parse/Parser.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=322107&r1=322106&r2=322107&view=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Tue Jan 9 11:21:04 2018
@@ -2675,30 +2675,42 @@ private:
/// \brief Parses clause with a single expression of a kind \a Kind.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
- OMPClause *ParseOpenMPSingleExprClause(OpenMPClauseKind Kind);
+ OMPClause *ParseOpenMPSingleExprClause(OpenMPClauseKind Kind,
+ bool ParseOnly);
/// \brief Parses simple clause of a kind \a Kind.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
- OMPClause *ParseOpenMPSimpleClause(OpenMPClauseKind Kind);
+ OMPClause *ParseOpenMPSimpleClause(OpenMPClauseKind Kind, bool ParseOnly);
/// \brief Parses clause with a single expression and an additional argument
/// of a kind \a Kind.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
- OMPClause *ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind);
+ OMPClause *ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind,
+bool ParseOnly);
/// \brief Parses clause without any additional arguments.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
- OMPClause *ParseOpenMPClause(OpenMPClauseKind Kind);
+ OMPClause *ParseOpenMPClause(OpenMPClauseKind Kind, bool ParseOnly = false);
/// \brief Parses clause with the list of variables of a kind \a Kind.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
OMPClause *ParseOpenMPVarListClause(OpenMPDirectiveKind DKind,
- OpenMPClauseKind Kind);
+ OpenMPClauseKind Kind, bool ParseOnly);
public:
/// Parses simple expression in parens for single-expression clauses of
OpenMP
Modified: cfe/trunk/lib/Parse/ParseOpenMP.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseOpenMP.cpp?rev=322107&r1=322106&r2=322107&view=diff
==
--- cfe/trunk/lib/Parse/ParseOpenMP.cpp (original)
+++ cfe/trunk/lib/Parse/ParseOpenMP.cpp Tue Jan 9 11:21:04 2018
@@ -1205,11 +1205,13 @@ OMPClause *Parser::ParseOpenMPClause(Ope
OpenMPClauseKind CKind, bool FirstClause)
{
OMPClause *Clause = nullptr;
bool ErrorFound = false;
+ bool WrongDirective = false;
// Check if clause is allowed for the given directive.
if (CKind != OMPC_unknown && !isAllowedClauseForDirective(DKind, CKind)) {
Diag(Tok, diag::err_omp_unexpected_clause) << getOpenMPClauseName(CKind)
<<
getOpenMPDirectiveName(DKind);
ErrorFound = true;
+WrongDirective = true;
}
switch (CKind) {
@@ -1253,9 +1255,9 @@ OMPClause *Parser::ParseOpenMPClause(Ope
}
if (CKind == OMPC_ordered && PP.LookAhead(/*N=*/0).isNot(tok::l_paren))
- Clause = ParseOp
[PATCH] D41841: [OpenMP] Fix handling of clause on wrong directive
This revision was automatically updated to reflect the committed changes.
Closed by commit rL322107: [OpenMP] Fix handling of clause on wrong directive,
by Joel. E. Denny (authored by ABataev, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D41841?vs=128993&id=129132#toc
Repository:
rL LLVM
https://reviews.llvm.org/D41841
Files:
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/test/OpenMP/distribute_simd_loop_messages.cpp
cfe/trunk/test/OpenMP/for_misc_messages.c
cfe/trunk/test/OpenMP/parallel_messages.cpp
cfe/trunk/test/OpenMP/simd_loop_messages.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_messages.cpp
cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp
Index: cfe/trunk/include/clang/Parse/Parser.h
===
--- cfe/trunk/include/clang/Parse/Parser.h
+++ cfe/trunk/include/clang/Parse/Parser.h
@@ -2675,30 +2675,42 @@
/// \brief Parses clause with a single expression of a kind \a Kind.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
- OMPClause *ParseOpenMPSingleExprClause(OpenMPClauseKind Kind);
+ OMPClause *ParseOpenMPSingleExprClause(OpenMPClauseKind Kind,
+ bool ParseOnly);
/// \brief Parses simple clause of a kind \a Kind.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
- OMPClause *ParseOpenMPSimpleClause(OpenMPClauseKind Kind);
+ OMPClause *ParseOpenMPSimpleClause(OpenMPClauseKind Kind, bool ParseOnly);
/// \brief Parses clause with a single expression and an additional argument
/// of a kind \a Kind.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
- OMPClause *ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind);
+ OMPClause *ParseOpenMPSingleExprWithArgClause(OpenMPClauseKind Kind,
+bool ParseOnly);
/// \brief Parses clause without any additional arguments.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
- OMPClause *ParseOpenMPClause(OpenMPClauseKind Kind);
+ OMPClause *ParseOpenMPClause(OpenMPClauseKind Kind, bool ParseOnly = false);
/// \brief Parses clause with the list of variables of a kind \a Kind.
///
/// \param Kind Kind of current clause.
+ /// \param ParseOnly true to skip the clause's semantic actions and return
+ /// nullptr.
///
OMPClause *ParseOpenMPVarListClause(OpenMPDirectiveKind DKind,
- OpenMPClauseKind Kind);
+ OpenMPClauseKind Kind, bool ParseOnly);
public:
/// Parses simple expression in parens for single-expression clauses of OpenMP
Index: cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp
===
--- cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp
+++ cfe/trunk/test/OpenMP/taskloop_loop_messages.cpp
@@ -294,10 +294,8 @@
c[ii] = a[ii];
#pragma omp parallel
-// expected-error@+2 {{unexpected OpenMP clause 'linear' in directive '#pragma omp taskloop'}}
-// expected-note@+1 {{defined as linear}}
+// expected-error@+1 {{unexpected OpenMP clause 'linear' in directive '#pragma omp taskloop'}}
#pragma omp taskloop linear(ii)
-// expected-error@+1 {{loop iteration variable in the associated loop of 'omp taskloop' directive may not be linear, predetermined as private}}
for (ii = 0; ii < 10; ii++)
c[ii] = a[ii];
Index: cfe/trunk/test/OpenMP/for_misc_messages.c
===
--- cfe/trunk/test/OpenMP/for_misc_messages.c
+++ cfe/trunk/test/OpenMP/for_misc_messages.c
@@ -54,6 +54,20 @@
#pragma omp for foo bar
for (i = 0; i < 16; ++i)
;
+// At one time, this failed an assert.
+// expected-error@+1 {{unexpected OpenMP clause 'num_teams' in directive '#pragma omp for'}}
+#pragma omp for num_teams(3)
+ for (i = 0; i < 16; ++i)
+;
+// At one time, this error was reported twice.
+// expected-error@+1 {{unexpected OpenMP clause 'uniform' in directive '#pragma omp for'}}
+#pragma omp for uniform
+ for (i = 0; i < 16; ++i)
+;
+// expected-error@+1 {{unexpected OpenMP clause 'if' in directive '#pragma omp for'}}
+#pragma omp for if(0)
+ for (i = 0; i < 16; ++i)
+;
}
void test_non_identifiers() {
Index: cfe/trunk/test/OpenMP/simd_loop_messages.cpp
===
--- cfe/trunk/test/OpenMP/simd_loop_messages.cpp
+++ cfe/trunk/test/OpenMP/simd_loop_messages.cpp
@@ -236,9 +236,7 @@
for (ii = 0;
[PATCH] D41755: [CMake] Collect target names in the global LLVM_RUNTIMES property
phosek updated this revision to Diff 129138.
Repository:
rCXX libc++
https://reviews.llvm.org/D41755
Files:
lib/CMakeLists.txt
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -356,10 +356,11 @@
if (LIBCXX_INSTALL_EXPERIMENTAL_LIBRARY)
set(experimental_lib cxx_experimental)
endif()
- install(TARGETS ${LIBCXX_TARGETS} ${experimental_lib}
+ install(TARGETS ${LIBCXX_TARGETS} ${experimental_lib} EXPORT LLVMRuntimes
LIBRARY DESTINATION ${LIBCXX_INSTALL_PREFIX}lib${LIBCXX_LIBDIR_SUFFIX}
COMPONENT cxx
ARCHIVE DESTINATION ${LIBCXX_INSTALL_PREFIX}lib${LIBCXX_LIBDIR_SUFFIX}
COMPONENT cxx
)
+ set_property(GLOBAL APPEND PROPERTY LLVM_RUNTIMES ${LIBCXX_TARGETS}
${experimental_lib})
# NOTE: This install command must go after the cxx install command otherwise
# it will not be executed after the library symlinks are installed.
if (LIBCXX_ENABLE_SHARED AND LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -356,10 +356,11 @@
if (LIBCXX_INSTALL_EXPERIMENTAL_LIBRARY)
set(experimental_lib cxx_experimental)
endif()
- install(TARGETS ${LIBCXX_TARGETS} ${experimental_lib}
+ install(TARGETS ${LIBCXX_TARGETS} ${experimental_lib} EXPORT LLVMRuntimes
LIBRARY DESTINATION ${LIBCXX_INSTALL_PREFIX}lib${LIBCXX_LIBDIR_SUFFIX} COMPONENT cxx
ARCHIVE DESTINATION ${LIBCXX_INSTALL_PREFIX}lib${LIBCXX_LIBDIR_SUFFIX} COMPONENT cxx
)
+ set_property(GLOBAL APPEND PROPERTY LLVM_RUNTIMES ${LIBCXX_TARGETS} ${experimental_lib})
# NOTE: This install command must go after the cxx install command otherwise
# it will not be executed after the library symlinks are installed.
if (LIBCXX_ENABLE_SHARED AND LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41754: [CMake] Collect target names in the global LLVM_RUNTIMES property
phosek updated this revision to Diff 129140.
Repository:
rCXXA libc++abi
https://reviews.llvm.org/D41754
Files:
src/CMakeLists.txt
Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -180,10 +180,11 @@
add_custom_target(cxxabi DEPENDS ${LIBCXXABI_TARGETS})
if (LIBCXXABI_INSTALL_LIBRARY)
- install(TARGETS ${LIBCXXABI_TARGETS}
+ install(TARGETS ${LIBCXXABI_TARGETS} EXPORT LLVMRuntimes
LIBRARY DESTINATION
${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi
ARCHIVE DESTINATION
${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi
)
+ set_property(GLOBAL APPEND PROPERTY LLVM_RUNTIMES ${LIBCXXABI_TARGETS})
endif()
if (NOT CMAKE_CONFIGURATION_TYPES AND LIBCXXABI_INSTALL_LIBRARY)
Index: src/CMakeLists.txt
===
--- src/CMakeLists.txt
+++ src/CMakeLists.txt
@@ -180,10 +180,11 @@
add_custom_target(cxxabi DEPENDS ${LIBCXXABI_TARGETS})
if (LIBCXXABI_INSTALL_LIBRARY)
- install(TARGETS ${LIBCXXABI_TARGETS}
+ install(TARGETS ${LIBCXXABI_TARGETS} EXPORT LLVMRuntimes
LIBRARY DESTINATION ${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi
ARCHIVE DESTINATION ${LIBCXXABI_INSTALL_PREFIX}lib${LIBCXXABI_LIBDIR_SUFFIX} COMPONENT cxxabi
)
+ set_property(GLOBAL APPEND PROPERTY LLVM_RUNTIMES ${LIBCXXABI_TARGETS})
endif()
if (NOT CMAKE_CONFIGURATION_TYPES AND LIBCXXABI_INSTALL_LIBRARY)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322112 - [OPENMP] Fix directive kind on stand-alone target data directives, NFC.
Author: abataev Date: Tue Jan 9 11:59:25 2018 New Revision: 322112 URL: http://llvm.org/viewvc/llvm-project?rev=322112&view=rev Log: [OPENMP] Fix directive kind on stand-alone target data directives, NFC. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=322112&r1=322111&r2=322112&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Jan 9 11:59:25 2018 @@ -7662,7 +7662,7 @@ void CGOpenMPRuntime::emitTargetDataStan if (D.hasClausesOfKind()) CGF.EmitOMPTargetTaskBasedDirective(D, ThenGen, InputInfo); else - emitInlinedDirective(CGF, OMPD_target_update, ThenGen); + emitInlinedDirective(CGF, D.getDirectiveKind(), ThenGen); }; if (IfCond) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37650: [continued] Add a way to get the CXCursor that corresponds to a completion result
svenbrauch added a comment. I know it breaks ABI compatiblity, I just don't know whether it needs to be kept (e.g. for clang-6.0). The alternative requires sorting by a separate index list which is a bit ugly (and slower). If you feel like working on it in the next few days that would of course be great, otherwise if it is clear that this solution doesn't work, I will pick it up again later. Thanks! Repository: rC Clang https://reviews.llvm.org/D37650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41213: [libcxx] [test] Improve MSVC portability.
STL_MSFT added a comment. Ping? (And happy Patch Tuesday!) https://reviews.llvm.org/D41213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41867: [Frontend] Remove unused FileMgr in pp arg parse
v.g.vassilev accepted this revision. v.g.vassilev added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D41867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r322116 - Try to fix build failure caused by r322097
Author: ioeric
Date: Tue Jan 9 12:26:49 2018
New Revision: 322116
URL: http://llvm.org/viewvc/llvm-project?rev=322116&view=rev
Log:
Try to fix build failure caused by r322097
Avoid mapping during output when Detail is nullptr; otherwise, an empty
"Detail" field will be populated in YAML output.
Modified:
clang-tools-extra/trunk/clangd/index/SymbolYAML.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=322116&r1=322115&r2=322116&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Tue Jan 9 12:26:49 2018
@@ -92,7 +92,8 @@ template <> struct MappingTraits
IO.mapOptional("CompletionSnippetInsertText",
Sym.CompletionSnippetInsertText);
-IO.mapOptional("Detail", Sym.Detail);
+if (!IO.outputting() || Sym.Detail)
+ IO.mapOptional("Detail", Sym.Detail);
}
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r322112 - [OPENMP] Fix directive kind on stand-alone target data directives, NFC.
Why is this NFC and doesn't change a test? Am 2018-01-09 20:59, schrieb Alexey Bataev via cfe-commits: Author: abataev Date: Tue Jan 9 11:59:25 2018 New Revision: 322112 URL: http://llvm.org/viewvc/llvm-project?rev=322112&view=rev Log: [OPENMP] Fix directive kind on stand-alone target data directives, NFC. Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=322112&r1=322111&r2=322112&view=diff == --- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original) +++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Jan 9 11:59:25 2018 @@ -7662,7 +7662,7 @@ void CGOpenMPRuntime::emitTargetDataStan if (D.hasClausesOfKind()) CGF.EmitOMPTargetTaskBasedDirective(D, ThenGen, InputInfo); else - emitInlinedDirective(CGF, OMPD_target_update, ThenGen); + emitInlinedDirective(CGF, D.getDirectiveKind(), ThenGen); }; if (IfCond) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41516: emmintrin.h documentation fixes and updates
efriedma added inline comments.
Comment at: cfe/trunk/lib/Headers/emmintrin.h:4683
///
-/// This intrinsic has no corresponding instruction.
+/// This intrinsic corresponds to the MOVDQ2Q instruction.
///
kromanova wrote:
> efriedma wrote:
> > kromanova wrote:
> > > kromanova wrote:
> > > > I'm not sure about this change.
> > > >
> > > > Intel documentation says they generate MOVDQ2Q (don't have icc handy to
> > > > try).
> > > > However, I've tried on Linux/X86_64 with clang and gcc, - and we just
> > > > return.
> > > >
> > > Though I suspect it's possible to generate movdq2q, I couldn't come up
> > > with an test to trigger this instruction generation.
> > > Should we revert this change?
> > >
> > >
> > > ```
> > > __m64 fooepi64_pi64 (__m128i a, __m128 c)
> > > {
> > > __m64 x;
> > >
> > > x = _mm_movepi64_pi64 (a);
> > > return x;
> > > }
> > >
> > > ```
> > >
> > > on Linux we generate return instruction.
> > > I would expect (v)movq %xmm0,%rax to be generated instead of retq.
> > > Am I missing something? Why do we return 64 bit integer in xmm register
> > > rather than in %rax?
> > >
> > The x86-64 calling convention rules say that __m64 is passed/returned in
> > SSE registers.
> >
> > Try the following, which generates movdq2q:
> > ```
> > __m64 foo(__m128i a, __m128 c)
> > {
> > return _mm_add_pi8(_mm_movepi64_pi64(a), _mm_set1_pi8(5));
> > }
> > ```
> Thanks! That explains it :)
> I can see that MOVDQ2Q gets generated.
>
> What about intrinsic below, _mm_movpi64_epi64? Can we ever generate
> MOVD+VMOVQ as stated in the review?
> Or should we write VMOVQ / MOVQ?
Testcase:
```
#include
__m128 foo(__m128i a, __m128 c)
{
return _mm_movpi64_epi64(_mm_add_pi8(_mm_movepi64_pi64(a), _mm_set1_pi8(5)));
}
```
In this case, we should generate movq2dq, but currently don't (I assume due to
a missing DAGCombine). I don't see how you could ever get MOVD... but see
https://reviews.llvm.org/rL321898, which could be the source of some confusion.
Repository:
rL LLVM
https://reviews.llvm.org/D41516
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning
oren_ben_simhon created this revision.
oren_ben_simhon added reviewers: erichkeane, craig.topper, AndreiGrischenko,
aaboud.
Herald added subscribers: llvm-commits, javed.absar.
The patch adds nocf_check target independent attribute for disabling checks
that were enabled by cf-protection flag.
The attribute can be appertained to functions and function pointers.
Attribute name follows GCC's similar attribute name.
Please see the following for more information:
https://reviews.llvm.org/D41879
Repository:
rL LLVM
https://reviews.llvm.org/D41880
Files:
include/clang/AST/Type.h
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
include/clang/Sema/Sema.h
lib/AST/Type.cpp
lib/AST/TypePrinter.cpp
lib/CodeGen/CGCall.cpp
lib/Sema/SemaDeclAttr.cpp
lib/Sema/SemaType.cpp
test/CodeGen/attributes.c
test/CodeGen/cetintrin.c
test/CodeGen/x86-cf-protection.c
test/Sema/attr-nocf_check.c
Index: test/Sema/attr-nocf_check.c
===
--- /dev/null
+++ test/Sema/attr-nocf_check.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+
+// Function pointer definition.
+typedef void (*FuncPointerWithNoCfCheck)(void) __attribute__((nocf_check)); // no-warning
+typedef void (*FuncPointer)(void);
+
+// Allow function declaration and definition mismatch.
+void __attribute__((nocf_check)) testNoCfCheck(); // no-warning
+void __attribute__((nocf_check)) testNoCfCheck(){}; // no-warning
+
+// No variable or parameter declaration
+__attribute__((nocf_check)) int i;// expected-warning {{'nocf_check' attribute only applies to functions and methods}}
+void testNoCfCheckImpl(double __attribute__((nocf_check)) i) {} // expected-warning {{'nocf_check' attribute only applies to functions and methods}}
+
+// Allow attributed function pointers as well as casting between attributed
+// and non-attributed function pointers.
+void testNoCfCheckMismatch(FuncPointer f) {
+ FuncPointerWithNoCfCheck fNoCfCheck = f; // no-warning
+ (*fNoCfCheck)(); // no-warning
+ f = fNoCfCheck;// no-warning
+}
+
+// 'nocf_check' Attribute has no parameters.
+int testNoCfCheckParams() __attribute__((nocf_check(1))); // expected-error {{'nocf_check' attribute takes no arguments}}
Index: test/CodeGen/x86-cf-protection.c
===
--- test/CodeGen/x86-cf-protection.c
+++ test/CodeGen/x86-cf-protection.c
@@ -1,5 +1,5 @@
-// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN
-// RUN: not %clang_cc1 -fsyntax-only -S -emit-llvm -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH
+// RUN: not %clang_cc1 -fsyntax-only -S -triple i386-unknown-unknown -fcf-protection=return %s 2>&1 | FileCheck %s --check-prefix=RETURN
+// RUN: not %clang_cc1 -fsyntax-only -S -triple i386-unknown-unknown -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=BRANCH
// RETURN: error: option 'cf-protection=return' cannot be specified without '-mshstk'
// BRANCH: error: option 'cf-protection=branch' cannot be specified without '-mibt'
Index: test/CodeGen/cetintrin.c
===
--- test/CodeGen/cetintrin.c
+++ test/CodeGen/cetintrin.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -ffreestanding %s -triple=i386-apple-darwin -target-feature +shstk -emit-llvm -o - -Wall -Werror | FileCheck %s
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +shstk -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefix=X86_64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-unknown-unknown -target-feature +shstk -emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-unknown -target-feature +shstk -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefix=X86_64
#include
Index: test/CodeGen/attributes.c
===
--- test/CodeGen/attributes.c
+++ test/CodeGen/attributes.c
@@ -97,8 +97,20 @@
// CHECK: define void @t22() [[NUW]] section ".bar"
+// CHECK: define void @t23() [[NOCF_CHECK_FUNC:#[0-9]+]]
+void __attribute__((nocf_check)) t23(void) {}
+
+// CHECK: call void %{{[a-z0-9]+}}() [[NOCF_CHECK_CALL:#[0-9]+]]
+typedef void (*f_t)(void);
+void t24(f_t f1) {
+ __attribute__((nocf_check)) f_t p = f1;
+ (*p)();
+}
+
// CHECK: attributes [[NUW]] = { noinline nounwind{{.*}} }
// CHECK: attributes [[NR]] = { noinline noreturn nounwind{{.*}} }
// CHECK: attributes [[COLDDEF]] = { cold {{.*}}}
// CHECK: attributes [[COLDDECL]] = { cold {{.*}}}
+// CHECK: attributes [[NOCF_CHECK_FUNC]] = { {{.*}} nocf_check {{.*}}}
// CHECK: attributes [[COLDSITE]] = { cold {{.*}}}
+// CHECK: attributes [[NOCF_CHECK_CALL]] = { nocf_check }
Index:
[PATCH] D41881: [analyzer] Flag bcmp, bcopy and bzero as obsolete
trixirt created this revision.
trixirt added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.
bcmp, bcopy and bzero are obsolete functions.
Flag them as such so users will not use them.
Repository:
rC Clang
https://reviews.llvm.org/D41881
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
test/Analysis/security-syntax-checks.m
www/analyzer/available_checks.html
Index: www/analyzer/available_checks.html
===
--- www/analyzer/available_checks.html
+++ www/analyzer/available_checks.html
@@ -1173,6 +1173,40 @@
+security.insecureAPI.bcmp
+(C)
+Warn on uses of the bcmp function.
+
+
+void test() {
+ bcmp(ptr0, ptr1, n); // warn
+}
+
+
+
+security.insecureAPI.bcopy
+(C)
+Warn on uses of the bcopy function.
+
+
+void test() {
+ bcopy(src, dst, n); // warn
+}
+
+
+
+security.insecureAPI.bzero
+(C)
+Warn on uses of the bzero function.
+
+
+void test() {
+ bzero(ptr, n); // warn
+}
+
+
+
+
security.insecureAPI.getpw
(C)
Warn on uses of the getpw function.
Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -37,6 +37,27 @@
for (FooType x = 10001.0f; x <= 10010.0f; x++ ) {} // expected-warning{{Variable 'x' with floating point type 'FooType'}}
}
+// Obsolete function bcmp
+int bcmp(void *, void *, size_t);
+
+int test_bcmp(void *a, void *b, size_t n) {
+ return bcmp(a, b, n); // expected-warning{{The bcmp() function is obsoleted by memcmp()}}
+}
+
+// Obsolete function bcopy
+void bcopy(void *, void *, size_t);
+
+void test_bcopy(void *a, void *b, size_t n) {
+ bcopy(a, b, n); // expected-warning{{The bcopy() function is obsoleted by memcpy() or memmove(}}
+}
+
+// Obsolete function bzero
+void bzero(void *, size_t);
+
+void test_bzero(void *a, size_t n) {
+ bzero(a, n); // expected-warning{{The bzero() function is obsoleted by memset()}}
+}
+
// rule request: gets() buffer overflow
// Part of recommendation: 300-BSI (buildsecurityin.us-cert.gov)
char* gets(char *buf);
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -37,6 +37,9 @@
namespace {
struct ChecksFilter {
+ DefaultBool check_bcmp;
+ DefaultBool check_bcopy;
+ DefaultBool check_bzero;
DefaultBool check_gets;
DefaultBool check_getpw;
DefaultBool check_mktemp;
@@ -47,6 +50,9 @@
DefaultBool check_FloatLoopCounter;
DefaultBool check_UncheckedReturn;
+ CheckName checkName_bcmp;
+ CheckName checkName_bcopy;
+ CheckName checkName_bzero;
CheckName checkName_gets;
CheckName checkName_getpw;
CheckName checkName_mktemp;
@@ -89,6 +95,9 @@
// Checker-specific methods.
void checkLoopConditionForFloat(const ForStmt *FS);
+ void checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_bcopy(const CallExpr *CE, const FunctionDecl *FD);
+ void checkCall_bzero(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_gets(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD);
void checkCall_mktemp(const CallExpr *CE, const FunctionDecl *FD);
@@ -129,6 +138,9 @@
// Set the evaluation function by switching on the callee name.
FnCheck evalFunction = llvm::StringSwitch(Name)
+.Case("bcmp", &WalkAST::checkCall_bcmp)
+.Case("bcopy", &WalkAST::checkCall_bcopy)
+.Case("bzero", &WalkAST::checkCall_bzero)
.Case("gets", &WalkAST::checkCall_gets)
.Case("getpw", &WalkAST::checkCall_getpw)
.Case("mktemp", &WalkAST::checkCall_mktemp)
@@ -296,6 +308,132 @@
}
//===--===//
+// Check: Any use of bcmp.
+// CWE-477: Use of Obsolete Functions
+// bcmp was deprecated in POSIX.1-2008
+//===--===//
+
+void WalkAST::checkCall_bcmp(const CallExpr *CE, const FunctionDecl *FD) {
+ if (!filter.check_bcmp)
+return;
+
+ const FunctionProtoType *FPT = FD->getType()->getAs();
+ if (!FPT)
+return;
+
+ // Verify that the function takes three arguments.
+ if (FPT->getNumParams() != 3)
+return;
+
+ for (int i = 0; i < 2; i++) {
+// Verify the first and second argument type is void*.
+const PointerType *PT = FPT->getParamType(i)->getAs();
+if (!PT)
+ return;
+
+if (PT->getPointeeType().getUnqualifiedType() != BR.getContext().VoidTy)
+ return;
+ }
+
+ // Verify the third argument type is integer.
+ if (!FPT->getParamType(2)->isIntegralOrUnscopedEnumerationType())
+return;
+
+ // Issue a warning.
+ Pat
[PATCH] D41213: [libcxx] [test] Improve MSVC portability.
mclow.lists added a comment. > According to 15.8.2 [class.copy.assign]/2 and /4, this makes call_wrapper > non-assignable. Then I think we've got a LWG issue, because that's not what I remember the intent to have been. https://reviews.llvm.org/D41213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41867: [Frontend] Remove unused FileMgr in pp arg parse
This revision was automatically updated to reflect the committed changes.
Closed by commit rC322118: [Frontend] Remove unused FileMgr in pp arg parse
(authored by modocache, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41867?vs=129089&id=129152#toc
Repository:
rC Clang
https://reviews.llvm.org/D41867
Files:
lib/Frontend/CompilerInvocation.cpp
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2618,7 +2618,6 @@
}
static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
- FileManager &FileMgr,
DiagnosticsEngine &Diags,
frontend::ActionKind Action) {
using namespace options;
@@ -2840,12 +2839,7 @@
!LangOpts.Sanitize.has(SanitizerKind::Address) &&
!LangOpts.Sanitize.has(SanitizerKind::Memory);
- // FIXME: ParsePreprocessorArgs uses the FileManager to read the contents of
- // PCH file and find the original header name. Remove the need to do that in
- // ParsePreprocessorArgs and remove the FileManager
- // parameters from the function and the "FileManager.h" #include.
- FileManager FileMgr(Res.getFileSystemOpts());
- ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, FileMgr, Diags,
+ ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, Diags,
Res.getFrontendOpts().ProgramAction);
ParsePreprocessorOutputArgs(Res.getPreprocessorOutputOpts(), Args,
Res.getFrontendOpts().ProgramAction);
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2618,7 +2618,6 @@
}
static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
- FileManager &FileMgr,
DiagnosticsEngine &Diags,
frontend::ActionKind Action) {
using namespace options;
@@ -2840,12 +2839,7 @@
!LangOpts.Sanitize.has(SanitizerKind::Address) &&
!LangOpts.Sanitize.has(SanitizerKind::Memory);
- // FIXME: ParsePreprocessorArgs uses the FileManager to read the contents of
- // PCH file and find the original header name. Remove the need to do that in
- // ParsePreprocessorArgs and remove the FileManager
- // parameters from the function and the "FileManager.h" #include.
- FileManager FileMgr(Res.getFileSystemOpts());
- ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, FileMgr, Diags,
+ ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, Diags,
Res.getFrontendOpts().ProgramAction);
ParsePreprocessorOutputArgs(Res.getPreprocessorOutputOpts(), Args,
Res.getFrontendOpts().ProgramAction);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322118 - [Frontend] Remove unused FileMgr in pp arg parse
Author: modocache
Date: Tue Jan 9 13:26:47 2018
New Revision: 322118
URL: http://llvm.org/viewvc/llvm-project?rev=322118&view=rev
Log:
[Frontend] Remove unused FileMgr in pp arg parse
Summary:
A FIXME added 8 years ago (2010) in https://reviews.llvm.org/rL118203
mentioned that a FileManager should not need to be used when parsing
preprocessor arguments. In fact, its only use was removed 6 years ago
(2012), in https://reviews.llvm.org/rL166452. Remove the unused
variable and the obsolete FIXME.
Test Plan: `check-clang`
Reviewers: v.g.vassilev
Reviewed By: v.g.vassilev
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D41867
Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=322118&r1=322117&r2=322118&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Jan 9 13:26:47 2018
@@ -2618,7 +2618,6 @@ static bool isStrictlyPreprocessorAction
}
static void ParsePreprocessorArgs(PreprocessorOptions &Opts, ArgList &Args,
- FileManager &FileMgr,
DiagnosticsEngine &Diags,
frontend::ActionKind Action) {
using namespace options;
@@ -2840,12 +2839,7 @@ bool CompilerInvocation::CreateFromArgs(
!LangOpts.Sanitize.has(SanitizerKind::Address) &&
!LangOpts.Sanitize.has(SanitizerKind::Memory);
- // FIXME: ParsePreprocessorArgs uses the FileManager to read the contents of
- // PCH file and find the original header name. Remove the need to do that in
- // ParsePreprocessorArgs and remove the FileManager
- // parameters from the function and the "FileManager.h" #include.
- FileManager FileMgr(Res.getFileSystemOpts());
- ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, FileMgr, Diags,
+ ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, Diags,
Res.getFrontendOpts().ProgramAction);
ParsePreprocessorOutputArgs(Res.getPreprocessorOutputOpts(), Args,
Res.getFrontendOpts().ProgramAction);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41867: [Frontend] Remove unused FileMgr in pp arg parse
modocache added a comment. Great, thanks for the review, @v.g.vassilev! Repository: rC Clang https://reviews.llvm.org/D41867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41406: [analyzer] Add a new checker callback, check::NewAllocator.
NoQ added a comment.
> which would re-evaluate the cast and return the casted object
Rather not. Because i'm changing my mind again about avoiding the redundant
cast in `&element{T, HeapSymRegion{conj}}` - this time by not calling
`evalCast` after a conservatively evaluated operator new call (this would
ensure that no existing behavior breaks in the conservative case without
changing how all casts everywhere work), and this wouldn't be compatible with
this `CallEvent`-based approach because in the `CallEvent` we have no way of
figuring out if the call was inlined or evaluated conservatively. We could
still move the cast logic before the `PostCall` callback, and then retrieve the
casted value from the program state (wherever it is). But this is an example
of stuff that becomes harder when we merge the callbacks together, and it took
me a few hours to stumble upon that, so who knows what other problems would we
encounter, so i feel that future-proof-ness is worth it.
https://reviews.llvm.org/D41406
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r322126 - Wire up GCOV to the new pass manager
Author: dblaikie
Date: Tue Jan 9 14:03:47 2018
New Revision: 322126
URL: http://llvm.org/viewvc/llvm-project?rev=322126&view=rev
Log:
Wire up GCOV to the new pass manager
GCOV in the old pass manager also strips debug info (if debug info is
disabled/only produced for profiling anyway) after the GCOV pass runs.
I think the strip pass hasn't been ported to the new pass manager, so it
might take me a little while to wire that up.
Modified:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/test/CodeGen/code-coverage.c
Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=322126&r1=322125&r2=322126&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Tue Jan 9 14:03:47 2018
@@ -26,6 +26,7 @@
#include "llvm/Bitcode/BitcodeWriterPass.h"
#include "llvm/CodeGen/RegAllocRegistry.h"
#include "llvm/CodeGen/SchedulerRegistry.h"
+#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/IRPrintingPasses.h"
#include "llvm/IR/LegacyPassManager.h"
@@ -44,8 +45,8 @@
#include "llvm/Support/raw_ostream.h"
#include "llvm/Target/TargetMachine.h"
#include "llvm/Target/TargetOptions.h"
-#include "llvm/CodeGen/TargetSubtargetInfo.h"
#include "llvm/Transforms/Coroutines.h"
+#include "llvm/Transforms/GCOVProfiler.h"
#include "llvm/Transforms/IPO.h"
#include "llvm/Transforms/IPO/AlwaysInliner.h"
#include "llvm/Transforms/IPO/PassManagerBuilder.h"
@@ -471,6 +472,23 @@ static void initTargetOptions(llvm::Targ
Options.MCOptions.IASSearchPaths.push_back(
Entry.IgnoreSysRoot ? Entry.Path : HSOpts.Sysroot + Entry.Path);
}
+static Optional getGCOVOptions(const CodeGenOptions &CodeGenOpts)
{
+ if (CodeGenOpts.DisableGCov)
+return None;
+ if (!CodeGenOpts.EmitGcovArcs && !CodeGenOpts.EmitGcovNotes)
+return None;
+ // Not using 'GCOVOptions::getDefault' allows us to avoid exiting if
+ // LLVM's -default-gcov-version flag is set to something invalid.
+ GCOVOptions Options;
+ Options.EmitNotes = CodeGenOpts.EmitGcovNotes;
+ Options.EmitData = CodeGenOpts.EmitGcovArcs;
+ llvm::copy(CodeGenOpts.CoverageVersion, std::begin(Options.Version));
+ Options.UseCfgChecksum = CodeGenOpts.CoverageExtraChecksum;
+ Options.NoRedZone = CodeGenOpts.DisableRedZone;
+ Options.FunctionNamesInData = !CodeGenOpts.CoverageNoFunctionNamesInData;
+ Options.ExitBlockBeforeBody = CodeGenOpts.CoverageExitBlockBeforeBody;
+ return Options;
+}
void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
legacy::FunctionPassManager &FPM) {
@@ -613,20 +631,8 @@ void EmitAssemblyHelper::CreatePasses(le
if (!CodeGenOpts.RewriteMapFiles.empty())
addSymbolRewriterPass(CodeGenOpts, &MPM);
- if (!CodeGenOpts.DisableGCov &&
- (CodeGenOpts.EmitGcovArcs || CodeGenOpts.EmitGcovNotes)) {
-// Not using 'GCOVOptions::getDefault' allows us to avoid exiting if
-// LLVM's -default-gcov-version flag is set to something invalid.
-GCOVOptions Options;
-Options.EmitNotes = CodeGenOpts.EmitGcovNotes;
-Options.EmitData = CodeGenOpts.EmitGcovArcs;
-memcpy(Options.Version, CodeGenOpts.CoverageVersion, 4);
-Options.UseCfgChecksum = CodeGenOpts.CoverageExtraChecksum;
-Options.NoRedZone = CodeGenOpts.DisableRedZone;
-Options.FunctionNamesInData =
-!CodeGenOpts.CoverageNoFunctionNamesInData;
-Options.ExitBlockBeforeBody = CodeGenOpts.CoverageExitBlockBeforeBody;
-MPM.add(createGCOVProfilerPass(Options));
+ if (Optional Options = getGCOVOptions(CodeGenOpts)) {
+MPM.add(createGCOVProfilerPass(*Options));
if (CodeGenOpts.getDebugInfo() == codegenoptions::NoDebugInfo)
MPM.add(createStripSymbolsPass(true));
}
@@ -954,6 +960,9 @@ void EmitAssemblyHelper::EmitAssemblyWit
CodeGenOpts.DebugPassManager);
}
}
+if (Optional Options = getGCOVOptions(CodeGenOpts)) {
+ MPM.addPass(GCOVProfilerPass(*Options));
+}
}
// FIXME: We still use the legacy pass manager to do code generation. We
Modified: cfe/trunk/test/CodeGen/code-coverage.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/code-coverage.c?rev=322126&r1=322125&r2=322126&view=diff
==
--- cfe/trunk/test/CodeGen/code-coverage.c (original)
+++ cfe/trunk/test/CodeGen/code-coverage.c Tue Jan 9 14:03:47 2018
@@ -2,7 +2,10 @@
// RUN: %clang_cc1 -emit-llvm -disable-red-zone -femit-coverage-data
-coverage-no-function-names-in-data %s -o - | FileCheck %s --check-prefix
WITHOUTNAMES
// RUN: %clang_cc1 -emit-llvm -disable-red-zone -femit-coverage-data
-coverage-notes-file=aaa.gcno -coverage-data-file=bbb.gcda -dwarf-column-info
-debug-info-kind=limited -dwa
[PATCH] D41213: [libcxx] [test] Improve MSVC portability.
mclow.lists added a comment. Investigating the assignability of `not_fn`. All the rest of this looks fine. https://reviews.llvm.org/D41213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
