[PATCH] D45444: [clang-tidy] implement new check for const-correctness
JonasToth added a comment. Thank you very much :) Am 12.08.2018 um 00:17 schrieb Shuai Wang via Phabricator: > shuaiwang added a comment. > > In https://reviews.llvm.org/D45444#1196271, @JonasToth wrote: > >> Always the same with the templates ;) So uninstantiated templates should >> >> just be ignored. >> >> I think it would be better to have it in the ExprMutAnalyzer, because >> >> that part can not decide on const-ness. Fixing it here would just >> circumvent the problem but not fix it, would you agree? > > Agreed :) > I'll create a diff handling various template related cases in > ExprMutationAnalyzer. > > Repository: > > rCTE Clang Tools Extra > > https://reviews.llvm.org/D45444 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r339428 - Add Windows support for the GNUstep Objective-C ABI V2.
Merged both of these to 7.0 in r339538. On Sat, Aug 11, 2018 at 10:34 AM, David Chisnall via cfe-commits wrote: > Thanks, > > That fix looks exactly what is needed - and is something I didn’t realise > FileCheck could do! The order of those matters in the linked binary, but not > in the IR - the linker will arrange them sensibly based on the subsection > name. > > Thanks again, > > David > >> On 11 Aug 2018, at 04:12, >> wrote: >> >> Hi David, >> >> I made an attempt to fix the Windows bot test failures in r339494 by making >> the checks for the section boundaries not depend on the order they were >> emitted. >> >> It seems that when using Visual Studio to build clang, the compiler that is >> generated emits the __stop_ section boundaries before the __start_ ones, >> thus causing your checks to fail. For example, you were checking for the >> following: >> >> @__start___objc_selectors = linkonce_odr hidden global >> %.objc_section_sentinel zeroinitializer, section "__objc_selectors$a", >> comdat, align 1 >> @__stop__objc_selectors = linkonce_odr hidden global %.objc_section_sentinel >> zeroinitializer, section "__objc_selectors$z", comdat, align 1 >> @__start___objc_classes = linkonce_odr hidden global %.objc_section_sentinel >> zeroinitializer, section "__objc_classes$a", comdat, align 1 >> @__stop__objc_classes = linkonce_odr hidden global %.objc_section_sentinel >> zeroinitializer, section "__objc_classes$z", comdat, align 1 >> >> However the Visual Studio built clang was producing the following output: >> >> @__stop__objc_selectors = linkonce_odr hidden global %.objc_section_sentinel >> zeroinitializer, section "__objc_selectors$z", comdat, align 1 >> @__start___objc_selectors = linkonce_odr hidden global >> %.objc_section_sentinel zeroinitializer, section "__objc_selectors$a", >> comdat, align 1 >> @__stop__objc_classes = linkonce_odr hidden global %.objc_section_sentinel >> zeroinitializer, section "__objc_classes$z", comdat, align 1 >> @__start___objc_classes = linkonce_odr hidden global %.objc_section_sentinel >> zeroinitializer, section "__objc_classes$a", comdat, align 1 >> >> I don’t think that the order matters, only that they are actually emitted, >> but if I am wrong, I apologize, and please revert my change and look into >> the problem. >> >> Douglas Yung >> >> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of >> Tom Weaver via cfe-commits >> Sent: Friday, August 10, 2018 10:01 >> To: David Chisnall >> Cc: cfe-commits@lists.llvm.org >> Subject: Re: r339428 - Add Windows support for the GNUstep Objective-C ABI >> V2. >> >> Hi David, >> >> revision 339428 seems to have caused failing tests on a couple of windows >> build bots, any chance you can take a look please? >> >> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/18985 >> >> >> Failing Tests (1): >> Clang :: CodeGenObjC/gnu-init.m >> >> Expected Passes: 30627 >> Expected Failures : 65 >> Unsupported Tests : 12223 >> Unexpected Failures: 1 >> >> Thanks >> >> Tom Weaver >> >> On 10 August 2018 at 13:53, David Chisnall via cfe-commits >> wrote: >> Author: theraven >> Date: Fri Aug 10 05:53:13 2018 >> New Revision: 339428 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=339428&view=rev >> Log: >> Add Windows support for the GNUstep Objective-C ABI V2. >> >> Summary: >> Introduces funclet-based unwinding for Objective-C and fixes an issue >> where global blocks can't have their isa pointers initialised on >> Windows. >> >> After discussion with Dustin, this changes the name mangling of >> Objective-C types to prevent a C++ catch statement of type struct X* >> from catching an Objective-C object of type X*. >> >> Reviewers: rjmccall, DHowett-MSFT >> >> Reviewed By: rjmccall, DHowett-MSFT >> >> Subscribers: mgrang, mstorsjo, smeenai, cfe-commits >> >> Differential Revision: https://reviews.llvm.org/D50144 >> >> Modified: >> cfe/trunk/include/clang/Driver/Options.td >> cfe/trunk/lib/AST/MicrosoftMangle.cpp >> cfe/trunk/lib/CodeGen/CGException.cpp >> cfe/trunk/lib/CodeGen/CGObjCGNU.cpp >> cfe/trunk/lib/CodeGen/CGObjCRuntime.cpp >> cfe/trunk/lib/CodeGen/CGObjCRuntime.h >> cfe/trunk/lib/CodeGen/CodeGenFunction.h >> cfe/trunk/lib/Driver/ToolChains/Clang.cpp >> cfe/trunk/test/CodeGenObjC/gnu-init.m >> cfe/trunk/test/CodeGenObjC/gnustep2-proto.m >> cfe/trunk/test/CodeGenObjCXX/arc-marker-funclet.mm >> cfe/trunk/test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm >> cfe/trunk/test/CodeGenObjCXX/msabi-objc-extensions.mm >> cfe/trunk/test/CodeGenObjCXX/msabi-objc-types.mm >> >> Modified: cfe/trunk/include/clang/Driver/Options.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=339428&r1=339427&r2=339428&view=diff >> == >> --- cfe/trunk/include/cl
[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check
hokein added inline comments.
Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:24
+
+ const auto IsDuration =
+ expr(hasType(cxxRecordDecl(hasName("::absl::Duration";
maybe call it `DurationExpr` since you have declared the variable as
`expr(...)`.
Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:30
+ hasOverloadedOperatorName("/"),
+ hasArgument(0, expr(IsDuration)),
+ hasArgument(1, expr(IsDuration))).bind("OpCall"))),
`expr(IsDuration)` seems have a duplicated `expr`, isn't `hasArgument(0,
IsDuration())` enough?
Comment at: test/clang-tidy/abseil-duration-division.cpp:18
+
+absl::Duration d;
+
could you make it to a local variable? It willmake the test code easier to
read, otherwise readers have to scroll up the file to see what is variable `d`.
Comment at: test/clang-tidy/abseil-duration-division.cpp:58
+ // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration
objects
+ // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+ // absl::FDivDuration(t1, t2);}
I think we should ignore templates. The template function could be used by
other types, fixing the template is not correct.
https://reviews.llvm.org/D50389
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50580: [clang-tidy] Abseil: no namespace check
hokein added a comment.
The check is missing its document, please add one in `docs/clang-tidy/checks/`.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
aaron.ballman wrote:
> I think this needs a `not(isExpansionInSystemHeader())` in there, as this is
> going to trigger on code that includes a header file using an `absl`
> namespace. If I'm incorrect and users typically include abseil as something
> other than system includes, you'll have to find a different way to solve this.
Yeah, we have discussed this issue internally. abseil-user code usually
includes abseil header like `#include "whatever/abseil/base/optimization.h"`,
so `IsExpansionInSystemHeader` doesn't work for most cases.
We need a way to filter out all headers being a part of abseil library. I think
we can create a matcher `InExpansionInAbseilHeader` -- basically using
`IsExpansionInFileMatching` with a regex expression that matches typical abseil
include path. What do you think?
And we'll have more abseil checks (e.g. `abseil-no-internal-deps`) that use
`InExpansionInAbseilHeader`, we should put it to a common header.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50580
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D21508: Diagnose friend function template redefinitions
sepavloff updated this revision to Diff 160298.
sepavloff added a comment.
Rebased the patch
Repository:
rC Clang
https://reviews.llvm.org/D21508
Files:
include/clang/AST/DeclBase.h
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
test/Modules/friend-definition.cpp
test/SemaCXX/friend2.cpp
Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -129,6 +129,83 @@
void func_22() {} // expected-error{{redefinition of 'func_22'}}
+// Case of template friend functions.
+
+template void func_31(T *x);
+template
+struct C31a {
+ template friend void func_31(T *x) {}
+};
+template
+struct C31b {
+ template friend void func_31(T *x) {}
+};
+
+
+template inline void func_32(T *x) {}
+template
+struct C32a {
+ template friend void func_32(T *x) {}
+};
+template
+struct C32b {
+ template friend void func_32(T *x) {}
+};
+
+
+template
+struct C33a {
+ template friend void func_33(T *x) {}
+};
+template
+struct C33b {
+ template friend void func_33(T *x) {}
+};
+
+
+template inline void func_34(T *x) {} // expected-note{{previous definition is here}}
+template
+struct C34 {
+ template friend void func_34(T *x) {} // expected-error{{redefinition of 'func_34'}}
+};
+
+C34 v34; // expected-note{{in instantiation of template class 'C34' requested here}}
+
+
+template inline void func_35(T *x);
+template
+struct C35a {
+ template friend void func_35(T *x) {} // expected-note{{previous definition is here}}
+};
+template
+struct C35b {
+ template friend void func_35(T *x) {} // expected-error{{redefinition of 'func_35'}}
+};
+
+C35a v35a;
+C35b v35b; // expected-note{{in instantiation of template class 'C35b' requested here}}
+
+
+template void func_36(T *x);
+template
+struct C36 {
+ template friend void func_36(T *x) {} // expected-error{{redefinition of 'func_36'}}
+ // expected-note@-1{{previous definition is here}}
+};
+
+C36 v36a;
+C36 v36b; //expected-note{{in instantiation of template class 'C36' requested here}}
+
+
+template void func_37(T *x);
+template
+struct C37 {
+ template friend void func_37(T *x) {} // expected-note{{previous definition is here}}
+};
+
+C37 v37;
+template void func_37(T *x) {} // expected-error{{redefinition of 'func_37'}}
+
namespace pr22307 {
Index: test/Modules/friend-definition.cpp
===
--- test/Modules/friend-definition.cpp
+++ test/Modules/friend-definition.cpp
@@ -7,6 +7,7 @@
#pragma clang module begin A
template struct A {
friend A operator+(const A&, const A&) { return {}; }
+ template friend void func_1(const A&, const T2 &) {}
};
#pragma clang module end
#pragma clang module endbuild
@@ -36,4 +37,5 @@
void h() {
A a;
a + a;
+ func_1(a, 0);
}
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1793,7 +1793,9 @@
// If the original function was part of a friend declaration,
// inherit its namespace state and add it to the owner.
if (isFriend) {
-PrincipalDecl->setObjectOfFriendDecl();
+Function->setObjectOfFriendDecl();
+if (FunctionTemplateDecl *FT = Function->getDescribedFunctionTemplate())
+ FT->setObjectOfFriendDecl();
DC->makeDeclVisibleInContext(PrincipalDecl);
bool QueuedInstantiation = false;
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12654,6 +12654,29 @@
}
}
}
+
+ if (!Definition)
+// Similar to friend functions a friend function template may be a
+// definition and do not have a body if it is instantiated in a class
+// template.
+if (FunctionTemplateDecl *FTD = FD->getDescribedFunctionTemplate()) {
+ for (auto I : FTD->redecls()) {
+auto D = cast(I);
+if (D != FTD) {
+ assert(!D->isThisDeclarationADefinition() &&
+ "More than one definition in redeclaration chain");
+ if (D->getFriendObjectKind() != Decl::FOK_None)
+if (FunctionTemplateDecl *FT =
+ D->getInstantiatedFromMemberTemplate()) {
+ if (FT->isThisDeclarationADefinition()) {
+Definition = D->getTemplatedDecl();
+break;
+ }
+}
+}
+ }
+}
+
if (!Definition)
return;
Index: include/clang/AST/DeclBase.h
===
--- include/clang/AST/DeclBase.h
+++ include/clang/AST/DeclBase.h
@@ -1078,11 +1078,11 @@
unsigned OldNS = IdentifierNamespace;
assert((OldNS & (IDNS_Tag | IDNS_Ordinary |
IDNS_TagFriend | IDNS_OrdinaryFr
[PATCH] D50443: [clang] Store code completion token range in preprocessor.
kadircet updated this revision to Diff 160299.
kadircet added a comment.
- Resolve discussions.
- Fix typo.
Repository:
rC Clang
https://reviews.llvm.org/D50443
Files:
include/clang/Lex/Preprocessor.h
lib/Lex/Preprocessor.cpp
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -868,6 +868,7 @@
if (Result.is(tok::code_completion) && Result.getIdentifierInfo()) {
// Remember the identifier before code completion token.
setCodeCompletionIdentifierInfo(Result.getIdentifierInfo());
+setCodeCompletionTokenRange(Result.getLocation(), Result.getEndLoc());
// Set IdenfitierInfo to null to avoid confusing code that handles both
// identifiers and completion tokens.
Result.setIdentifierInfo(nullptr);
Index: include/clang/Lex/Preprocessor.h
===
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -310,6 +310,9 @@
/// on the stem that is to be code completed.
IdentifierInfo *CodeCompletionII = nullptr;
+ /// Range for the code completion token.
+ SourceRange CodeCompletionTokenRange;
+
/// The directory that the main file should be considered to occupy,
/// if it does not correspond to a real file (as happens when building a
/// module).
@@ -1131,6 +1134,16 @@
CodeCompletionII = Filter;
}
+ /// Set the code completion token range for detecting replacement range later
+ /// on.
+ void setCodeCompletionTokenRange(const SourceLocation Start,
+ const SourceLocation End) {
+CodeCompletionTokenRange = {Start, End};
+ }
+ SourceRange getCodeCompletionTokenRange() const {
+return CodeCompletionTokenRange;
+ }
+
/// Get the code completion token for filtering purposes.
StringRef getCodeCompletionFilter() {
if (CodeCompletionII)
Index: lib/Lex/Preprocessor.cpp
===
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -868,6 +868,7 @@
if (Result.is(tok::code_completion) && Result.getIdentifierInfo()) {
// Remember the identifier before code completion token.
setCodeCompletionIdentifierInfo(Result.getIdentifierInfo());
+setCodeCompletionTokenRange(Result.getLocation(), Result.getEndLoc());
// Set IdenfitierInfo to null to avoid confusing code that handles both
// identifiers and completion tokens.
Result.setIdentifierInfo(nullptr);
Index: include/clang/Lex/Preprocessor.h
===
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -310,6 +310,9 @@
/// on the stem that is to be code completed.
IdentifierInfo *CodeCompletionII = nullptr;
+ /// Range for the code completion token.
+ SourceRange CodeCompletionTokenRange;
+
/// The directory that the main file should be considered to occupy,
/// if it does not correspond to a real file (as happens when building a
/// module).
@@ -1131,6 +1134,16 @@
CodeCompletionII = Filter;
}
+ /// Set the code completion token range for detecting replacement range later
+ /// on.
+ void setCodeCompletionTokenRange(const SourceLocation Start,
+ const SourceLocation End) {
+CodeCompletionTokenRange = {Start, End};
+ }
+ SourceRange getCodeCompletionTokenRange() const {
+return CodeCompletionTokenRange;
+ }
+
/// Get the code completion token for filtering purposes.
StringRef getCodeCompletionFilter() {
if (CodeCompletionII)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hokein added inline comments.
Comment at: docs/clang-tidy/checks/abseil-no-internal-deps.rst:6
+
+Gives a warning if code using Abseil depends on internal details. If something
is in a namespace or filename/path that includes the word “internal”, code is
not allowed to depend upon it beaucse it’s an implementation detail. They
cannot friend it, include it, you mention it or refer to it in any way. Doing
so violtaes Abseil's compatibility guidelines and may result in breakage.
+
aaron.ballman wrote:
> JonasToth wrote:
> > s/violtaes/violates/
> Please wrap lines to 80 cols.
>
> Nothing in this check looks at filenames and paths, but this suggests the
> check will find those. Is that intended work that's missed in this patch, or
> are the docs incorrect?
Please provide a link for the abseil guideline.
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
nit: please make sure the code follow LLVM code style, even for test code :)
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:11
+
+namespace absl {
+std::string StringsFunction (std::string s1){
Since we have multiple abseil checks that might use these fake abseil
declarations, I'd suggest pull out these to a common header, and include it in
this test file.
https://reviews.llvm.org/D50542
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r339540 - [clang] Store code completion token range in preprocessor.
Author: kadircet
Date: Mon Aug 13 01:13:35 2018
New Revision: 339540
URL: http://llvm.org/viewvc/llvm-project?rev=339540&view=rev
Log:
[clang] Store code completion token range in preprocessor.
Summary:
This change is to support a new fature in clangd, tests will be send
toclang-tools-extra with that change.
Unittests are included in: https://reviews.llvm.org/D50449
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50443
Modified:
cfe/trunk/include/clang/Lex/Preprocessor.h
cfe/trunk/lib/Lex/Preprocessor.cpp
Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=339540&r1=339539&r2=339540&view=diff
==
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Mon Aug 13 01:13:35 2018
@@ -310,6 +310,9 @@ class Preprocessor {
/// on the stem that is to be code completed.
IdentifierInfo *CodeCompletionII = nullptr;
+ /// Range for the code completion token.
+ SourceRange CodeCompletionTokenRange;
+
/// The directory that the main file should be considered to occupy,
/// if it does not correspond to a real file (as happens when building a
/// module).
@@ -1131,6 +1134,16 @@ public:
CodeCompletionII = Filter;
}
+ /// Set the code completion token range for detecting replacement range later
+ /// on.
+ void setCodeCompletionTokenRange(const SourceLocation Start,
+ const SourceLocation End) {
+CodeCompletionTokenRange = {Start, End};
+ }
+ SourceRange getCodeCompletionTokenRange() const {
+return CodeCompletionTokenRange;
+ }
+
/// Get the code completion token for filtering purposes.
StringRef getCodeCompletionFilter() {
if (CodeCompletionII)
Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=339540&r1=339539&r2=339540&view=diff
==
--- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
+++ cfe/trunk/lib/Lex/Preprocessor.cpp Mon Aug 13 01:13:35 2018
@@ -868,6 +868,7 @@ void Preprocessor::Lex(Token &Result) {
if (Result.is(tok::code_completion) && Result.getIdentifierInfo()) {
// Remember the identifier before code completion token.
setCodeCompletionIdentifierInfo(Result.getIdentifierInfo());
+setCodeCompletionTokenRange(Result.getLocation(), Result.getEndLoc());
// Set IdenfitierInfo to null to avoid confusing code that handles both
// identifiers and completion tokens.
Result.setIdentifierInfo(nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50443: [clang] Store code completion token range in preprocessor.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339540: [clang] Store code completion token range in
preprocessor. (authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50443
Files:
cfe/trunk/include/clang/Lex/Preprocessor.h
cfe/trunk/lib/Lex/Preprocessor.cpp
Index: cfe/trunk/lib/Lex/Preprocessor.cpp
===
--- cfe/trunk/lib/Lex/Preprocessor.cpp
+++ cfe/trunk/lib/Lex/Preprocessor.cpp
@@ -868,6 +868,7 @@
if (Result.is(tok::code_completion) && Result.getIdentifierInfo()) {
// Remember the identifier before code completion token.
setCodeCompletionIdentifierInfo(Result.getIdentifierInfo());
+setCodeCompletionTokenRange(Result.getLocation(), Result.getEndLoc());
// Set IdenfitierInfo to null to avoid confusing code that handles both
// identifiers and completion tokens.
Result.setIdentifierInfo(nullptr);
Index: cfe/trunk/include/clang/Lex/Preprocessor.h
===
--- cfe/trunk/include/clang/Lex/Preprocessor.h
+++ cfe/trunk/include/clang/Lex/Preprocessor.h
@@ -310,6 +310,9 @@
/// on the stem that is to be code completed.
IdentifierInfo *CodeCompletionII = nullptr;
+ /// Range for the code completion token.
+ SourceRange CodeCompletionTokenRange;
+
/// The directory that the main file should be considered to occupy,
/// if it does not correspond to a real file (as happens when building a
/// module).
@@ -1131,6 +1134,16 @@
CodeCompletionII = Filter;
}
+ /// Set the code completion token range for detecting replacement range later
+ /// on.
+ void setCodeCompletionTokenRange(const SourceLocation Start,
+ const SourceLocation End) {
+CodeCompletionTokenRange = {Start, End};
+ }
+ SourceRange getCodeCompletionTokenRange() const {
+return CodeCompletionTokenRange;
+ }
+
/// Get the code completion token for filtering purposes.
StringRef getCodeCompletionFilter() {
if (CodeCompletionII)
Index: cfe/trunk/lib/Lex/Preprocessor.cpp
===
--- cfe/trunk/lib/Lex/Preprocessor.cpp
+++ cfe/trunk/lib/Lex/Preprocessor.cpp
@@ -868,6 +868,7 @@
if (Result.is(tok::code_completion) && Result.getIdentifierInfo()) {
// Remember the identifier before code completion token.
setCodeCompletionIdentifierInfo(Result.getIdentifierInfo());
+setCodeCompletionTokenRange(Result.getLocation(), Result.getEndLoc());
// Set IdenfitierInfo to null to avoid confusing code that handles both
// identifiers and completion tokens.
Result.setIdentifierInfo(nullptr);
Index: cfe/trunk/include/clang/Lex/Preprocessor.h
===
--- cfe/trunk/include/clang/Lex/Preprocessor.h
+++ cfe/trunk/include/clang/Lex/Preprocessor.h
@@ -310,6 +310,9 @@
/// on the stem that is to be code completed.
IdentifierInfo *CodeCompletionII = nullptr;
+ /// Range for the code completion token.
+ SourceRange CodeCompletionTokenRange;
+
/// The directory that the main file should be considered to occupy,
/// if it does not correspond to a real file (as happens when building a
/// module).
@@ -1131,6 +1134,16 @@
CodeCompletionII = Filter;
}
+ /// Set the code completion token range for detecting replacement range later
+ /// on.
+ void setCodeCompletionTokenRange(const SourceLocation Start,
+ const SourceLocation End) {
+CodeCompletionTokenRange = {Start, End};
+ }
+ SourceRange getCodeCompletionTokenRange() const {
+return CodeCompletionTokenRange;
+ }
+
/// Get the code completion token for filtering purposes.
StringRef getCodeCompletionFilter() {
if (CodeCompletionII)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47111: : Implement monotonic_buffer_resource.
Quuxplusone added a comment. Long-delayed ping! Repository: rCXX libc++ https://reviews.llvm.org/D47111 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50517: [clangd] Generate incomplete trigrams for the Dex index
kbobyrev updated this revision to Diff 160302.
kbobyrev marked an inline comment as done.
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
Address the post-LGTM comment.
https://reviews.llvm.org/D50517
Files:
clang-tools-extra/clangd/index/dex/Iterator.h
clang-tools-extra/clangd/index/dex/Trigram.cpp
clang-tools-extra/clangd/index/dex/Trigram.h
clang-tools-extra/unittests/clangd/DexIndexTests.cpp
Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -250,45 +250,57 @@
}
TEST(DexIndexTrigrams, IdentifierTrigrams) {
- EXPECT_THAT(generateIdentifierTrigrams("X86"), trigramsAre({"x86"}));
+ EXPECT_THAT(generateIdentifierTrigrams("X86"),
+ trigramsAre({"x86", "x$$", "x8$", "$$$"}));
- EXPECT_THAT(generateIdentifierTrigrams("nl"), trigramsAre({}));
+ EXPECT_THAT(generateIdentifierTrigrams("nl"),
+ trigramsAre({"nl$", "n$$", "$$$"}));
+
+ EXPECT_THAT(generateIdentifierTrigrams("n"), trigramsAre({"n$$", "$$$"}));
EXPECT_THAT(generateIdentifierTrigrams("clangd"),
- trigramsAre({"cla", "lan", "ang", "ngd"}));
+ trigramsAre({"c$$", "cl$", "cla", "lan", "ang", "ngd", "$$$"}));
EXPECT_THAT(generateIdentifierTrigrams("abc_def"),
- trigramsAre({"abc", "abd", "ade", "bcd", "bde", "cde", "def"}));
+ trigramsAre({"a$$", "abc", "abd", "ade", "bcd", "bde", "cde",
+ "def", "ab$", "ad$", "$$$"}));
- EXPECT_THAT(
- generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"abc", "abd", "acd", "ace", "bcd", "bce", "bde", "cde"}));
+ EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
+ trigramsAre({"a$$", "a_$", "a_b", "abc", "abd", "acd", "ace",
+ "bcd", "bce", "bde", "cde", "ab$", "$$$"}));
- EXPECT_THAT(
- generateIdentifierTrigrams("unique_ptr"),
- trigramsAre({"uni", "unp", "upt", "niq", "nip", "npt", "iqu", "iqp",
- "ipt", "que", "qup", "qpt", "uep", "ept", "ptr"}));
+ EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
+ trigramsAre({"u$$", "uni", "unp", "upt", "niq", "nip", "npt",
+ "iqu", "iqp", "ipt", "que", "qup", "qpt", "uep",
+ "ept", "ptr", "un$", "up$", "$$$"}));
EXPECT_THAT(generateIdentifierTrigrams("TUDecl"),
- trigramsAre({"tud", "tde", "ude", "dec", "ecl"}));
+ trigramsAre({"t$$", "tud", "tde", "ude", "dec", "ecl", "tu$",
+ "td$", "$$$"}));
EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
- trigramsAre({"iso", "iok", "sok"}));
-
- EXPECT_THAT(generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({
- "abc", "abd", "abg", "ade", "adg", "adk", "agh", "agk", "bcd",
- "bcg", "bde", "bdg", "bdk", "bgh", "bgk", "cde", "cdg", "cdk",
- "cgh", "cgk", "def", "deg", "dek", "dgh", "dgk", "dkl", "efg",
- "efk", "egh", "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk",
- "gkl", "hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm",
- }));
+ trigramsAre({"i$$", "iso", "iok", "sok", "is$", "io$", "$$$"}));
+
+ EXPECT_THAT(
+ generateIdentifierTrigrams("abc_defGhij__klm"),
+ trigramsAre({"a$$", "abc", "abd", "abg", "ade", "adg", "adk", "agh",
+ "agk", "bcd", "bcg", "bde", "bdg", "bdk", "bgh", "bgk",
+ "cde", "cdg", "cdk", "cgh", "cgk", "def", "deg", "dek",
+ "dgh", "dgk", "dkl", "efg", "efk", "egh", "egk", "ekl",
+ "fgh", "fgk", "fkl", "ghi", "ghk", "gkl", "hij", "hik",
+ "hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$", "$$$"}));
}
TEST(DexIndexTrigrams, QueryTrigrams) {
- EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
+ EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c$$"}));
+ EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl$"}));
+ EXPECT_THAT(generateQueryTrigrams("cla"), trigramsAre({"cla"}));
- EXPECT_THAT(generateQueryTrigrams("nl"), trigramsAre({}));
+ EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_$$"}));
+ EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
+ EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
+
+ EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
EXPECT_THAT(generateQueryTrigrams("clangd"),
trigramsAre({"cla", "lan", "ang", "ngd"}));
Index: clang-tools-extra/clangd/index/dex/Trigram.h
===
--- clang-tools-extra/clangd/index/dex/Trigram.h
+++ clang-tools-extra/clangd/index/dex/Trigram.h
@@ -36,14 +36,20 @@
/// First, given Identifier (u
[PATCH] D50449: [clangd] Support textEdit in addition to insertText.
kadircet updated this revision to Diff 160303.
kadircet added a comment.
- Rebase.
- Resolve discussions.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50449
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/SourceCode.cpp
clangd/SourceCode.h
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SourceCodeTests.cpp
Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -37,6 +37,13 @@
return Pos;
}
+Range range(const std::pair p1, const std::pair p2) {
+ Range range;
+ range.start = position(p1.first, p1.second);
+ range.end = position(p2.first, p2.second);
+ return range;
+}
+
TEST(SourceCodeTests, PositionToOffset) {
// line out of bounds
EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
@@ -119,6 +126,14 @@
EXPECT_THAT(offsetToPosition(File, 30), Pos(2, 9)) << "out of bounds";
}
+TEST(SourceCodeTests, IsRangeConsecutive) {
+ EXPECT_TRUE(IsRangeConsecutive(range({2, 2}, {2, 3}), range({2, 3}, {2, 4})));
+ EXPECT_FALSE(
+ IsRangeConsecutive(range({0, 2}, {0, 3}), range({2, 3}, {2, 4})));
+ EXPECT_FALSE(
+ IsRangeConsecutive(range({2, 2}, {2, 3}), range({2, 4}, {2, 5})));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1439,6 +1439,82 @@
}
}
+TEST(CompletionTest, RenderWithFixItMerged) {
+ TextEdit FixIt;
+ FixIt.range.end.character = 5;
+ FixIt.newText = "->";
+
+ CodeCompletion C;
+ C.Name = "x";
+ C.RequiredQualifier = "Foo::";
+ C.FixIts = {FixIt};
+ C.CompletionTokenRange.start.character = 5;
+
+ CodeCompleteOptions Opts;
+ Opts.IncludeFixIts = true;
+
+ auto R = C.render(Opts);
+ EXPECT_TRUE(R.textEdit);
+ EXPECT_EQ(R.textEdit->newText, "->Foo::x");
+ EXPECT_TRUE(R.additionalTextEdits.empty());
+}
+
+TEST(CompletionTest, RenderWithFixItNonMerged) {
+ TextEdit FixIt;
+ FixIt.range.end.character = 4;
+ FixIt.newText = "->";
+
+ CodeCompletion C;
+ C.Name = "x";
+ C.RequiredQualifier = "Foo::";
+ C.FixIts = {FixIt};
+ C.CompletionTokenRange.start.character = 5;
+
+ CodeCompleteOptions Opts;
+ Opts.IncludeFixIts = true;
+
+ auto R = C.render(Opts);
+ EXPECT_TRUE(R.textEdit);
+ EXPECT_EQ(R.textEdit->newText, "Foo::x");
+ EXPECT_THAT(R.additionalTextEdits, UnorderedElementsAre(FixIt));
+}
+
+TEST(CompletionTest, CompletionTokenRange) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ IgnoreDiagnostics DiagConsumer;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ constexpr const char *TestCodes[] = {
+ R"cpp(
+class Auxilary {
+ public:
+ void AuxFunction();
+};
+void f() {
+ Auxilary x;
+ x.[[Aux]]^;
+}
+ )cpp",
+ R"cpp(
+class Auxilary {
+ public:
+ void AuxFunction();
+};
+void f() {
+ Auxilary x;
+ x.[[]]^;
+}
+ )cpp"};
+ for (const auto &Text : TestCodes) {
+Annotations TestCode(Text);
+auto Results = completions(Server, TestCode.code(), TestCode.point());
+
+EXPECT_EQ(Results.Completions.size(), 1u);
+EXPECT_THAT(Results.Completions.front().CompletionTokenRange, TestCode.range());
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/SourceCode.h
===
--- clangd/SourceCode.h
+++ clangd/SourceCode.h
@@ -76,6 +76,8 @@
/// are normalized as much as possible.
llvm::Optional getRealPath(const FileEntry *F,
const SourceManager &SourceMgr);
+
+bool IsRangeConsecutive(const Range &Left, const Range &Right);
} // namespace clangd
} // namespace clang
#endif
Index: clangd/SourceCode.cpp
===
--- clangd/SourceCode.cpp
+++ clangd/SourceCode.cpp
@@ -224,5 +224,10 @@
return Result;
}
+bool IsRangeConsecutive(const Range &Left, const Range &Right) {
+ return Left.end.line == Right.start.line &&
+ Left.end.character == Right.start.character;
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -123,6 +123,9 @@
/// converting '->' to '.' on member access.
std::vector FixIts;
+ /// Holds the range of the token we are going to replace with this completion.
+ Range CompletionTokenRange;
+
// Scores are used to rank completion items.
struct Scores {
// The score that items are ranked by.
Index: clangd/CodeComplete.cpp
=
[PATCH] D50449: [clangd] Support textEdit in addition to insertText.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339543: [clangd] Support textEdit in addition to insertText.
(authored by kadircet, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50449
Files:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
@@ -37,6 +37,13 @@
return Pos;
}
+Range range(const std::pair p1, const std::pair p2) {
+ Range range;
+ range.start = position(p1.first, p1.second);
+ range.end = position(p2.first, p2.second);
+ return range;
+}
+
TEST(SourceCodeTests, PositionToOffset) {
// line out of bounds
EXPECT_THAT_EXPECTED(positionToOffset(File, position(-1, 2)), Failed());
@@ -119,6 +126,14 @@
EXPECT_THAT(offsetToPosition(File, 30), Pos(2, 9)) << "out of bounds";
}
+TEST(SourceCodeTests, IsRangeConsecutive) {
+ EXPECT_TRUE(IsRangeConsecutive(range({2, 2}, {2, 3}), range({2, 3}, {2, 4})));
+ EXPECT_FALSE(
+ IsRangeConsecutive(range({0, 2}, {0, 3}), range({2, 3}, {2, 4})));
+ EXPECT_FALSE(
+ IsRangeConsecutive(range({2, 2}, {2, 3}), range({2, 4}, {2, 5})));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -1439,6 +1439,82 @@
}
}
+TEST(CompletionTest, RenderWithFixItMerged) {
+ TextEdit FixIt;
+ FixIt.range.end.character = 5;
+ FixIt.newText = "->";
+
+ CodeCompletion C;
+ C.Name = "x";
+ C.RequiredQualifier = "Foo::";
+ C.FixIts = {FixIt};
+ C.CompletionTokenRange.start.character = 5;
+
+ CodeCompleteOptions Opts;
+ Opts.IncludeFixIts = true;
+
+ auto R = C.render(Opts);
+ EXPECT_TRUE(R.textEdit);
+ EXPECT_EQ(R.textEdit->newText, "->Foo::x");
+ EXPECT_TRUE(R.additionalTextEdits.empty());
+}
+
+TEST(CompletionTest, RenderWithFixItNonMerged) {
+ TextEdit FixIt;
+ FixIt.range.end.character = 4;
+ FixIt.newText = "->";
+
+ CodeCompletion C;
+ C.Name = "x";
+ C.RequiredQualifier = "Foo::";
+ C.FixIts = {FixIt};
+ C.CompletionTokenRange.start.character = 5;
+
+ CodeCompleteOptions Opts;
+ Opts.IncludeFixIts = true;
+
+ auto R = C.render(Opts);
+ EXPECT_TRUE(R.textEdit);
+ EXPECT_EQ(R.textEdit->newText, "Foo::x");
+ EXPECT_THAT(R.additionalTextEdits, UnorderedElementsAre(FixIt));
+}
+
+TEST(CompletionTest, CompletionTokenRange) {
+ MockFSProvider FS;
+ MockCompilationDatabase CDB;
+ IgnoreDiagnostics DiagConsumer;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ constexpr const char *TestCodes[] = {
+ R"cpp(
+class Auxilary {
+ public:
+ void AuxFunction();
+};
+void f() {
+ Auxilary x;
+ x.[[Aux]]^;
+}
+ )cpp",
+ R"cpp(
+class Auxilary {
+ public:
+ void AuxFunction();
+};
+void f() {
+ Auxilary x;
+ x.[[]]^;
+}
+ )cpp"};
+ for (const auto &Text : TestCodes) {
+Annotations TestCode(Text);
+auto Results = completions(Server, TestCode.code(), TestCode.point());
+
+EXPECT_EQ(Results.Completions.size(), 1u);
+EXPECT_THAT(Results.Completions.front().CompletionTokenRange, TestCode.range());
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/clangd/CodeComplete.h
===
--- clang-tools-extra/trunk/clangd/CodeComplete.h
+++ clang-tools-extra/trunk/clangd/CodeComplete.h
@@ -123,6 +123,9 @@
/// converting '->' to '.' on member access.
std::vector FixIts;
+ /// Holds the range of the token we are going to replace with this completion.
+ Range CompletionTokenRange;
+
// Scores are used to rank completion items.
struct Scores {
// The score that items are ranked by.
Index: clang-tools-extra/trunk/clangd/SourceCode.cpp
===
--- clang-tools-extra/trunk/clangd/SourceCode.cpp
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp
@@ -224,5 +224,10 @@
return Result;
}
+bool IsRangeConsecutive(const Range &Left, const Range &Right) {
+ return Left.end.line == Right.start.line &&
+ Left.end.character == Right.start.character;
+}
[clang-tools-extra] r339543 - [clangd] Support textEdit in addition to insertText.
Author: kadircet
Date: Mon Aug 13 01:23:01 2018
New Revision: 339543
URL: http://llvm.org/viewvc/llvm-project?rev=339543&view=rev
Log:
[clangd] Support textEdit in addition to insertText.
Summary:
Completion replies contains textEdits as well. Note that this change
relies on https://reviews.llvm.org/D50443.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: mgrang, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50449
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeComplete.h
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/SourceCodeTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=339543&r1=339542&r2=339543&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon Aug 13 01:23:01 2018
@@ -34,6 +34,7 @@
#include "index/Index.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Format/Format.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendActions.h"
@@ -285,6 +286,11 @@ struct CodeCompletionBuilder {
Completion.FixIts.push_back(
toTextEdit(FixIt, ASTCtx.getSourceManager(),
ASTCtx.getLangOpts()));
}
+ std::sort(Completion.FixIts.begin(), Completion.FixIts.end(),
+[](const TextEdit &X, const TextEdit &Y) {
+ return std::tie(X.range.start.line, X.range.start.character)
<
+ std::tie(Y.range.start.line, Y.range.start.character);
+});
}
if (C.IndexResult) {
Completion.Origin |= C.IndexResult->Origin;
@@ -1044,6 +1050,23 @@ private:
// This is called by run() once Sema code completion is done, but before the
// Sema data structures are torn down. It does all the real work.
CodeCompleteResult runWithSema() {
+const auto &CodeCompletionRange = CharSourceRange::getCharRange(
+Recorder->CCSema->getPreprocessor().getCodeCompletionTokenRange());
+Range TextEditRange;
+// When we are getting completions with an empty identifier, for example
+//std::vector asdf;
+//asdf.^;
+// Then the range will be invalid and we will be doing insertion, use
+// current cursor position in such cases as range.
+if (CodeCompletionRange.isValid()) {
+ TextEditRange = halfOpenToRange(Recorder->CCSema->getSourceManager(),
+ CodeCompletionRange);
+} else {
+ const auto &Pos = sourceLocToPosition(
+ Recorder->CCSema->getSourceManager(),
+ Recorder->CCSema->getPreprocessor().getCodeCompletionLoc());
+ TextEditRange.start = TextEditRange.end = Pos;
+}
Filter = FuzzyMatcher(
Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
QueryScopes = getQueryScopes(Recorder->CCContext,
@@ -1063,6 +1086,7 @@ private:
for (auto &C : Top) {
Output.Completions.push_back(toCodeCompletion(C.first));
Output.Completions.back().Score = C.second;
+ Output.Completions.back().CompletionTokenRange = TextEditRange;
}
Output.HasMore = Incomplete;
Output.Context = Recorder->CCContext.getKind();
@@ -1278,16 +1302,29 @@ CompletionItem CodeCompletion::render(co
LSP.documentation = Documentation;
LSP.sortText = sortText(Score.Total, Name);
LSP.filterText = Name;
- // FIXME(kadircet): Use LSP.textEdit instead of insertText, because it causes
- // undesired behaviours. Like completing "this.^" into "this-push_back".
- LSP.insertText = RequiredQualifier + Name;
+ LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name};
+ // Merge continious additionalTextEdits into main edit. The main motivation
+ // behind this is to help LSP clients, it seems most of them are confused
when
+ // they are provided with additionalTextEdits that are consecutive to main
+ // edit.
+ // Note that we store additional text edits from back to front in a line.
That
+ // is mainly to help LSP clients again, so that changes do not effect each
+ // other.
+ for (const auto &FixIt : FixIts) {
+if (IsRangeConsecutive(FixIt.range, LSP.textEdit->range)) {
+ LSP.textEdit->newText = FixIt.newText + LSP.textEdit->newText;
+ LSP.textEdit->range.start = FixIt.range.start;
+} else {
+ LSP.additionalTextEdits.push_back(FixIt);
+}
+ }
if (Opts.EnableSnippets)
-LSP.insertText += SnippetSuffix;
+LSP.textEdit->newText += SnippetSuffix;
+ // FIXME(kadircet): Do not
[PATCH] D50555: [clangd] Introduce scoring mechanism for SignatureInformations.
kadircet updated this revision to Diff 160306.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Rebase & Resolve discussions.
- Resolve discussions.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50555
Files:
clangd/CodeComplete.cpp
clangd/Quality.cpp
clangd/Quality.h
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1515,6 +1515,28 @@
}
}
+TEST(SignatureHelpTest, OverloadsOrdering) {
+ const auto Results = signatures(R"cpp(
+void foo(int x);
+void foo(int x, float y);
+void foo(float x, int y);
+void foo(float x, float y);
+void foo(int x, int y = 0);
+int main() { foo(^); }
+ )cpp");
+ EXPECT_THAT(
+ Results.signatures,
+ ElementsAre(
+ Sig("foo(int x) -> void", {"int x"}),
+ Sig("foo(int x, int y = 0) -> void", {"int x", "int y = 0"}),
+ Sig("foo(float x, int y) -> void", {"float x", "int y"}),
+ Sig("foo(int x, float y) -> void", {"int x", "float y"}),
+ Sig("foo(float x, float y) -> void", {"float x", "float y"})));
+ // We always prefer the first signature.
+ EXPECT_EQ(0, Results.activeSignature);
+ EXPECT_EQ(0, Results.activeParameter);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -163,6 +163,16 @@
/// LSP. (The highest score compares smallest so it sorts at the top).
std::string sortText(float Score, llvm::StringRef Tiebreak = "");
+struct SignatureQualitySignals {
+ uint32_t NumberOfParameters = 0;
+ uint32_t NumberOfOptionalParameters = 0;
+ bool ContainsActiveParameter = false;
+ CodeCompleteConsumer::OverloadCandidate::CandidateKind Kind =
+ CodeCompleteConsumer::OverloadCandidate::CandidateKind::CK_Function;
+};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &,
+ const SignatureQualitySignals &);
+
} // namespace clangd
} // namespace clang
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -401,5 +401,17 @@
return S;
}
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const SignatureQualitySignals &S) {
+ OS << formatv("=== Signature Quality:\n");
+ OS << formatv("\tNumber of parameters: {0}\n", S.NumberOfParameters);
+ OS << formatv("\tNumber of optional parameters: {0}\n",
+S.NumberOfOptionalParameters);
+ OS << formatv("\tContains active parameter: {0}\n",
+S.ContainsActiveParameter);
+ OS << formatv("\tKind: {0}\n", S.Kind);
+ return OS;
+}
+
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -129,16 +129,16 @@
/// Get the optional chunk as a string. This function is possibly recursive.
///
/// The parameter info for each parameter is appended to the Parameters.
-std::string
-getOptionalParameters(const CodeCompletionString &CCS,
- std::vector &Parameters) {
+std::string getOptionalParameters(const CodeCompletionString &CCS,
+ std::vector &Parameters,
+ SignatureQualitySignals &Signal) {
std::string Result;
for (const auto &Chunk : CCS) {
switch (Chunk.Kind) {
case CodeCompletionString::CK_Optional:
assert(Chunk.Optional &&
"Expected the optional code completion string to be non-null.");
- Result += getOptionalParameters(*Chunk.Optional, Parameters);
+ Result += getOptionalParameters(*Chunk.Optional, Parameters, Signal);
break;
case CodeCompletionString::CK_VerticalSpace:
break;
@@ -154,6 +154,8 @@
ParameterInformation Info;
Info.label = Chunk.Text;
Parameters.push_back(std::move(Info));
+ Signal.ContainsActiveParameter = true;
+ Signal.NumberOfOptionalParameters++;
break;
}
default:
@@ -685,6 +687,9 @@
llvm::unique_function ResultsCallback;
};
+using ScoredSignature =
+std::pair;
+
class SignatureHelpCollector final : public CodeCompleteConsumer {
public:
@@ -698,7 +703,9 @@
void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
unsigned NumCandidates) override {
+std::vector ScoredSignatures;
SigHelp.signatures.reserve(NumCandidates);
+ScoredSignatures.reserve(NumCandidates);
// FIXME(rwols): How can we determine the "active overload candidate"?
// Right now the overloaded candidates seem to be p
[PATCH] D50555: [clangd] Introduce scoring mechanism for SignatureInformations.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE339547: [clangd] Introduce scoring mechanism for
SignatureInformations. (authored by kadircet, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50555?vs=160306&id=160307#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50555
Files:
clangd/CodeComplete.cpp
clangd/Quality.cpp
clangd/Quality.h
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1515,6 +1515,28 @@
}
}
+TEST(SignatureHelpTest, OverloadsOrdering) {
+ const auto Results = signatures(R"cpp(
+void foo(int x);
+void foo(int x, float y);
+void foo(float x, int y);
+void foo(float x, float y);
+void foo(int x, int y = 0);
+int main() { foo(^); }
+ )cpp");
+ EXPECT_THAT(
+ Results.signatures,
+ ElementsAre(
+ Sig("foo(int x) -> void", {"int x"}),
+ Sig("foo(int x, int y = 0) -> void", {"int x", "int y = 0"}),
+ Sig("foo(float x, int y) -> void", {"float x", "int y"}),
+ Sig("foo(int x, float y) -> void", {"int x", "float y"}),
+ Sig("foo(float x, float y) -> void", {"float x", "float y"})));
+ // We always prefer the first signature.
+ EXPECT_EQ(0, Results.activeSignature);
+ EXPECT_EQ(0, Results.activeParameter);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -129,16 +129,16 @@
/// Get the optional chunk as a string. This function is possibly recursive.
///
/// The parameter info for each parameter is appended to the Parameters.
-std::string
-getOptionalParameters(const CodeCompletionString &CCS,
- std::vector &Parameters) {
+std::string getOptionalParameters(const CodeCompletionString &CCS,
+ std::vector &Parameters,
+ SignatureQualitySignals &Signal) {
std::string Result;
for (const auto &Chunk : CCS) {
switch (Chunk.Kind) {
case CodeCompletionString::CK_Optional:
assert(Chunk.Optional &&
"Expected the optional code completion string to be non-null.");
- Result += getOptionalParameters(*Chunk.Optional, Parameters);
+ Result += getOptionalParameters(*Chunk.Optional, Parameters, Signal);
break;
case CodeCompletionString::CK_VerticalSpace:
break;
@@ -154,6 +154,8 @@
ParameterInformation Info;
Info.label = Chunk.Text;
Parameters.push_back(std::move(Info));
+ Signal.ContainsActiveParameter = true;
+ Signal.NumberOfOptionalParameters++;
break;
}
default:
@@ -685,6 +687,9 @@
llvm::unique_function ResultsCallback;
};
+using ScoredSignature =
+std::pair;
+
class SignatureHelpCollector final : public CodeCompleteConsumer {
public:
@@ -698,7 +703,9 @@
void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
unsigned NumCandidates) override {
+std::vector ScoredSignatures;
SigHelp.signatures.reserve(NumCandidates);
+ScoredSignatures.reserve(NumCandidates);
// FIXME(rwols): How can we determine the "active overload candidate"?
// Right now the overloaded candidates seem to be provided in a "best fit"
// order, so I'm not too worried about this.
@@ -712,11 +719,45 @@
CurrentArg, S, *Allocator, CCTUInfo, true);
assert(CCS && "Expected the CodeCompletionString to be non-null");
// FIXME: for headers, we need to get a comment from the index.
- SigHelp.signatures.push_back(processOverloadCandidate(
+ ScoredSignatures.push_back(processOverloadCandidate(
Candidate, *CCS,
getParameterDocComment(S.getASTContext(), Candidate, CurrentArg,
/*CommentsFromHeaders=*/false)));
}
+std::sort(ScoredSignatures.begin(), ScoredSignatures.end(),
+ [](const ScoredSignature &L, const ScoredSignature &R) {
+// Ordering follows:
+// - Less number of parameters is better.
+// - Function is better than FunctionType which is better than
+// Function Template.
+// - High score is better.
+// - Shorter signature is better.
+// - Alphebatically smaller is better.
+if (L.first.NumberOfParameters != R.first.NumberOfParameters)
+ return L.first.NumberOfParameters <
+ R.first.NumberOfParameters;
+if (L.first.NumberOfOptionalParameters !=
+R.first.
[clang-tools-extra] r339547 - [clangd] Introduce scoring mechanism for SignatureInformations.
Author: kadircet
Date: Mon Aug 13 01:40:05 2018
New Revision: 339547
URL: http://llvm.org/viewvc/llvm-project?rev=339547&view=rev
Log:
[clangd] Introduce scoring mechanism for SignatureInformations.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: mgrang, ioeric, MaskRay, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50555
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=339547&r1=339546&r2=339547&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Mon Aug 13 01:40:05 2018
@@ -129,16 +129,16 @@ toCompletionItemKind(CodeCompletionResul
/// Get the optional chunk as a string. This function is possibly recursive.
///
/// The parameter info for each parameter is appended to the Parameters.
-std::string
-getOptionalParameters(const CodeCompletionString &CCS,
- std::vector &Parameters) {
+std::string getOptionalParameters(const CodeCompletionString &CCS,
+ std::vector
&Parameters,
+ SignatureQualitySignals &Signal) {
std::string Result;
for (const auto &Chunk : CCS) {
switch (Chunk.Kind) {
case CodeCompletionString::CK_Optional:
assert(Chunk.Optional &&
"Expected the optional code completion string to be non-null.");
- Result += getOptionalParameters(*Chunk.Optional, Parameters);
+ Result += getOptionalParameters(*Chunk.Optional, Parameters, Signal);
break;
case CodeCompletionString::CK_VerticalSpace:
break;
@@ -154,6 +154,8 @@ getOptionalParameters(const CodeCompleti
ParameterInformation Info;
Info.label = Chunk.Text;
Parameters.push_back(std::move(Info));
+ Signal.ContainsActiveParameter = true;
+ Signal.NumberOfOptionalParameters++;
break;
}
default:
@@ -685,6 +687,9 @@ private:
llvm::unique_function ResultsCallback;
};
+using ScoredSignature =
+std::pair;
+
class SignatureHelpCollector final : public CodeCompleteConsumer {
public:
@@ -698,7 +703,9 @@ public:
void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
unsigned NumCandidates) override {
+std::vector ScoredSignatures;
SigHelp.signatures.reserve(NumCandidates);
+ScoredSignatures.reserve(NumCandidates);
// FIXME(rwols): How can we determine the "active overload candidate"?
// Right now the overloaded candidates seem to be provided in a "best fit"
// order, so I'm not too worried about this.
@@ -712,11 +719,45 @@ public:
CurrentArg, S, *Allocator, CCTUInfo, true);
assert(CCS && "Expected the CodeCompletionString to be non-null");
// FIXME: for headers, we need to get a comment from the index.
- SigHelp.signatures.push_back(processOverloadCandidate(
+ ScoredSignatures.push_back(processOverloadCandidate(
Candidate, *CCS,
getParameterDocComment(S.getASTContext(), Candidate, CurrentArg,
/*CommentsFromHeaders=*/false)));
}
+std::sort(ScoredSignatures.begin(), ScoredSignatures.end(),
+ [](const ScoredSignature &L, const ScoredSignature &R) {
+// Ordering follows:
+// - Less number of parameters is better.
+// - Function is better than FunctionType which is better than
+// Function Template.
+// - High score is better.
+// - Shorter signature is better.
+// - Alphebatically smaller is better.
+if (L.first.NumberOfParameters != R.first.NumberOfParameters)
+ return L.first.NumberOfParameters <
+ R.first.NumberOfParameters;
+if (L.first.NumberOfOptionalParameters !=
+R.first.NumberOfOptionalParameters)
+ return L.first.NumberOfOptionalParameters <
+ R.first.NumberOfOptionalParameters;
+if (L.first.Kind != R.first.Kind) {
+ using OC = CodeCompleteConsumer::OverloadCandidate;
+ switch (L.first.Kind) {
+ case OC::CK_Function:
+return true;
+ case OC::CK_FunctionType:
+return R.first.Kind != OC::CK_Function;
+ case OC::CK_FunctionTemplate:
+return false;
+ }
+
[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category
ilya-biryukov added inline comments. Comment at: clangd/Diagnostics.h:40 DiagnosticsEngine::Level Severity = DiagnosticsEngine::Note; + unsigned Category; // Since File is only descriptive, we store a separate flag to distinguish Maybe store the string name of the category directly here? This struct was never optimized for efficiency, but aims to be independent of clang, so that the clients might interpret it without calling any clang-specific code (we do reuse the `Level` enum, though, but that's merely for code reuse) Comment at: clangd/Protocol.h:549 + /// The diagnostic's category. Can be omitted. + /// An LSP extension that's used to send the name of the category over to the + /// client. Maybe add a brief example of category names here, i.e. "semantic issue", etc.? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50571 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48072: Sema: Fix PR12350 destructor name lookup, addressing (some of) DR244
jmorse added a comment. ping Repository: rC Clang https://reviews.llvm.org/D48072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r339548 - [clangd] Generate incomplete trigrams for the Dex index
Author: omtcyfz
Date: Mon Aug 13 01:57:06 2018
New Revision: 339548
URL: http://llvm.org/viewvc/llvm-project?rev=339548&view=rev
Log:
[clangd] Generate incomplete trigrams for the Dex index
This patch handles trigram generation "short" identifiers and queries.
Trigram generator produces incomplete trigrams for short names so that
the same query iterator API can be used to match symbols which don't
have enough symbols to form a trigram and correctly handle queries which
also are not sufficient for generating a full trigram.
Reviewed by: ioeric
Differential revision: https://reviews.llvm.org/D50517
Modified:
clang-tools-extra/trunk/clangd/index/dex/Iterator.h
clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
clang-tools-extra/trunk/clangd/index/dex/Trigram.h
clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.h?rev=339548&r1=339547&r2=339548&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h Mon Aug 13 01:57:06 2018
@@ -47,6 +47,14 @@ using DocID = uint32_t;
/// Contains sorted sequence of DocIDs all of which belong to symbols matching
/// certain criteria, i.e. containing a Search Token. PostingLists are values
/// for the inverted index.
+// FIXME(kbobyrev): Posting lists for incomplete trigrams (one/two symbols) are
+// likely to be very dense and hence require special attention so that the
index
+// doesn't use too much memory. Possible solution would be to construct
+// compressed posting lists which consist of ranges of DocIDs instead of
+// distinct DocIDs. A special case would be the empty query: for that case
+// TrueIterator should be implemented - an iterator which doesn't actually
store
+// any PostingList within itself, but "contains" all DocIDs in range
+// [0, IndexSize).
using PostingList = std::vector;
/// Immutable reference to PostingList object.
using PostingListRef = llvm::ArrayRef;
Modified: clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp?rev=339548&r1=339547&r2=339548&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp Mon Aug 13 01:57:06
2018
@@ -10,11 +10,9 @@
#include "Trigram.h"
#include "../../FuzzyMatch.h"
#include "Token.h"
-
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/StringExtras.h"
-
#include
#include
#include
@@ -25,12 +23,11 @@ namespace clang {
namespace clangd {
namespace dex {
-// FIXME(kbobyrev): Deal with short symbol symbol names. A viable approach
would
-// be generating unigrams and bigrams here, too. This would prevent symbol
index
-// from applying fuzzy matching on a tremendous number of symbols and allow
-// supplementary retrieval for short queries.
-//
-// Short names (total segment length <3 characters) are currently ignored.
+/// This is used to mark unigrams and bigrams and distinct them from complete
+/// trigrams. Since '$' is not present in valid identifier names, it is safe to
+/// use it as the special symbol.
+static const char END_MARKER = '$';
+
std::vector generateIdentifierTrigrams(llvm::StringRef Identifier) {
// Apply fuzzy matching text segmentation.
std::vector Roles(Identifier.size());
@@ -50,17 +47,45 @@ std::vector generateIdentifierTri
// not available then 0 is stored.
std::vector> Next(LowercaseIdentifier.size());
unsigned NextTail = 0, NextHead = 0, NextNextHead = 0;
+ // Store two first HEAD characters in the identifier (if present).
+ std::deque TwoHeads;
for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
Next[I] = {{NextTail, NextHead, NextNextHead}};
NextTail = Roles[I] == Tail ? I : 0;
if (Roles[I] == Head) {
NextNextHead = NextHead;
NextHead = I;
+ TwoHeads.push_front(LowercaseIdentifier[I]);
+ if (TwoHeads.size() > 2)
+TwoHeads.pop_back();
}
}
DenseSet UniqueTrigrams;
- std::array Chars;
+
+ auto add = [&](std::string Chars) {
+UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
+ };
+
+ // FIXME(kbobyrev): Instead of producing empty trigram for each identifier,
+ // just use True Iterator on the query side when the query string is empty.
+ add({{END_MARKER, END_MARKER, END_MARKER}});
+
+ if (TwoHeads.size() == 2)
+add({{TwoHeads.front(), TwoHeads.back(), END_MARKER}});
+
+ if (!LowercaseIdentifier.empty())
+add({{LowercaseIdentifier.front(), END_MARKER, END_MARKER}});
+
+ if (LowercaseIdentifier.size() >= 2)
+add({{LowercaseIdentifier[0], Lowercase
[PATCH] D50517: [clangd] Generate incomplete trigrams for the Dex index
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339548: [clangd] Generate incomplete trigrams for the Dex
index (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D50517?vs=160302&id=160310#toc
Repository:
rL LLVM
https://reviews.llvm.org/D50517
Files:
clang-tools-extra/trunk/clangd/index/dex/Iterator.h
clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
clang-tools-extra/trunk/clangd/index/dex/Trigram.h
clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
@@ -271,45 +271,57 @@
}
TEST(DexIndexTrigrams, IdentifierTrigrams) {
- EXPECT_THAT(generateIdentifierTrigrams("X86"), trigramsAre({"x86"}));
+ EXPECT_THAT(generateIdentifierTrigrams("X86"),
+ trigramsAre({"x86", "x$$", "x8$", "$$$"}));
- EXPECT_THAT(generateIdentifierTrigrams("nl"), trigramsAre({}));
+ EXPECT_THAT(generateIdentifierTrigrams("nl"),
+ trigramsAre({"nl$", "n$$", "$$$"}));
+
+ EXPECT_THAT(generateIdentifierTrigrams("n"), trigramsAre({"n$$", "$$$"}));
EXPECT_THAT(generateIdentifierTrigrams("clangd"),
- trigramsAre({"cla", "lan", "ang", "ngd"}));
+ trigramsAre({"c$$", "cl$", "cla", "lan", "ang", "ngd", "$$$"}));
EXPECT_THAT(generateIdentifierTrigrams("abc_def"),
- trigramsAre({"abc", "abd", "ade", "bcd", "bde", "cde", "def"}));
-
- EXPECT_THAT(
- generateIdentifierTrigrams("a_b_c_d_e_"),
- trigramsAre({"abc", "abd", "acd", "ace", "bcd", "bce", "bde", "cde"}));
+ trigramsAre({"a$$", "abc", "abd", "ade", "bcd", "bde", "cde",
+ "def", "ab$", "ad$", "$$$"}));
- EXPECT_THAT(
- generateIdentifierTrigrams("unique_ptr"),
- trigramsAre({"uni", "unp", "upt", "niq", "nip", "npt", "iqu", "iqp",
- "ipt", "que", "qup", "qpt", "uep", "ept", "ptr"}));
+ EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
+ trigramsAre({"a$$", "a_$", "a_b", "abc", "abd", "acd", "ace",
+ "bcd", "bce", "bde", "cde", "ab$", "$$$"}));
+
+ EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
+ trigramsAre({"u$$", "uni", "unp", "upt", "niq", "nip", "npt",
+ "iqu", "iqp", "ipt", "que", "qup", "qpt", "uep",
+ "ept", "ptr", "un$", "up$", "$$$"}));
EXPECT_THAT(generateIdentifierTrigrams("TUDecl"),
- trigramsAre({"tud", "tde", "ude", "dec", "ecl"}));
+ trigramsAre({"t$$", "tud", "tde", "ude", "dec", "ecl", "tu$",
+ "td$", "$$$"}));
EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
- trigramsAre({"iso", "iok", "sok"}));
+ trigramsAre({"i$$", "iso", "iok", "sok", "is$", "io$", "$$$"}));
- EXPECT_THAT(generateIdentifierTrigrams("abc_defGhij__klm"),
- trigramsAre({
- "abc", "abd", "abg", "ade", "adg", "adk", "agh", "agk", "bcd",
- "bcg", "bde", "bdg", "bdk", "bgh", "bgk", "cde", "cdg", "cdk",
- "cgh", "cgk", "def", "deg", "dek", "dgh", "dgk", "dkl", "efg",
- "efk", "egh", "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk",
- "gkl", "hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm",
- }));
+ EXPECT_THAT(
+ generateIdentifierTrigrams("abc_defGhij__klm"),
+ trigramsAre({"a$$", "abc", "abd", "abg", "ade", "adg", "adk", "agh",
+ "agk", "bcd", "bcg", "bde", "bdg", "bdk", "bgh", "bgk",
+ "cde", "cdg", "cdk", "cgh", "cgk", "def", "deg", "dek",
+ "dgh", "dgk", "dkl", "efg", "efk", "egh", "egk", "ekl",
+ "fgh", "fgk", "fkl", "ghi", "ghk", "gkl", "hij", "hik",
+ "hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$", "$$$"}));
}
TEST(DexIndexTrigrams, QueryTrigrams) {
- EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
+ EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c$$"}));
+ EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl$"}));
+ EXPECT_THAT(generateQueryTrigrams("cla"), trigramsAre({"cla"}));
+
+ EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_$$"}));
+ EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
+ EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
- EXPECT_THAT(generateQueryTrigrams("nl"), trigramsAre({}));
+ EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
EXPECT_THAT(generateQueryTrigrams("clangd"),
trigramsAre({"cla", "lan", "ang", "ngd"}));
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
===
[PATCH] D50559: [gnu-objc] Make selector order deterministic.
theraven updated this revision to Diff 160312.
theraven added a comment.
- Add a test case.
Repository:
rC Clang
https://reviews.llvm.org/D50559
Files:
lib/CodeGen/CGObjCGNU.cpp
test/CodeGenObjC/gnu-deterministic-selectors.m
Index: test/CodeGenObjC/gnu-deterministic-selectors.m
===
--- /dev/null
+++ test/CodeGenObjC/gnu-deterministic-selectors.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5
%s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s
-emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*,
i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64
0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2
x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8*
getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0),
i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]*
@.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr
inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, {
i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+ SEL a = @selector(z);
+ SEL b = @selector(a);
+ SEL c = @selector(g);
+ SEL d = @selector(l);
+ SEL e = @selector(j);
+}
Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
ConstantInitBuilder builder(CGM);
auto selectors = builder.beginArray(selStructTy);
auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+ allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
- std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+ std::string selNameStr = untypedSel.getAsString();
llvm::Constant *selName = ExportUniqueString(selNameStr,
".objc_sel_name");
- for (TypedSelector &sel : entry.second) {
+ for (TypedSelector &sel : table[untypedSel]) {
llvm::Constant *selectorTypeEncoding = NULLPtr;
if (!sel.first.empty())
selectorTypeEncoding =
Index: test/CodeGenObjC/gnu-deterministic-selectors.m
===
--- /dev/null
+++ test/CodeGenObjC/gnu-deterministic-selectors.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s -emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, { i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+ SEL a = @selector(z);
+ SEL b = @selector(a);
+ SEL c = @selector(g);
+ SEL d = @selector(l);
+ SEL e = @selector(j);
+}
Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
ConstantInitBuilder builder(CGM);
auto selectors = builder.beginArray(selStructTy);
auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+ allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
- std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+ std::string selNameStr = untypedSel.getAsString();
llvm::Constant *selName = ExportUniqueString(selNameStr, ".objc_sel_name");
- for (TypedSelector &sel : entry.second) {
+ for (TypedSelec
[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY
hans added a comment. The reason we noticed this was that it caused a *50 GB* size increase of the build output on our buildbots, which was enough to cause infrastructure problems. This change was also committed shortly before the 7.0 branch, so it's part of the 7.0.0 release candidates. Should we back it out until a solution is found? Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50507: [CodeGen][ARM] Coerce FP16 vectors to integer vectors when needed
miyuki updated this revision to Diff 160317.
miyuki added a comment.
Handle return of homogeneous aggregates
https://reviews.llvm.org/D50507
Files:
lib/CodeGen/TargetInfo.cpp
test/CodeGen/arm-vfp16-arguments.c
test/CodeGen/arm_neon_intrinsics.c
Index: test/CodeGen/arm_neon_intrinsics.c
===
--- test/CodeGen/arm_neon_intrinsics.c
+++ test/CodeGen/arm_neon_intrinsics.c
@@ -1,5 +1,6 @@
// RUN: %clang_cc1 -triple thumbv7s-apple-darwin -target-abi apcs-gnu\
-// RUN: -target-cpu swift -fallow-half-arguments-and-returns -ffreestanding \
+// RUN: -target-cpu swift -fallow-half-arguments-and-returns \
+// RUN: -target-feature +fullfp16 -ffreestanding \
// RUN: -disable-O0-optnone -emit-llvm -o - %s \
// RUN: | opt -S -mem2reg | FileCheck %s
@@ -3896,9 +3897,8 @@
// CHECK-LABEL: @test_vld1q_f16(
// CHECK: [[TMP0:%.*]] = bitcast half* %a to i8*
-// CHECK: [[VLD1:%.*]] = call <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8* [[TMP0]], i32 2)
-// CHECK: [[TMP1:%.*]] = bitcast <8 x i16> [[VLD1]] to <8 x half>
-// CHECK: ret <8 x half> [[TMP1]]
+// CHECK: [[VLD1:%.*]] = call <8 x half> @llvm.arm.neon.vld1.v8f16.p0i8(i8* [[TMP0]], i32 2)
+// CHECK: ret <8 x half> [[VLD1]]
float16x8_t test_vld1q_f16(float16_t const * a) {
return vld1q_f16(a);
}
@@ -3990,9 +3990,8 @@
// CHECK-LABEL: @test_vld1_f16(
// CHECK: [[TMP0:%.*]] = bitcast half* %a to i8*
-// CHECK: [[VLD1:%.*]] = call <4 x i16> @llvm.arm.neon.vld1.v4i16.p0i8(i8* [[TMP0]], i32 2)
-// CHECK: [[TMP1:%.*]] = bitcast <4 x i16> [[VLD1]] to <4 x half>
-// CHECK: ret <4 x half> [[TMP1]]
+// CHECK: [[VLD1:%.*]] = call <4 x half> @llvm.arm.neon.vld1.v4f16.p0i8(i8* [[TMP0]], i32 2)
+// CHECK: ret <4 x half> [[VLD1]]
float16x4_t test_vld1_f16(float16_t const * a) {
return vld1_f16(a);
}
@@ -4106,12 +4105,11 @@
// CHECK-LABEL: @test_vld1q_dup_f16(
// CHECK: [[TMP0:%.*]] = bitcast half* %a to i8*
-// CHECK: [[TMP1:%.*]] = bitcast i8* [[TMP0]] to i16*
-// CHECK: [[TMP2:%.*]] = load i16, i16* [[TMP1]], align 2
-// CHECK: [[TMP3:%.*]] = insertelement <8 x i16> undef, i16 [[TMP2]], i32 0
-// CHECK: [[LANE:%.*]] = shufflevector <8 x i16> [[TMP3]], <8 x i16> [[TMP3]], <8 x i32> zeroinitializer
-// CHECK: [[TMP4:%.*]] = bitcast <8 x i16> [[LANE]] to <8 x half>
-// CHECK: ret <8 x half> [[TMP4]]
+// CHECK: [[TMP1:%.*]] = bitcast i8* [[TMP0]] to half*
+// CHECK: [[TMP2:%.*]] = load half, half* [[TMP1]], align 2
+// CHECK: [[TMP3:%.*]] = insertelement <8 x half> undef, half [[TMP2]], i32 0
+// CHECK: [[LANE:%.*]] = shufflevector <8 x half> [[TMP3]], <8 x half> [[TMP3]], <8 x i32> zeroinitializer
+// CHECK: ret <8 x half> [[LANE]]
float16x8_t test_vld1q_dup_f16(float16_t const * a) {
return vld1q_dup_f16(a);
}
@@ -4233,12 +4231,11 @@
// CHECK-LABEL: @test_vld1_dup_f16(
// CHECK: [[TMP0:%.*]] = bitcast half* %a to i8*
-// CHECK: [[TMP1:%.*]] = bitcast i8* [[TMP0]] to i16*
-// CHECK: [[TMP2:%.*]] = load i16, i16* [[TMP1]], align 2
-// CHECK: [[TMP3:%.*]] = insertelement <4 x i16> undef, i16 [[TMP2]], i32 0
-// CHECK: [[LANE:%.*]] = shufflevector <4 x i16> [[TMP3]], <4 x i16> [[TMP3]], <4 x i32> zeroinitializer
-// CHECK: [[TMP4:%.*]] = bitcast <4 x i16> [[LANE]] to <4 x half>
-// CHECK: ret <4 x half> [[TMP4]]
+// CHECK: [[TMP1:%.*]] = bitcast i8* [[TMP0]] to half*
+// CHECK: [[TMP2:%.*]] = load half, half* [[TMP1]], align 2
+// CHECK: [[TMP3:%.*]] = insertelement <4 x half> undef, half [[TMP2]], i32 0
+// CHECK: [[LANE:%.*]] = shufflevector <4 x half> [[TMP3]], <4 x half> [[TMP3]], <4 x i32> zeroinitializer
+// CHECK: ret <4 x half> [[LANE]]
float16x4_t test_vld1_dup_f16(float16_t const * a) {
return vld1_dup_f16(a);
}
@@ -4365,12 +4362,11 @@
// CHECK-LABEL: @test_vld1q_lane_f16(
// CHECK: [[TMP0:%.*]] = bitcast half* %a to i8*
// CHECK: [[TMP1:%.*]] = bitcast <8 x half> %b to <16 x i8>
-// CHECK: [[TMP2:%.*]] = bitcast <16 x i8> [[TMP1]] to <8 x i16>
-// CHECK: [[TMP3:%.*]] = bitcast i8* [[TMP0]] to i16*
-// CHECK: [[TMP4:%.*]] = load i16, i16* [[TMP3]], align 2
-// CHECK: [[VLD1_LANE:%.*]] = insertelement <8 x i16> [[TMP2]], i16 [[TMP4]], i32 7
-// CHECK: [[TMP5:%.*]] = bitcast <8 x i16> [[VLD1_LANE]] to <8 x half>
-// CHECK: ret <8 x half> [[TMP5]]
+// CHECK: [[TMP2:%.*]] = bitcast <16 x i8> [[TMP1]] to <8 x half>
+// CHECK: [[TMP3:%.*]] = bitcast i8* [[TMP0]] to half*
+// CHECK: [[TMP4:%.*]] = load half, half* [[TMP3]], align 2
+// CHECK: [[VLD1_LANE:%.*]] = insertelement <8 x half> [[TMP2]], half [[TMP4]], i32 7
+// CHECK: ret <8 x half> [[VLD1_LANE]]
float16x8_t test_vld1q_lane_f16(float16_t const * a, float16x8_t b) {
return vld1q_lane_f16(a, b, 7);
}
@@ -4498,12 +4494,11 @@
// CHECK-LABEL: @test_vld1_lane_f16(
// CHECK: [[TMP0:%.*]] = bitcast half* %a to i8*
// CHECK: [[TMP1:%.*]] = bitcast <4 x half> %b to <8 x i8>
-// CHECK: [[TMP2:%.*]] = bitcast <8 x i8
[PATCH] D50627: [clangd] Add a testcase for empty preamble.
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, javed.absar.
clangd maintains the last good preamble for each TU and clang treats an empty
preamble as an error, therefore, clangd will use the stale preamble for
a latest AST, which causes some wired issues for the features using AST
(e.g. GoToDefintion).
A testcase:
1. Open a file contains some includes (like "#include ") in clangd
2. Change the whole content of the file with the source without includes (like
"double funcc()")
3. Call findDefinition in "double fun^cc()", it will goes to #include file
vector -- because we discard the empty preamble built in step2, and still hold
the stale preamble in step1
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50627
Files:
unittests/clangd/TUSchedulerTests.cpp
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -308,6 +308,49 @@
UnorderedElementsAre(Foo, AnyOf(Bar, Baz)));
}
+TEST_F(TUSchedulerTests, EmptyPreamble) {
+ TUScheduler S(
+ /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
+ PreambleParsedCallback(),
+ /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+ ASTRetentionPolicy());
+
+ auto Foo = testPath("foo.cpp");
+ auto Header = testPath("foo.h");
+
+ Files[Header] = "void foo()";
+ Timestamps[Header] = time_t(0);
+ auto WithPreamble = R"cpp(
+#include "foo.h"
+int main() {}
+ )cpp";
+ auto WithEmptyPreamble = R"cpp(int main() {})cpp";
+ S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto,
+ [](std::vector) {});
+ S.runWithPreamble("getNonEmptyPreamble", Foo,
+[&](llvm::Expected Preamble) {
+ // We expect to get a non-empty preamble.
+ ASSERT_TRUE(bool(Preamble));
+ EXPECT_GT(Preamble->Preamble->Preamble.getBounds().Size,
+0u);
+});
+ // Wait for the preamble is being built.
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+
+ // Update the file which results in an empty preamble.
+ S.update(Foo, getInputs(Foo, WithEmptyPreamble), WantDiagnostics::Auto,
+ [](std::vector) {});
+ // Wait for the preamble is being built.
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+ S.runWithPreamble("getEmptyPreamble", Foo,
+[&](llvm::Expected Preamble) {
+ // We expect to get an empty preamble.
+ ASSERT_TRUE(bool(Preamble));
+ EXPECT_EQ(Preamble->Preamble->Preamble.getBounds().Size,
+0u);
+});
+}
+
TEST_F(TUSchedulerTests, RunWaitsForPreamble) {
// Testing strategy: we update the file and schedule a few preamble reads at
// the same time. All reads should get the same non-null preamble.
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -308,6 +308,49 @@
UnorderedElementsAre(Foo, AnyOf(Bar, Baz)));
}
+TEST_F(TUSchedulerTests, EmptyPreamble) {
+ TUScheduler S(
+ /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
+ PreambleParsedCallback(),
+ /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+ ASTRetentionPolicy());
+
+ auto Foo = testPath("foo.cpp");
+ auto Header = testPath("foo.h");
+
+ Files[Header] = "void foo()";
+ Timestamps[Header] = time_t(0);
+ auto WithPreamble = R"cpp(
+#include "foo.h"
+int main() {}
+ )cpp";
+ auto WithEmptyPreamble = R"cpp(int main() {})cpp";
+ S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto,
+ [](std::vector) {});
+ S.runWithPreamble("getNonEmptyPreamble", Foo,
+[&](llvm::Expected Preamble) {
+ // We expect to get a non-empty preamble.
+ ASSERT_TRUE(bool(Preamble));
+ EXPECT_GT(Preamble->Preamble->Preamble.getBounds().Size,
+0u);
+});
+ // Wait for the preamble is being built.
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+
+ // Update the file which results in an empty preamble.
+ S.update(Foo, getInputs(Foo, WithEmptyPreamble), WantDiagnostics::Auto,
+ [](std::vector) {});
+ // Wait for the preamble is being built.
+ ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(1)));
+ S.runWithPreamble("getEmptyPreamble", Foo,
+[&](llvm::Expected Preamble) {
+ // We expect to get an empty preamble.
+ ASSERT_TRUE(bool(Preamble));
+ EXPECT_EQ(Preamble->Preamble->Pream
[PATCH] D50628: [Preamble] Empty preamble is not an error.
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Empty preamble is valid for source file which doesn't have any
preprocessor and #includes.
This patch makes clang treat an empty preamble as a normal preamble.
Check: ninja check-clang
A testcase is added in https://reviews.llvm.org/D50627.
Repository:
rC Clang
https://reviews.llvm.org/D50628
Files:
include/clang/Frontend/PrecompiledPreamble.h
lib/Frontend/ASTUnit.cpp
lib/Frontend/PrecompiledPreamble.cpp
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -237,9 +237,6 @@
PreambleCallbacks &Callbacks) {
assert(VFS && "VFS is null");
- if (!Bounds.Size)
-return BuildPreambleError::PreambleIsEmpty;
-
auto PreambleInvocation = std::make_shared(Invocation);
FrontendOptions &FrontendOpts = PreambleInvocation->getFrontendOpts();
PreprocessorOptions &PreprocessorOpts =
@@ -423,9 +420,6 @@
PreprocessorOptions &PreprocessorOpts =
PreambleInvocation->getPreprocessorOpts();
- if (!Bounds.Size)
-return false;
-
// We've previously computed a preamble. Check whether we have the same
// preamble now that we did before, and that there's enough space in
// the main-file buffer within the precompiled preamble to fit the
@@ -758,8 +752,6 @@
std::string BuildPreambleErrorCategory::message(int condition) const {
switch (static_cast(condition)) {
- case BuildPreambleError::PreambleIsEmpty:
-return "Preamble is empty";
case BuildPreambleError::CouldntCreateTempFile:
return "Could not create temporary file for PCH";
case BuildPreambleError::CouldntCreateTargetInfo:
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1363,7 +1363,6 @@
} else {
switch (static_cast(NewPreamble.getError().value()))
{
case BuildPreambleError::CouldntCreateTempFile:
- case BuildPreambleError::PreambleIsEmpty:
// Try again next time.
PreambleRebuildCounter = 1;
return nullptr;
Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -286,8 +286,7 @@
};
enum class BuildPreambleError {
- PreambleIsEmpty = 1,
- CouldntCreateTempFile,
+ CouldntCreateTempFile = 1,
CouldntCreateTargetInfo,
BeginSourceFileFailed,
CouldntEmitPCH
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -237,9 +237,6 @@
PreambleCallbacks &Callbacks) {
assert(VFS && "VFS is null");
- if (!Bounds.Size)
-return BuildPreambleError::PreambleIsEmpty;
-
auto PreambleInvocation = std::make_shared(Invocation);
FrontendOptions &FrontendOpts = PreambleInvocation->getFrontendOpts();
PreprocessorOptions &PreprocessorOpts =
@@ -423,9 +420,6 @@
PreprocessorOptions &PreprocessorOpts =
PreambleInvocation->getPreprocessorOpts();
- if (!Bounds.Size)
-return false;
-
// We've previously computed a preamble. Check whether we have the same
// preamble now that we did before, and that there's enough space in
// the main-file buffer within the precompiled preamble to fit the
@@ -758,8 +752,6 @@
std::string BuildPreambleErrorCategory::message(int condition) const {
switch (static_cast(condition)) {
- case BuildPreambleError::PreambleIsEmpty:
-return "Preamble is empty";
case BuildPreambleError::CouldntCreateTempFile:
return "Could not create temporary file for PCH";
case BuildPreambleError::CouldntCreateTargetInfo:
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1363,7 +1363,6 @@
} else {
switch (static_cast(NewPreamble.getError().value())) {
case BuildPreambleError::CouldntCreateTempFile:
- case BuildPreambleError::PreambleIsEmpty:
// Try again next time.
PreambleRebuildCounter = 1;
return nullptr;
Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -286,8 +286,7 @@
};
enum class BuildPreambleError {
- PreambleIsEmpty = 1,
- CouldntCreateTempFile,
+ CouldntCreateTempFile = 1,
CouldntCreateTargetInfo,
BeginSourceFileFailed,
CouldntEmitPCH
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50630: [AST] Update/correct the static_asserts for the bit-fields in Type
riccibruno created this revision.
riccibruno added reviewers: erichkeane, rjmccall.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
The current static_assert only checks that ObjCObjectTypeBitfields
fits into an unsigned. However it turns out that FunctionTypeBitfields
do not currently fits into an unsigned. Therefore the anonymous
union containing the bit-fields always use 8 bytes instead of 4.
This patch remove the lone misguided static_assert and systematically
check the size of each bit-field.
Repository:
rC Clang
https://reviews.llvm.org/D50630
Files:
include/clang/AST/Type.h
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1557,8 +1557,6 @@
unsigned IsKindOf : 1;
};
- static_assert(NumTypeBits + 7 + 6 + 1 <= 32, "Does not fit in an unsigned");
-
class ReferenceTypeBitfields {
friend class ReferenceType;
@@ -1637,6 +1635,27 @@
ReferenceTypeBitfields ReferenceTypeBits;
TypeWithKeywordBitfields TypeWithKeywordBits;
VectorTypeBitfields VectorTypeBits;
+
+static_assert(sizeof(TypeBitfields) <= 4,
+ "TypeBitfields is larger than 4 bytes!");
+static_assert(sizeof(ArrayTypeBitfields) <= 8,
+ "ArrayTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AttributedTypeBitfields) <= 8,
+ "AttributedTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AutoTypeBitfields) <= 8,
+ "AutoTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(BuiltinTypeBitfields) <= 8,
+ "BuiltinTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(FunctionTypeBitfields) <= 8,
+ "FunctionTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ObjCObjectTypeBitfields) <= 8,
+ "ObjCObjectTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ReferenceTypeBitfields) <= 8,
+ "ReferenceTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(TypeWithKeywordBitfields) <= 8,
+ "TypeWithKeywordBitfields is larger than 8 bytes!");
+static_assert(sizeof(VectorTypeBitfields) <= 8,
+ "VectorTypeBitfields is larger than 8 bytes!");
};
private:
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1557,8 +1557,6 @@
unsigned IsKindOf : 1;
};
- static_assert(NumTypeBits + 7 + 6 + 1 <= 32, "Does not fit in an unsigned");
-
class ReferenceTypeBitfields {
friend class ReferenceType;
@@ -1637,6 +1635,27 @@
ReferenceTypeBitfields ReferenceTypeBits;
TypeWithKeywordBitfields TypeWithKeywordBits;
VectorTypeBitfields VectorTypeBits;
+
+static_assert(sizeof(TypeBitfields) <= 4,
+ "TypeBitfields is larger than 4 bytes!");
+static_assert(sizeof(ArrayTypeBitfields) <= 8,
+ "ArrayTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AttributedTypeBitfields) <= 8,
+ "AttributedTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AutoTypeBitfields) <= 8,
+ "AutoTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(BuiltinTypeBitfields) <= 8,
+ "BuiltinTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(FunctionTypeBitfields) <= 8,
+ "FunctionTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ObjCObjectTypeBitfields) <= 8,
+ "ObjCObjectTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ReferenceTypeBitfields) <= 8,
+ "ReferenceTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(TypeWithKeywordBitfields) <= 8,
+ "TypeWithKeywordBitfields is larger than 8 bytes!");
+static_assert(sizeof(VectorTypeBitfields) <= 8,
+ "VectorTypeBitfields is larger than 8 bytes!");
};
private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50631: [AST] Stuff more data into FunctionTypeBitfields
riccibruno created this revision.
riccibruno added reviewers: erichkeane, rjmccall.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
Since FunctionTypeBitfields is already > 32 bits wide we might
as well stuff the remaining bits from FunctionProtoType into it.
The patch is very mechanical, but changes the maximum number of
parameters from 2^15-1 to 2^12-1 to fit into FunctionTypeBitfields.
This seems still large enough, and in any case according to the
annex B of the C++ standard we only need to be able to accept
256 parameters.
Also the member `unsigned RefQualifier : 2;` is moved in
FunctionTypeBitfields. This is intentional and done to avoid
the unsigned boundary.
Repository:
rC Clang
https://reviews.llvm.org/D50631
Files:
include/clang/AST/Type.h
lib/AST/Type.cpp
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2844,24 +2844,23 @@
FunctionProtoType::FunctionProtoType(QualType result, ArrayRef params,
QualType canonical,
const ExtProtoInfo &epi)
-: FunctionType(FunctionProto, result, canonical,
- result->isDependentType(),
+: FunctionType(FunctionProto, result, canonical, result->isDependentType(),
result->isInstantiationDependentType(),
result->isVariablyModifiedType(),
- result->containsUnexpandedParameterPack(), epi.ExtInfo),
- NumParams(params.size()),
- NumExceptions(epi.ExceptionSpec.Exceptions.size()),
- ExceptionSpecType(epi.ExceptionSpec.Type),
- HasExtParameterInfos(epi.ExtParameterInfos != nullptr),
- Variadic(epi.Variadic), HasTrailingReturn(epi.HasTrailingReturn) {
- assert(NumParams == params.size() && "function has too many parameters");
-
+ result->containsUnexpandedParameterPack(), epi.ExtInfo) {
FunctionTypeBits.TypeQuals = epi.TypeQuals;
FunctionTypeBits.RefQualifier = epi.RefQualifier;
+ FunctionTypeBits.NumParams = params.size();
+ assert(getNumParams() == params.size() && "function w. too many parameters!");
+ FunctionTypeBits.NumExceptions = epi.ExceptionSpec.Exceptions.size();
+ FunctionTypeBits.ExceptionSpecType = epi.ExceptionSpec.Type;
+ FunctionTypeBits.HasExtParameterInfos = !!epi.ExtParameterInfos;
+ FunctionTypeBits.Variadic = epi.Variadic;
+ FunctionTypeBits.HasTrailingReturn = epi.HasTrailingReturn;
// Fill in the trailing argument array.
auto *argSlot = reinterpret_cast(this+1);
- for (unsigned i = 0; i != NumParams; ++i) {
+ for (unsigned i = 0, N = getNumParams(); i != N; ++i) {
if (params[i]->isDependentType())
setDependent();
else if (params[i]->isInstantiationDependentType())
@@ -2875,7 +2874,7 @@
if (getExceptionSpecType() == EST_Dynamic) {
// Fill in the exception array.
-QualType *exnSlot = argSlot + NumParams;
+QualType *exnSlot = argSlot + getNumParams();
unsigned I = 0;
for (QualType ExceptionType : epi.ExceptionSpec.Exceptions) {
// Note that, before C++17, a dependent exception specification does
@@ -2895,7 +2894,7 @@
epi.ExceptionSpec.NoexceptExpr->isValueDependent());
// Store the noexcept expression and context.
-auto **noexSlot = reinterpret_cast(argSlot + NumParams);
+auto **noexSlot = reinterpret_cast(argSlot + getNumParams());
*noexSlot = epi.ExceptionSpec.NoexceptExpr;
if (epi.ExceptionSpec.NoexceptExpr->isValueDependent() ||
@@ -2907,15 +2906,15 @@
} else if (getExceptionSpecType() == EST_Uninstantiated) {
// Store the function decl from which we will resolve our
// exception specification.
-auto **slot = reinterpret_cast(argSlot + NumParams);
+auto **slot = reinterpret_cast(argSlot + getNumParams());
slot[0] = epi.ExceptionSpec.SourceDecl;
slot[1] = epi.ExceptionSpec.SourceTemplate;
// This exception specification doesn't make the type dependent, because
// it's not instantiated as part of instantiating the type.
} else if (getExceptionSpecType() == EST_Unevaluated) {
// Store the function decl from which we will resolve our
// exception specification.
-auto **slot = reinterpret_cast(argSlot + NumParams);
+auto **slot = reinterpret_cast(argSlot + getNumParams());
slot[0] = epi.ExceptionSpec.SourceDecl;
}
@@ -2935,7 +2934,7 @@
if (epi.ExtParameterInfos) {
auto *extParamInfos =
const_cast(getExtParameterInfosBuffer());
-for (unsigned i = 0; i != NumParams; ++i)
+for (unsigned i = 0, N = getNumParams(); i != N; ++i)
extParamInfos[i] = epi.ExtParameterInfos[i];
}
}
@@ -2981,7 +2980,7 @@
case EST_Dynamic:
// A dynamic exception specification is throwing unless every exception
// type is an (unexpanded) pack expansion type.
-for (unsigned I = 0, N = NumExceptions; I != N; ++I)
+for (unsig
[PATCH] D50627: [clangd] Add a testcase for empty preamble.
ilya-biryukov added a comment.
Maybe also add a test for find-definition that was broken before? (non-empty
preamble -> empty preamble -> bad gotodef that goes to included file instead of
the local variable)
To have a regression test against similar failures.
Comment at: unittests/clangd/TUSchedulerTests.cpp:312
+TEST_F(TUSchedulerTests, EmptyPreamble) {
+ TUScheduler S(
+ /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
Maybe call PrecompiledPreamble directly? it requires some setup, but tests the
modified code without extra layers that TUScheduler adds.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50627
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50628: [Preamble] Empty preamble is not an error.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D50628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50438: [clangd] Sort GoToDefinition results.
ilya-biryukov added inline comments.
Comment at: clangd/XRefs.cpp:71
+struct DeclInfo {
+ const Decl *D;
NIT: maybe call `Occurence` instead? As this is actually a `Decl` with some
extra data, computed based on the expression it originated from. Occurence
seems like a nice that for that kind of thing.
Comment at: clangd/XRefs.cpp:105
+// Sort results. Declarations being referenced explicitly come first.
+std::sort(Result.begin(), Result.end(),
+ [](const DeclInfo &L, const DeclInfo &R) {
Maybe sort further at the callers instead?
It would be a more intrusive change, but would also give a well-defined result
order for findDefinitions and other cases. E.g. findDefinitions currently gives
results in an arbitrary order (specifically, the one supplied by DenseMap+sort)
when multiple decls are returned.
WDYT?
Comment at: clangd/XRefs.cpp:128
+ void insertDecl(const Decl *D, bool IsExplicitReferenced) {
+auto It = Decls.find(D);
+if (It != Decls.end())
Maybe simplify to `Decls[D] |= IsExplicitReferenced`?
Comment at: clangd/XRefs.cpp:143
+ auto hasImplicitExpr = [](const Expr *E) {
+if (E && E->child_begin() != E->child_end()) {
+ // Use the first child is good enough for most cases -- normally the
NIT: [use early
exits](https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code)
by inverting condition
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50438
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r339264 - [CGObjCGNU] Rename GetSelector helper method to fix -Woverloaded-virtual warning (PR38210)
Merged to 7.0 in r339555.
On Wed, Aug 8, 2018 at 5:53 PM, Simon Pilgrim via cfe-commits
wrote:
> Author: rksimon
> Date: Wed Aug 8 08:53:14 2018
> New Revision: 339264
>
> URL: http://llvm.org/viewvc/llvm-project?rev=339264&view=rev
> Log:
> [CGObjCGNU] Rename GetSelector helper method to fix -Woverloaded-virtual
> warning (PR38210)
>
> As suggested by @theraven on PR38210, this patch fixes the gcc
> -Woverloaded-virtual warnings by renaming the extra CGObjCGNU::GetSelector
> method to CGObjCGNU::GetTypedSelector
>
> Differential Revision: https://reviews.llvm.org/D50448
>
> Modified:
> cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCGNU.cpp?rev=339264&r1=339263&r2=339264&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGObjCGNU.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGObjCGNU.cpp Wed Aug 8 08:53:14 2018
> @@ -510,8 +510,8 @@ protected:
>
>/// Returns a selector with the specified type encoding. An empty string
> is
>/// used to return an untyped selector (with the types field set to NULL).
> - virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
> - const std::string &TypeEncoding);
> + virtual llvm::Value *GetTypedSelector(CodeGenFunction &CGF, Selector Sel,
> +const std::string &TypeEncoding);
>
>/// Returns the name of ivar offset variables. In the GNUstep v1 ABI, this
>/// contains the class and ivar names, in the v2 ABI this contains the type
> @@ -1342,8 +1342,8 @@ class CGObjCGNUstep2 : public CGObjCGNUs
>return Val;
> return llvm::ConstantExpr::getBitCast(Val, Ty);
>}
> - llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
> -const std::string &TypeEncoding) override {
> + llvm::Value *GetTypedSelector(CodeGenFunction &CGF, Selector Sel,
> +const std::string &TypeEncoding) override {
> return GetConstantSelector(Sel, TypeEncoding);
>}
>llvm::Constant *GetTypeString(llvm::StringRef TypeEncoding) {
> @@ -2121,8 +2121,8 @@ llvm::Value *CGObjCGNU::EmitNSAutoreleas
>return Value;
> }
>
> -llvm::Value *CGObjCGNU::GetSelector(CodeGenFunction &CGF, Selector Sel,
> -const std::string &TypeEncoding) {
> +llvm::Value *CGObjCGNU::GetTypedSelector(CodeGenFunction &CGF, Selector Sel,
> + const std::string &TypeEncoding) {
>SmallVectorImpl &Types = SelectorTable[Sel];
>llvm::GlobalAlias *SelValue = nullptr;
>
> @@ -2155,13 +2155,13 @@ Address CGObjCGNU::GetAddrOfSelector(Cod
> }
>
> llvm::Value *CGObjCGNU::GetSelector(CodeGenFunction &CGF, Selector Sel) {
> - return GetSelector(CGF, Sel, std::string());
> + return GetTypedSelector(CGF, Sel, std::string());
> }
>
> llvm::Value *CGObjCGNU::GetSelector(CodeGenFunction &CGF,
> const ObjCMethodDecl *Method) {
>std::string SelTypes =
> CGM.getContext().getObjCEncodingForMethodDecl(Method);
> - return GetSelector(CGF, Method->getSelector(), SelTypes);
> + return GetTypedSelector(CGF, Method->getSelector(), SelTypes);
> }
>
> llvm::Constant *CGObjCGNU::GetEHType(QualType T) {
>
>
> ___
> 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] D50580: [clang-tidy] Abseil: no namespace check
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:23
+
+ Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
hokein wrote:
> aaron.ballman wrote:
> > I think this needs a `not(isExpansionInSystemHeader())` in there, as this
> > is going to trigger on code that includes a header file using an `absl`
> > namespace. If I'm incorrect and users typically include abseil as something
> > other than system includes, you'll have to find a different way to solve
> > this.
> Yeah, we have discussed this issue internally. abseil-user code usually
> includes abseil header like `#include "whatever/abseil/base/optimization.h"`,
> so `IsExpansionInSystemHeader` doesn't work for most cases.
>
> We need a way to filter out all headers being a part of abseil library. I
> think we can create a matcher `InExpansionInAbseilHeader` -- basically using
> `IsExpansionInFileMatching` with a regex expression that matches typical
> abseil include path. What do you think?
>
> And we'll have more abseil checks (e.g. `abseil-no-internal-deps`) that use
> `InExpansionInAbseilHeader`, we should put it to a common header.
I think that is a sensible approach.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50580
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r339074 - [lit, python] Always add quotes around the python path in lit
Merged to 7.0 in r339554.
On Tue, Aug 7, 2018 at 12:37 AM, Stella Stamenova via cfe-commits
wrote:
> Author: stella.stamenova
> Date: Mon Aug 6 15:37:45 2018
> New Revision: 339074
>
> URL: http://llvm.org/viewvc/llvm-project?rev=339074&view=rev
> Log:
> [lit, python] Always add quotes around the python path in lit
>
> Summary:
> The issue with the python path is that the path to python on Windows can
> contain spaces. To make the tests always work, the path to python needs to be
> surrounded by quotes.
>
> This is a companion change to: https://reviews.llvm.org/D50206
>
> Reviewers: asmith, zturner
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D50281
>
> Modified:
> cfe/trunk/test/Tooling/clang-diff-json.cpp
>
> Modified: cfe/trunk/test/Tooling/clang-diff-json.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Tooling/clang-diff-json.cpp?rev=339074&r1=339073&r2=339074&view=diff
> ==
> --- cfe/trunk/test/Tooling/clang-diff-json.cpp (original)
> +++ cfe/trunk/test/Tooling/clang-diff-json.cpp Mon Aug 6 15:37:45 2018
> @@ -1,10 +1,10 @@
> // RUN: clang-diff -ast-dump-json %s -- \
> -// RUN: | '%python' -c 'import json, sys;
> json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True,
> indent=2)' \
> +// RUN: | %python -c 'import json, sys;
> json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True,
> indent=2)' \
> // RUN: | FileCheck %s
>
> -// CHECK: "begin": 301,
> +// CHECK: "begin": 311,
> // CHECK: "type": "FieldDecl",
> -// CHECK: "end": 321,
> +// CHECK: "end": 319,
> // CHECK: "type": "CXXRecordDecl",
> class A {
>int x;
>
>
> ___
> 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
[clang-tools-extra] r339557 - Fix MSVC 'std::min: no matching overloaded function found' error.
Author: rksimon
Date: Mon Aug 13 05:24:48 2018
New Revision: 339557
URL: http://llvm.org/viewvc/llvm-project?rev=339557&view=rev
Log:
Fix MSVC 'std::min: no matching overloaded function found' error.
Modified:
clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
Modified: clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp?rev=339557&r1=339556&r2=339557&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Trigram.cpp Mon Aug 13 05:24:48
2018
@@ -128,7 +128,8 @@ std::vector generateQueryTrigrams
// If the number of symbols which can form fuzzy matching trigram is not
// sufficient, generate a single incomplete trigram for query.
if (ValidSymbolsCount < 3) {
-std::string Chars = LowercaseQuery.substr(0, std::min(3UL, Query.size()));
+std::string Chars =
+LowercaseQuery.substr(0, std::min(3UL, Query.size()));
Chars.append(3 - Chars.size(), END_MARKER);
UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
} else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY
ldionne added a comment. In https://reviews.llvm.org/D49240#1196878, @hans wrote: > The reason we noticed this was that it caused a *50 GB* size increase of the > build output on our buildbots, which was enough to cause infrastructure > problems. > > This change was also committed shortly before the 7.0 branch, so it's part of > the 7.0.0 release candidates. > > Should we back it out until a solution is found? The thing is -- there's no solution without changing the guarantees that libc++ make. Today, libc++ guarantees that you can link TUs that were built with different versions of libc++ together. If we remove that guarantee, then we can use linkonce_odr and solve the problem that Chromium is having. Is that guarantee something that Chromium is relying upon? Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r339558 - Revert "Allow relockable scopes with thread safety attributes."
Author: hokein
Date: Mon Aug 13 05:50:30 2018
New Revision: 339558
URL: http://llvm.org/viewvc/llvm-project?rev=339558&view=rev
Log:
Revert "Allow relockable scopes with thread safety attributes."
This reverts commit r339456.
The change introduces a new crash, see
class SCOPED_LOCKABLE FileLock {
public:
explicit FileLock()
EXCLUSIVE_LOCK_FUNCTION(file_);
~FileLock() UNLOCK_FUNCTION(file_);
void Lock() EXCLUSIVE_LOCK_FUNCTION(file_);
Mutex file_;
};
void relockShared2() {
FileLock file_lock;
file_lock.Lock();
}
Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp
Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=339558&r1=339557&r2=339558&view=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Mon Aug 13 05:50:30 2018
@@ -86,8 +86,8 @@ static void warnInvalidLock(ThreadSafety
namespace {
-/// A set of CapabilityExpr objects, which are compiled from thread safety
-/// attributes on a function.
+/// A set of CapabilityInfo objects, which are compiled from the
+/// requires attributes on a function.
class CapExprSet : public SmallVector {
public:
/// Push M onto list, but discard duplicates.
@@ -944,23 +944,6 @@ public:
if (FullyRemove)
FSet.removeLock(FactMan, Cp);
}
-
- void Relock(FactSet &FSet, FactManager &FactMan, LockKind LK,
- SourceLocation RelockLoc, ThreadSafetyHandler &Handler,
- StringRef DiagKind) const {
-for (const auto *UnderlyingMutex : UnderlyingMutexes) {
- CapabilityExpr UnderCp(UnderlyingMutex, false);
-
- // We're relocking the underlying mutexes. Warn on double locking.
- if (FSet.findLock(FactMan, UnderCp))
-Handler.handleDoubleLock(DiagKind, UnderCp.toString(), RelockLoc);
- else {
-FSet.removeLock(FactMan, !UnderCp);
-FSet.addLock(FactMan, llvm::make_unique(UnderCp, LK,
- RelockLoc));
- }
-}
- }
};
/// Class which implements the core thread safety analysis routines.
@@ -991,9 +974,6 @@ public:
void removeLock(FactSet &FSet, const CapabilityExpr &CapE,
SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind,
StringRef DiagKind);
- void relockScopedLock(FactSet &FSet, const CapabilityExpr &CapE,
-SourceLocation RelockLoc, LockKind Kind,
-StringRef DiagKind);
template
void getMutexIDs(CapExprSet &Mtxs, AttrType *Attr, Expr *Exp,
@@ -1305,27 +1285,6 @@ void ThreadSafetyAnalyzer::removeLock(Fa
DiagKind);
}
-void ThreadSafetyAnalyzer::relockScopedLock(FactSet &FSet,
-const CapabilityExpr &Cp,
-SourceLocation RelockLoc,
-LockKind Kind, StringRef DiagKind)
{
- if (Cp.shouldIgnore())
-return;
-
- const FactEntry *LDat = FSet.findLock(FactMan, Cp);
- if (!LDat) {
-// FIXME: It's possible to manually destruct the scope and then relock it.
-// Should that be a separate warning? For now we pretend nothing happened.
-// It's undefined behavior, so not related to thread safety.
-return;
- }
-
- // We should only land here if Cp is a scoped capability, so if we have any
- // fact, it must be a ScopedLockableFactEntry.
- const auto *SLDat = static_cast(LDat);
- SLDat->Relock(FSet, FactMan, Kind, RelockLoc, Handler, DiagKind);
-}
-
/// Extract the list of mutexIDs from the attribute on an expression,
/// and push them onto Mtxs, discarding any duplicates.
template
@@ -1767,19 +1726,14 @@ void BuildLockset::handleCall(Expr *Exp,
StringRef CapDiagKind = "mutex";
// Figure out if we're constructing an object of scoped lockable class
- bool isScopedConstructor = false, isScopedMemberCall = false;
+ bool isScopedVar = false;
if (VD) {
if (const auto *CD = dyn_cast(D)) {
const CXXRecordDecl* PD = CD->getParent();
if (PD && PD->hasAttr())
-isScopedConstructor = true;
+isScopedVar = true;
}
}
- if (const auto *MCD = dyn_cast(Exp)) {
-const CXXRecordDecl *PD = MCD->getRecordDecl();
-if (PD && PD->hasAttr())
- isScopedMemberCall = true;
- }
for(const Attr *At : D->attrs()) {
switch (At->getKind()) {
@@ -1859,7 +1813,7 @@ void BuildLockset::handleCall(Expr *Exp,
POK_FunctionCall, ClassifyDiagnostic(A),
Exp->getExprLoc());
// use for adopting a lock
- if (isScopedConstructor) {
+ if (isScopedVar) {
Analyzer->getMutexIDs(A->isShared() ? ScopedSharedReqs
[PATCH] D49885: Thread safety analysis: Allow relockable scopes
hokein added a comment.
Hello, this patch seems introduce a new crash, and I have reverted it in
r339558.
Here is the minimal test case:
class SCOPED_LOCKABLE FileLock {
public:
explicit FileLock()
EXCLUSIVE_LOCK_FUNCTION(file_);
~FileLock() UNLOCK_FUNCTION(file_);
//void Release() UNLOCK_FUNCTION(file_);
void Lock() EXCLUSIVE_LOCK_FUNCTION(file_);
Mutex file_;
};
void relockShared2() {
FileLock file_lock;
file_lock.Lock();
}
Repository:
rC Clang
https://reviews.llvm.org/D49885
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50516: [ASTImporter] Improved import of friend templates.
balazske updated this revision to Diff 160335.
balazske added a comment.
- replaced ASSERT in test
Repository:
rC Clang
https://reviews.llvm.org/D50516
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -2677,6 +2677,93 @@
EXPECT_EQ(FromIndex, 3u);
}
+TEST_P(
+ASTImporterTestBase,
+ImportOfFriendRecordDoesNotMergeDefinition) {
+ Decl *FromTU = getTuDecl(
+ R"(
+ class A {
+template class F {};
+class X {
+ template friend class F;
+};
+ };
+ )",
+ Lang_CXX, "input0.cc");
+
+ auto *FromClass = FirstDeclMatcher().match(
+ FromTU, cxxRecordDecl(hasName("F"), isDefinition()));
+ auto *FromFriendClass = LastDeclMatcher().match(
+ FromTU, cxxRecordDecl(hasName("F")));
+
+ ASSERT_TRUE(FromClass);
+ ASSERT_TRUE(FromFriendClass);
+ ASSERT_NE(FromClass, FromFriendClass);
+ ASSERT_EQ(FromFriendClass->getDefinition(), FromClass);
+ ASSERT_EQ(FromFriendClass->getPreviousDecl(), FromClass);
+ ASSERT_EQ(
+ FromFriendClass->getDescribedClassTemplate()->getPreviousDecl(),
+ FromClass->getDescribedClassTemplate());
+
+ auto *ToClass = cast(Import(FromClass, Lang_CXX));
+ auto *ToFriendClass = cast(Import(FromFriendClass, Lang_CXX));
+
+ EXPECT_TRUE(ToClass);
+ EXPECT_TRUE(ToFriendClass);
+ EXPECT_NE(ToClass, ToFriendClass);
+ EXPECT_EQ(ToFriendClass->getDefinition(), ToClass);
+ EXPECT_EQ(ToFriendClass->getPreviousDecl(), ToClass);
+ EXPECT_EQ(
+ ToFriendClass->getDescribedClassTemplate()->getPreviousDecl(),
+ ToClass->getDescribedClassTemplate());
+}
+
+TEST_P(
+ASTImporterTestBase,
+ImportOfRecursiveFriendClass) {
+ Decl *FromTu = getTuDecl(
+ R"(
+ class declToImport {
+friend class declToImport;
+ };
+ )",
+ Lang_CXX, "input.cc");
+
+ auto *FromD = FirstDeclMatcher().match(
+ FromTu, cxxRecordDecl(hasName("declToImport")));
+ auto *ToD = Import(FromD, Lang_CXX);
+ auto Pattern = cxxRecordDecl(hasName("declToImport"), has(friendDecl()));
+ ASSERT_TRUE(MatchVerifier{}.match(FromD, Pattern));
+ EXPECT_TRUE(MatchVerifier{}.match(ToD, Pattern));
+}
+
+TEST_P(
+ASTImporterTestBase,
+ImportOfRecursiveFriendClassTemplate) {
+ Decl *FromTu = getTuDecl(
+ R"(
+ template class declToImport {
+template friend class declToImport;
+ };
+ )",
+ Lang_CXX, "input.cc");
+
+ auto *FromD = FirstDeclMatcher().match(
+ FromTu, classTemplateDecl(hasName("declToImport")));
+ auto *ToD = Import(FromD, Lang_CXX);
+
+ auto Pattern = classTemplateDecl(
+ has(cxxRecordDecl(has(friendDecl(has(classTemplateDecl()));
+ ASSERT_TRUE(MatchVerifier{}.match(FromD, Pattern));
+ EXPECT_TRUE(MatchVerifier{}.match(ToD, Pattern));
+
+ auto *Class =
+ FirstDeclMatcher().match(ToD, classTemplateDecl());
+ auto *Friend = FirstDeclMatcher().match(ToD, friendDecl());
+ EXPECT_NE(Friend->getFriendDecl(), Class);
+ EXPECT_EQ(Friend->getFriendDecl()->getPreviousDecl(), Class);
+}
+
struct DeclContextTest : ASTImporterTestBase {};
TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2164,11 +2164,21 @@
}
Decl *ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
+ bool IsFriendTemplate = false;
+ if (auto *DCXX = dyn_cast(D)) {
+IsFriendTemplate =
+DCXX->getDescribedClassTemplate() &&
+DCXX->getDescribedClassTemplate()->getFriendObjectKind() !=
+Decl::FOK_None;
+ }
+
// If this record has a definition in the translation unit we're coming from,
// but this particular declaration is not that definition, import the
// definition and map to that.
TagDecl *Definition = D->getDefinition();
if (Definition && Definition != D &&
+ // Friend template declaration must be imported on its own.
+ !IsFriendTemplate &&
// In contrast to a normal CXXRecordDecl, the implicit
// CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
// The definition of the implicit CXXRecordDecl in this case is the
@@ -2241,7 +2251,7 @@
PrevDecl = FoundRecord;
if (RecordDecl *FoundDef = FoundRecord->getDefinition()) {
- if ((SearchName && !D->isCompleteDefinition())
+ if ((SearchName && !D->isCompleteDefinition() && !IsFriendTemplate)
|| (D->isCompleteDefinition() &&
D->isAnonymousStructOrUnion()
== FoundDef->isAnonymousStructOrUnion() &&
@@ -2281,6 +2291,9 @@
!IsStructuralMatch(D, FoundRecord))
continue;
+ if (IsFriendTemplate)
+continue;
[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check
deannagarcia added inline comments.
Comment at: test/clang-tidy/abseil-duration-division.cpp:58
+ // CHECK-MESSAGES: [[@LINE-4]]:45: warning: operator/ on absl::Duration
objects
+ // CHECK-FIXES: double DoubleDivision(T t1, T t2) {return
+ // absl::FDivDuration(t1, t2);}
hokein wrote:
> I think we should ignore templates. The template function could be used by
> other types, fixing the template is not correct.
I removed this template function, but left the template function TakesGeneric
below to show that the automatic type does not require/give a warning. Is that
alright?
https://reviews.llvm.org/D50389
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check
deannagarcia updated this revision to Diff 160336.
deannagarcia marked 6 inline comments as done.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesInt(int);
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+ absl::Duration d;
+
+ const double num_double = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+ const float num_float = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+ const auto SomeVal = 1.0 + d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+
+ TakesDouble(d/d);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+void TakesDouble(double);
+template
+void TakesGeneric(T);
+
+void Negatives() {
+ absl::Duration d;
+ const int num_int = d/d;
+ const long num_long = d/d;
+ const short num_short = d/d;
+ const char num_char = d/d;
+ const auto num_auto = d/d;
+ const auto SomeVal = 1 + d/d;
+
+ TakesInt(d/d);
+ TakesGeneric(d/d);
+ // Explicit cast should disable the warning.
+ const double num_cast1 = static_cast(d/d);
+ const double num_cast2 = (double)(d/d);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
=
.. toctree::
+ abseil-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends th
[PATCH] D50630: [AST] Update/correct the static_asserts for the bit-fields in Type
erichkeane added inline comments. Comment at: include/clang/AST/Type.h:1639 + +static_assert(sizeof(TypeBitfields) <= 4, + "TypeBitfields is larger than 4 bytes!"); I don't really see value in ensuring that TypeBitfields is <= 4 bytes, it seems to me that all we care about is the union size, so just <=8 is fine. Repository: rC Clang https://reviews.llvm.org/D50630 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50389: [clang-tidy] Abseil: integral division of Duration check
deannagarcia updated this revision to Diff 160338.
https://reviews.llvm.org/D50389
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationDivisionCheck.cpp
clang-tidy/abseil/DurationDivisionCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-division.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-division.cpp
Index: test/clang-tidy/abseil-duration-division.cpp
===
--- test/clang-tidy/abseil-duration-division.cpp
+++ test/clang-tidy/abseil-duration-division.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s abseil-duration-division %t
+
+namespace absl {
+
+class Duration {};
+
+int operator/(Duration lhs, Duration rhs);
+
+double FDivDuration(Duration num, Duration den);
+
+} // namespace absl
+
+void TakesDouble(double);
+
+#define MACRO_EQ(x, y) (x == y)
+#define MACRO_DIVEQ(x,y,z) (x/y == z)
+#define CHECK(x) (x)
+
+void Positives() {
+ absl::Duration d;
+
+ const double num_double = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: operator/ on absl::Duration objects performs integer division; did you mean to use FDivDuration()? [abseil-duration-division]
+ // CHECK-FIXES: const double num_double = absl::FDivDuration(d, d);
+ const float num_float = d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const float num_float = absl::FDivDuration(d, d);
+ const auto SomeVal = 1.0 + d/d;
+ // CHECK-MESSAGES: [[@LINE-1]]:31: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: const auto SomeVal = 1.0 + absl::FDivDuration(d, d);
+ if (MACRO_EQ(d/d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:17: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_EQ(absl::FDivDuration(d, d), 0.0)) {}
+ if (CHECK(MACRO_EQ(d/d, 0.0))) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:23: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (CHECK(MACRO_EQ(absl::FDivDuration(d, d), 0.0))) {}
+
+ // This one generates a message, but no fix.
+ if (MACRO_DIVEQ(d, d, 0.0)) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: if (MACRO_DIVEQ(d, d, 0.0)) {}
+
+ TakesDouble(d/d);
+ // CHECK-MESSAGES: [[@LINE-1]]:16: warning: operator/ on absl::Duration objects
+ // CHECK-FIXES: TakesDouble(absl::FDivDuration(d, d));
+}
+
+void TakesInt(int);
+template
+void TakesGeneric(T);
+
+void Negatives() {
+ absl::Duration d;
+ const int num_int = d/d;
+ const long num_long = d/d;
+ const short num_short = d/d;
+ const char num_char = d/d;
+ const auto num_auto = d/d;
+ const auto SomeVal = 1 + d/d;
+
+ TakesInt(d/d);
+ TakesGeneric(d/d);
+ // Explicit cast should disable the warning.
+ const double num_cast1 = static_cast(d/d);
+ const double num_cast2 = (double)(d/d);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
=
.. toctree::
+ abseil-duration-division
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/clang-tidy/checks/abseil-duration-division.rst
===
--- docs/clang-tidy/checks/abseil-duration-division.rst
+++ docs/clang-tidy/checks/abseil-duration-division.rst
@@ -0,0 +1,36 @@
+.. title:: clang-tidy - abseil-duration-division
+
+abseil-duration-division
+
+
+``absl::Duration`` arithmetic works like it does with integers. That means that
+division of two ``absl::Duration`` objects returns an ``int64`` with any fractional
+component truncated toward 0. See `this link `_ for more information on arithmetic with ``absl::Duration``.
+
+For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ int64 sec1 = d / absl::Seconds(1); // Truncates toward 0.
+ int64 sec2 = absl::ToInt64Seconds(d); // Equivalent to division.
+ assert(sec1 == 3 && sec2 == 3);
+
+ double dsec = d / absl::Seconds(1); // WRONG: Still truncates toward 0.
+ assert(dsec == 3.0);
+
+If you want floating-point division, you should use either the
+``absl::FDivDuration()`` function, or one of the unit conversion functions such
+as ``absl::ToDoubleSeconds()``. For example:
+
+.. code-block:: c++
+
+ absl::Duration d = absl::Seconds(3.5);
+ double dsec1 = absl::FDivDuration(d, absl::Seconds(1)); // GOOD: No truncation.
+ double dsec2 = absl::ToDoubleSeconds(d); // GOOD: No truncation.
+ assert(dsec1 == 3.5 && dsec2 == 3.5);
+
+
+This check looks for uses of ``absl::Duration`` division that is done in a
+floating-point context, and recommends the use of a function that returns a
+floating-po
[PATCH] D50631: [AST] Stuff more data into FunctionTypeBitfields
erichkeane added a subscriber: rsmith.
erichkeane added inline comments.
Comment at: include/clang/AST/Type.h:1527
+/// The number of parameters this function has, not counting '...'.
+unsigned NumParams : 12;
+
This concerns me a bit with variadic templates. I realize implimits says 256
but IMO variadic templates makes this a fairly easy limit to hit. I guess that
4096 is probably sufficient, though I'd like to hear someone else's opinion.
Comment at: include/clang/AST/Type.h:1530
+/// The number of types in the exception spec, if any.
+unsigned NumExceptions : 9;
+
IMO (and @rsmith should respond here instead), if we were looking to steal bits
from anywhere, this would be it. Exception-specifications that use types ares
are deprecated in the language and I'm not sure anyone ever used them anyway.
Comment at: lib/AST/Type.cpp:2863
auto *argSlot = reinterpret_cast(this+1);
- for (unsigned i = 0; i != NumParams; ++i) {
+ for (unsigned i = 0, N = getNumParams(); i != N; ++i) {
if (params[i]->isDependentType())
I would be unbelievably surprised if this change is worth-while. I can't see a
situation where this getNumParams call doesn't get inlined.
Repository:
rC Clang
https://reviews.llvm.org/D50631
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r339560 - [ASTImporter] Improved import of friend templates.
Author: balazske
Date: Mon Aug 13 06:08:37 2018
New Revision: 339560
URL: http://llvm.org/viewvc/llvm-project?rev=339560&view=rev
Log:
[ASTImporter] Improved import of friend templates.
Summary:
When importing a friend class template declaration,
this declaration should not be merged with any other existing declaration
for the same type. Otherwise the getFriendDecl of the FriendDecl can point
to an other already referenced declaration, this case causes problems.
Additionally the previous decl of class templates is set at import.
Reviewers: a.sidorin, a_sidorin
Reviewed By: a_sidorin
Subscribers: a_sidorin, martong, cfe-commits
Differential Revision: https://reviews.llvm.org/D50516
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=339560&r1=339559&r2=339560&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Mon Aug 13 06:08:37 2018
@@ -2164,11 +2164,21 @@ Decl *ASTNodeImporter::VisitEnumDecl(Enu
}
Decl *ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
+ bool IsFriendTemplate = false;
+ if (auto *DCXX = dyn_cast(D)) {
+IsFriendTemplate =
+DCXX->getDescribedClassTemplate() &&
+DCXX->getDescribedClassTemplate()->getFriendObjectKind() !=
+Decl::FOK_None;
+ }
+
// If this record has a definition in the translation unit we're coming from,
// but this particular declaration is not that definition, import the
// definition and map to that.
TagDecl *Definition = D->getDefinition();
if (Definition && Definition != D &&
+ // Friend template declaration must be imported on its own.
+ !IsFriendTemplate &&
// In contrast to a normal CXXRecordDecl, the implicit
// CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
// The definition of the implicit CXXRecordDecl in this case is the
@@ -2241,7 +2251,7 @@ Decl *ASTNodeImporter::VisitRecordDecl(R
PrevDecl = FoundRecord;
if (RecordDecl *FoundDef = FoundRecord->getDefinition()) {
- if ((SearchName && !D->isCompleteDefinition())
+ if ((SearchName && !D->isCompleteDefinition() && !IsFriendTemplate)
|| (D->isCompleteDefinition() &&
D->isAnonymousStructOrUnion()
== FoundDef->isAnonymousStructOrUnion() &&
@@ -2281,6 +2291,9 @@ Decl *ASTNodeImporter::VisitRecordDecl(R
!IsStructuralMatch(D, FoundRecord))
continue;
+ if (IsFriendTemplate)
+continue;
+
AdoptDecl = FoundRecord;
continue;
} else if (!SearchName) {
@@ -2348,7 +2361,7 @@ Decl *ASTNodeImporter::VisitRecordDecl(R
if (!ToDescribed)
return nullptr;
D2CXX->setDescribedClassTemplate(ToDescribed);
-if (!DCXX->isInjectedClassName()) {
+if (!DCXX->isInjectedClassName() && !IsFriendTemplate) {
// In a record describing a template the type should be an
// InjectedClassNameType (see Sema::CheckClassTemplate). Update the
// previously set type to the correct value here (ToDescribed is not
@@ -4371,12 +4384,14 @@ static ClassTemplateDecl *getDefinition(
}
Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
+ bool IsFriend = D->getFriendObjectKind() != Decl::FOK_None;
+
// If this record has a definition in the translation unit we're coming from,
// but this particular declaration is not that definition, import the
// definition and map to that.
auto *Definition =
cast_or_null(D->getTemplatedDecl()->getDefinition());
- if (Definition && Definition != D->getTemplatedDecl()) {
+ if (Definition && Definition != D->getTemplatedDecl() && !IsFriend) {
Decl *ImportedDef
= Importer.Import(Definition->getDescribedClassTemplate());
if (!ImportedDef)
@@ -4413,17 +4428,20 @@ Decl *ASTNodeImporter::VisitClassTemplat
// definition. So, try to get the definition if that is available in
// the redecl chain.
ClassTemplateDecl *TemplateWithDef = getDefinition(FoundTemplate);
- if (!TemplateWithDef)
+ if (TemplateWithDef)
+FoundTemplate = TemplateWithDef;
+ else
continue;
- FoundTemplate = TemplateWithDef; // Continue with the definition.
}
if (IsStructuralMatch(D, FoundTemplate)) {
- // The class templates structurally match; call it the same template.
+ if (!IsFriend) {
+Importer.MapImported(D->getTemplatedDecl(),
+ FoundTemplate->getTemplatedDecl());
+return Importer.MapImported(D, FoundTemplate);
+ }
- Importer.MapImported(D->getTemplatedDe
[PATCH] D50516: [ASTImporter] Improved import of friend templates.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339560: [ASTImporter] Improved import of friend templates.
(authored by balazske, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50516
Files:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2164,11 +2164,21 @@
}
Decl *ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
+ bool IsFriendTemplate = false;
+ if (auto *DCXX = dyn_cast(D)) {
+IsFriendTemplate =
+DCXX->getDescribedClassTemplate() &&
+DCXX->getDescribedClassTemplate()->getFriendObjectKind() !=
+Decl::FOK_None;
+ }
+
// If this record has a definition in the translation unit we're coming from,
// but this particular declaration is not that definition, import the
// definition and map to that.
TagDecl *Definition = D->getDefinition();
if (Definition && Definition != D &&
+ // Friend template declaration must be imported on its own.
+ !IsFriendTemplate &&
// In contrast to a normal CXXRecordDecl, the implicit
// CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
// The definition of the implicit CXXRecordDecl in this case is the
@@ -2241,7 +2251,7 @@
PrevDecl = FoundRecord;
if (RecordDecl *FoundDef = FoundRecord->getDefinition()) {
- if ((SearchName && !D->isCompleteDefinition())
+ if ((SearchName && !D->isCompleteDefinition() && !IsFriendTemplate)
|| (D->isCompleteDefinition() &&
D->isAnonymousStructOrUnion()
== FoundDef->isAnonymousStructOrUnion() &&
@@ -2281,6 +2291,9 @@
!IsStructuralMatch(D, FoundRecord))
continue;
+ if (IsFriendTemplate)
+continue;
+
AdoptDecl = FoundRecord;
continue;
} else if (!SearchName) {
@@ -2348,7 +2361,7 @@
if (!ToDescribed)
return nullptr;
D2CXX->setDescribedClassTemplate(ToDescribed);
-if (!DCXX->isInjectedClassName()) {
+if (!DCXX->isInjectedClassName() && !IsFriendTemplate) {
// In a record describing a template the type should be an
// InjectedClassNameType (see Sema::CheckClassTemplate). Update the
// previously set type to the correct value here (ToDescribed is not
@@ -4371,12 +4384,14 @@
}
Decl *ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
+ bool IsFriend = D->getFriendObjectKind() != Decl::FOK_None;
+
// If this record has a definition in the translation unit we're coming from,
// but this particular declaration is not that definition, import the
// definition and map to that.
auto *Definition =
cast_or_null(D->getTemplatedDecl()->getDefinition());
- if (Definition && Definition != D->getTemplatedDecl()) {
+ if (Definition && Definition != D->getTemplatedDecl() && !IsFriend) {
Decl *ImportedDef
= Importer.Import(Definition->getDescribedClassTemplate());
if (!ImportedDef)
@@ -4413,17 +4428,20 @@
// definition. So, try to get the definition if that is available in
// the redecl chain.
ClassTemplateDecl *TemplateWithDef = getDefinition(FoundTemplate);
- if (!TemplateWithDef)
+ if (TemplateWithDef)
+FoundTemplate = TemplateWithDef;
+ else
continue;
- FoundTemplate = TemplateWithDef; // Continue with the definition.
}
if (IsStructuralMatch(D, FoundTemplate)) {
- // The class templates structurally match; call it the same template.
+ if (!IsFriend) {
+Importer.MapImported(D->getTemplatedDecl(),
+ FoundTemplate->getTemplatedDecl());
+return Importer.MapImported(D, FoundTemplate);
+ }
- Importer.MapImported(D->getTemplatedDecl(),
- FoundTemplate->getTemplatedDecl());
- return Importer.MapImported(D, FoundTemplate);
+ continue;
}
}
@@ -4461,9 +4479,17 @@
ToTemplated->setDescribedClassTemplate(D2);
+ if (ToTemplated->getPreviousDecl()) {
+assert(
+ToTemplated->getPreviousDecl()->getDescribedClassTemplate() &&
+"Missing described template");
+D2->setPreviousDecl(
+ToTemplated->getPreviousDecl()->getDescribedClassTemplate());
+ }
D2->setAccess(D->getAccess());
D2->setLexicalDeclContext(LexicalDC);
- LexicalDC->addDeclInternal(D2);
+ if (!IsFriend)
+LexicalDC->addDeclInternal(D2);
if (FromTemplated->isCompleteDefinition() &&
!ToTemplated->isCompleteDefinition()) {
Index: cfe/trunk/unittests/AST/ASTImporterTest.c
[PATCH] D50631: [AST] Stuff more data into FunctionTypeBitfields
riccibruno added inline comments.
Comment at: include/clang/AST/Type.h:1530
+/// The number of types in the exception spec, if any.
+unsigned NumExceptions : 9;
+
erichkeane wrote:
> IMO (and @rsmith should respond here instead), if we were looking to steal
> bits from anywhere, this would be it. Exception-specifications that use
> types ares are deprecated in the language and I'm not sure anyone ever used
> them anyway.
Yes this seems a good idea. We could steal 2 bits (or more) from
here to restore the 14 bits in NumParams. I can't imagine that
someone will miss not being able to specify > 512 types
in an exception specification.
Comment at: lib/AST/Type.cpp:2863
auto *argSlot = reinterpret_cast(this+1);
- for (unsigned i = 0; i != NumParams; ++i) {
+ for (unsigned i = 0, N = getNumParams(); i != N; ++i) {
if (params[i]->isDependentType())
erichkeane wrote:
> I would be unbelievably surprised if this change is worth-while. I can't see
> a situation where this getNumParams call doesn't get inlined.
It was just for consistency with the other changes,
but perhaps it do not belongs to this patch.
Repository:
rC Clang
https://reviews.llvm.org/D50631
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50630: [AST] Update/correct the static_asserts for the bit-fields in Type
riccibruno marked an inline comment as done. riccibruno added inline comments. Comment at: include/clang/AST/Type.h:1639 + +static_assert(sizeof(TypeBitfields) <= 4, + "TypeBitfields is larger than 4 bytes!"); erichkeane wrote: > I don't really see value in ensuring that TypeBitfields is <= 4 bytes, it > seems to me that all we care about is the union size, so just <=8 is fine. I originally did this in the hope to provide more information to someone who trigger one of the various static_assert. But you are completely right that we only care about <= 8 bytes Will update the diff with this change. Repository: rC Clang https://reviews.llvm.org/D50630 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move
mboehme updated this revision to Diff 160347.
mboehme added a comment.
Rebase to head.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D49910
Files:
clang-tidy/bugprone/UseAfterMoveCheck.cpp
docs/clang-tidy/checks/bugprone-use-after-move.rst
test/clang-tidy/bugprone-use-after-move.cpp
Index: test/clang-tidy/bugprone-use-after-move.cpp
===
--- test/clang-tidy/bugprone-use-after-move.cpp
+++ test/clang-tidy/bugprone-use-after-move.cpp
@@ -107,6 +107,15 @@
int i;
};
+template
+class AnnotatedContainer {
+public:
+ AnnotatedContainer();
+
+ void foo() const;
+ [[clang::reinitializes]] void clear();
+};
+
// General tests.
@@ -898,6 +907,32 @@
}
}
+void reinitAnnotation() {
+ {
+AnnotatedContainer obj;
+std::move(obj);
+obj.foo();
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'obj' used after it was
+// CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+ }
+ {
+AnnotatedContainer obj;
+std::move(obj);
+obj.clear();
+obj.foo();
+ }
+ {
+// Calling clear() on a different object to the one that was moved is not
+// considered a reinitialization.
+AnnotatedContainer obj1, obj2;
+std::move(obj1);
+obj2.clear();
+obj1.foo();
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'obj1' used after it was
+// CHECK-MESSAGES: [[@LINE-4]]:5: note: move occurred here
+ }
+}
+
// Tests related to order of evaluation within expressions
Index: docs/clang-tidy/checks/bugprone-use-after-move.rst
===
--- docs/clang-tidy/checks/bugprone-use-after-move.rst
+++ docs/clang-tidy/checks/bugprone-use-after-move.rst
@@ -178,6 +178,9 @@
- ``reset()`` is called on the variable and the variable is of type
``std::unique_ptr``, ``std::shared_ptr`` or ``std::weak_ptr``.
+ - A member function marked with the ``[[clang::reinitializes]]`` attribute is
+called on the variable.
+
If the variable in question is a struct and an individual member variable of
that struct is written to, the check does not consider this to be a
reinitialization -- even if, eventually, all member variables of the struct are
Index: clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -308,6 +308,10 @@
cxxMemberCallExpr(
on(allOf(DeclRefMatcher, StandardSmartPointerTypeMatcher)),
callee(cxxMethodDecl(hasName("reset",
+ // Methods that have the [[clang::reinitializes]] attribute.
+ cxxMemberCallExpr(
+ on(DeclRefMatcher),
+ callee(cxxMethodDecl(hasAttr(clang::attr::Reinitializes,
// Passing variable to a function as a non-const pointer.
callExpr(forEachArgumentWithParam(
unaryOperator(hasOperatorName("&"),
Index: test/clang-tidy/bugprone-use-after-move.cpp
===
--- test/clang-tidy/bugprone-use-after-move.cpp
+++ test/clang-tidy/bugprone-use-after-move.cpp
@@ -107,6 +107,15 @@
int i;
};
+template
+class AnnotatedContainer {
+public:
+ AnnotatedContainer();
+
+ void foo() const;
+ [[clang::reinitializes]] void clear();
+};
+
// General tests.
@@ -898,6 +907,32 @@
}
}
+void reinitAnnotation() {
+ {
+AnnotatedContainer obj;
+std::move(obj);
+obj.foo();
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'obj' used after it was
+// CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+ }
+ {
+AnnotatedContainer obj;
+std::move(obj);
+obj.clear();
+obj.foo();
+ }
+ {
+// Calling clear() on a different object to the one that was moved is not
+// considered a reinitialization.
+AnnotatedContainer obj1, obj2;
+std::move(obj1);
+obj2.clear();
+obj1.foo();
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'obj1' used after it was
+// CHECK-MESSAGES: [[@LINE-4]]:5: note: move occurred here
+ }
+}
+
// Tests related to order of evaluation within expressions
Index: docs/clang-tidy/checks/bugprone-use-after-move.rst
===
--- docs/clang-tidy/checks/bugprone-use-after-move.rst
+++ docs/clang-tidy/checks/bugprone-use-after-move.rst
@@ -178,6 +178,9 @@
- ``reset()`` is called on the variable and the variable is of type
``std::unique_ptr``, ``std::shared_ptr`` or ``std:
[PATCH] D49911: Summary:Add clang::reinitializes attribute
mboehme updated this revision to Diff 160348.
mboehme marked 2 inline comments as done.
mboehme added a comment.
Rebase to head.
Repository:
rC Clang
https://reviews.llvm.org/D49911
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/Sema/SemaDeclAttr.cpp
test/SemaCXX/attr-reinitializes.cpp
Index: test/SemaCXX/attr-reinitializes.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-reinitializes.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+[[clang::reinitializes]] int a; // expected-error {{'reinitializes' attribute
only applies to non-static non-const member functions}}
+
+[[clang::reinitializes]] void f(); // expected-error {{only applies to}}
+
+struct A {
+ [[clang::reinitializes]] void foo();
+ __attribute__((reinitializes)) void gnu_foo();
+ [[clang::reinitializes]] void bar() const; // expected-error {{only applies
to}}
+ [[clang::reinitializes]] static void baz(); // expected-error {{only applies
to}}
+ [[clang::reinitializes]] int a; // expected-error {{only applies to}}
+
+ [[clang::reinitializes("arg")]] void qux(); // expected-error
{{'reinitializes' attribute takes no arguments}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6582,6 +6582,11 @@
case ParsedAttr::AT_XRayLogArgs:
handleXRayLogArgsAttr(S, D, AL);
break;
+
+ // Move semantics attribute.
+ case ParsedAttr::AT_Reinitializes:
+handleSimpleAttribute(S, D, AL);
+break;
}
}
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3458,3 +3458,31 @@
corresponding line within the inlined callee.
}];
}
+
+def ReinitializesDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+The ``reinitializes`` attribute can be applied to a non-static, non-const C++
+member function to indicate that this member function reinitializes the entire
+object to a known state, independent of the previous state of the object.
+
+This attribute can be interpreted by static analyzers that warn about uses of
an
+object that has been left in an indeterminate state by a move operation. If a
+member function marked with the ``reinitializes`` attribute is called on a
+moved-from object, the analyzer can conclude that the object is no longer in an
+indeterminate state.
+
+A typical example where this attribute would be used is on functions that clear
+a container class:
+
+.. code-block:: c++
+
+ template
+ class Container {
+ public:
+...
+[[clang::reinitializes]] void Clear();
+...
+ };
+ }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -90,6 +90,11 @@
[{!S->isBitField()}],
"non-bit-field non-static data members">;
+def NonStaticNonConstCXXMethod
+: SubsetSubjectisStatic() && !S->isConst()}],
+"non-static non-const member functions">;
+
def ObjCInstanceMethod : SubsetSubjectisInstanceMethod()}],
"Objective-C instance methods">;
@@ -2974,3 +2979,9 @@
let Subjects = SubjectList<[Var, Function, CXXRecord]>;
let Documentation = [InternalLinkageDocs];
}
+
+def Reinitializes : InheritableAttr {
+ let Spellings = [Clang<"reinitializes", 0>];
+ let Subjects = SubjectList<[NonStaticNonConstCXXMethod], ErrorDiag>;
+ let Documentation = [ReinitializesDocs];
+}
Index: test/SemaCXX/attr-reinitializes.cpp
===
--- /dev/null
+++ test/SemaCXX/attr-reinitializes.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+[[clang::reinitializes]] int a; // expected-error {{'reinitializes' attribute only applies to non-static non-const member functions}}
+
+[[clang::reinitializes]] void f(); // expected-error {{only applies to}}
+
+struct A {
+ [[clang::reinitializes]] void foo();
+ __attribute__((reinitializes)) void gnu_foo();
+ [[clang::reinitializes]] void bar() const; // expected-error {{only applies to}}
+ [[clang::reinitializes]] static void baz(); // expected-error {{only applies to}}
+ [[clang::reinitializes]] int a; // expected-error {{only applies to}}
+
+ [[clang::reinitializes("arg")]] void qux(); // expected-error {{'reinitializes' attribute takes no arguments}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6582,6 +6582,11 @@
case ParsedAttr::AT_XRayLogArgs:
handleXRayLogArgsAttr(S, D, AL);
break;
+
+ // Move semantics attribute.
+
[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move
mboehme added a comment. Thank you for the review! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D49910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY
hans added a comment. In https://reviews.llvm.org/D49240#1197052, @ldionne wrote: > In https://reviews.llvm.org/D49240#1196878, @hans wrote: > > > The reason we noticed this was that it caused a *50 GB* size increase of > > the build output on our buildbots, which was enough to cause infrastructure > > problems. > > > > This change was also committed shortly before the 7.0 branch, so it's part > > of the 7.0.0 release candidates. > > > > Should we back it out until a solution is found? > > > The thing is -- there's no solution without changing the guarantees that > libc++ make. Today, libc++ guarantees that you can link TUs that were built > with different versions of libc++ together. If we remove that guarantee, then > we can use linkonce_odr and solve the problem that Chromium is having. > > Is that guarantee something that Chromium is relying upon? I'm not sure (thakis can probably answer better), but isn't it enough to link against some system libraries that might use libc++ for this to be true? The previous solution, using inlining, while not very elegant didn't have this giant binary size problem, so maybe it was better? What should we put in the release notes, that folks should expect significantly larger binaries using the 7.0.0 version? Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r339568 - [OPENMP] Fix emission of the loop doacross constructs.
Author: abataev
Date: Mon Aug 13 07:05:43 2018
New Revision: 339568
URL: http://llvm.org/viewvc/llvm-project?rev=339568&view=rev
Log:
[OPENMP] Fix emission of the loop doacross constructs.
The number of loops associated with the OpenMP loop constructs should
not be considered as the number loops to collapse.
Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/test/OpenMP/ordered_doacross_codegen.c
cfe/trunk/test/OpenMP/ordered_doacross_codegen.cpp
cfe/trunk/test/OpenMP/parallel_for_simd_ast_print.cpp
Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=339568&r1=339567&r2=339568&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Mon Aug 13 07:05:43 2018
@@ -930,8 +930,11 @@ public:
/// \endcode
/// In this example directive '#pragma omp for' has 'ordered' clause with
/// parameter 2.
-class OMPOrderedClause : public OMPClause {
+class OMPOrderedClause final
+: public OMPClause,
+ private llvm::TrailingObjects {
friend class OMPClauseReader;
+ friend TrailingObjects;
/// Location of '('.
SourceLocation LParenLoc;
@@ -939,6 +942,26 @@ class OMPOrderedClause : public OMPClaus
/// Number of for-loops.
Stmt *NumForLoops = nullptr;
+ /// Real number of loops.
+ unsigned NumberOfLoops = 0;
+
+ /// Build 'ordered' clause.
+ ///
+ /// \param Num Expression, possibly associated with this clause.
+ /// \param NumLoops Number of loops, associated with this clause.
+ /// \param StartLoc Starting location of the clause.
+ /// \param LParenLoc Location of '('.
+ /// \param EndLoc Ending location of the clause.
+ OMPOrderedClause(Expr *Num, unsigned NumLoops, SourceLocation StartLoc,
+ SourceLocation LParenLoc, SourceLocation EndLoc)
+ : OMPClause(OMPC_ordered, StartLoc, EndLoc), LParenLoc(LParenLoc),
+NumForLoops(Num), NumberOfLoops(NumLoops) {}
+
+ /// Build an empty clause.
+ explicit OMPOrderedClause(unsigned NumLoops)
+ : OMPClause(OMPC_ordered, SourceLocation(), SourceLocation()),
+NumberOfLoops(NumLoops) {}
+
/// Set the number of associated for-loops.
void setNumForLoops(Expr *Num) { NumForLoops = Num; }
@@ -946,17 +969,17 @@ public:
/// Build 'ordered' clause.
///
/// \param Num Expression, possibly associated with this clause.
+ /// \param NumLoops Number of loops, associated with this clause.
/// \param StartLoc Starting location of the clause.
/// \param LParenLoc Location of '('.
/// \param EndLoc Ending location of the clause.
- OMPOrderedClause(Expr *Num, SourceLocation StartLoc,
-SourceLocation LParenLoc, SourceLocation EndLoc)
- : OMPClause(OMPC_ordered, StartLoc, EndLoc), LParenLoc(LParenLoc),
-NumForLoops(Num) {}
+ static OMPOrderedClause *Create(const ASTContext &C, Expr *Num,
+ unsigned NumLoops, SourceLocation StartLoc,
+ SourceLocation LParenLoc,
+ SourceLocation EndLoc);
/// Build an empty clause.
- explicit OMPOrderedClause()
- : OMPClause(OMPC_ordered, SourceLocation(), SourceLocation()) {}
+ static OMPOrderedClause* CreateEmpty(const ASTContext &C, unsigned NumLoops);
/// Sets the location of '('.
void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
@@ -967,6 +990,17 @@ public:
/// Return the number of associated for-loops.
Expr *getNumForLoops() const { return cast_or_null(NumForLoops); }
+ /// Set number of iterations for the specified loop.
+ void setLoopNumIterations(unsigned NumLoop, Expr *NumIterations);
+ /// Get number of iterations for all the loops.
+ ArrayRef getLoopNumIterations() const;
+
+ /// Set loop counter for the specified loop.
+ void setLoopCounter(unsigned NumLoop, Expr *Counter);
+ /// Get loops counter for the specified loop.
+ Expr *getLoopCunter(unsigned NumLoop);
+ const Expr *getLoopCunter(unsigned NumLoop) const;
+
child_range children() { return child_range(&NumForLoops, &NumForLoops + 1);
}
static bool classof(const OMPClause *T) {
@@ -3095,24 +3129,32 @@ class OMPDependClause final
/// Colon location.
SourceLocation ColonLoc;
+ /// Number of loops, associated with the depend clause.
+ unsigned NumLoops = 0;
+
/// Build clause with number of variables \a N.
///
/// \param StartLoc Starting location of the clause.
/// \param LParenLoc Location of '('.
/// \param EndLoc Ending location
[PATCH] D50635: Fix lint tests for D50449
kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, arphaman, jkorous, ioeric.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50635
Files:
test/clangd/completion-snippets.test
test/clangd/completion.test
Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -18,6 +18,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " a",
# CHECK-NEXT: "sortText": "{{.*}}a"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "a",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
@@ -38,6 +51,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " b",
# CHECK-NEXT: "sortText": "{{.*}}b"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "b",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
Index: test/clangd/completion-snippets.test
===
--- test/clangd/completion-snippets.test
+++ test/clangd/completion-snippets.test
@@ -34,6 +34,19 @@
# CHECK-NEXT: "kind": 3,
# CHECK-NEXT: "label": " func_with_args(int a, int b)",
# CHECK-NEXT: "sortText": "{{.*}}func_with_args"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "func_with_args(${1:int a}, ${2:int b})",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 0,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT:]
# CHECK-NEXT: }
Index: test/clangd/completion.test
===
--- test/clangd/completion.test
+++ test/clangd/completion.test
@@ -18,6 +18,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " a",
# CHECK-NEXT: "sortText": "{{.*}}a"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "a",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
@@ -38,6 +51,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " b",
# CHECK-NEXT: "sortText": "{{.*}}b"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "b",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
Index: test/clangd/completion-snippets.test
===
--- test/clangd/completion-snippets.test
+++ test/clangd/completion-snippets.test
@@ -34,6 +34,19 @@
# CHECK-NEXT: "kind": 3,
# CHECK-NEXT: "label": " func_with_args(int a, int b)",
# CHECK-NEXT: "sortText": "{{.*}}func_with_args"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "func_with_args(${1:int a}, ${2:int b})",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 0,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT:]
# CHECK-NEXT: }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r339569 - Summary:Add clang::reinitializes attribute
Author: mboehme
Date: Mon Aug 13 07:11:03 2018
New Revision: 339569
URL: http://llvm.org/viewvc/llvm-project?rev=339569&view=rev
Log:
Summary:Add clang::reinitializes attribute
Summary:
This is for use by clang-tidy's bugprone-use-after-move check -- see
corresponding clang-tidy patch at https://reviews.llvm.org/D49910.
Reviewers: aaron.ballman, rsmith
Reviewed By: aaron.ballman
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D49911
Added:
cfe/trunk/test/SemaCXX/attr-reinitializes.cpp
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=339569&r1=339568&r2=339569&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Mon Aug 13 07:11:03 2018
@@ -90,6 +90,11 @@ def NonBitField : SubsetSubjectisBitField()}],
"non-bit-field non-static data members">;
+def NonStaticNonConstCXXMethod
+: SubsetSubjectisStatic() && !S->isConst()}],
+"non-static non-const member functions">;
+
def ObjCInstanceMethod : SubsetSubjectisInstanceMethod()}],
"Objective-C instance methods">;
@@ -2974,3 +2979,9 @@ def InternalLinkage : InheritableAttr {
let Subjects = SubjectList<[Var, Function, CXXRecord]>;
let Documentation = [InternalLinkageDocs];
}
+
+def Reinitializes : InheritableAttr {
+ let Spellings = [Clang<"reinitializes", 0>];
+ let Subjects = SubjectList<[NonStaticNonConstCXXMethod], ErrorDiag>;
+ let Documentation = [ReinitializesDocs];
+}
Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=339569&r1=339568&r2=339569&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon Aug 13 07:11:03 2018
@@ -3458,3 +3458,31 @@ the resulting instructions with the call
corresponding line within the inlined callee.
}];
}
+
+def ReinitializesDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+The ``reinitializes`` attribute can be applied to a non-static, non-const C++
+member function to indicate that this member function reinitializes the entire
+object to a known state, independent of the previous state of the object.
+
+This attribute can be interpreted by static analyzers that warn about uses of
an
+object that has been left in an indeterminate state by a move operation. If a
+member function marked with the ``reinitializes`` attribute is called on a
+moved-from object, the analyzer can conclude that the object is no longer in an
+indeterminate state.
+
+A typical example where this attribute would be used is on functions that clear
+a container class:
+
+.. code-block:: c++
+
+ template
+ class Container {
+ public:
+...
+[[clang::reinitializes]] void Clear();
+...
+ };
+ }];
+}
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=339569&r1=339568&r2=339569&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Aug 13 07:11:03 2018
@@ -6582,6 +6582,11 @@ static void ProcessDeclAttribute(Sema &S
case ParsedAttr::AT_XRayLogArgs:
handleXRayLogArgsAttr(S, D, AL);
break;
+
+ // Move semantics attribute.
+ case ParsedAttr::AT_Reinitializes:
+handleSimpleAttribute(S, D, AL);
+break;
}
}
Added: cfe/trunk/test/SemaCXX/attr-reinitializes.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-reinitializes.cpp?rev=339569&view=auto
==
--- cfe/trunk/test/SemaCXX/attr-reinitializes.cpp (added)
+++ cfe/trunk/test/SemaCXX/attr-reinitializes.cpp Mon Aug 13 07:11:03 2018
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+[[clang::reinitializes]] int a; // expected-error {{'reinitializes' attribute
only applies to non-static non-const member functions}}
+
+[[clang::reinitializes]] void f(); // expected-error {{only applies to}}
+
+struct A {
+ [[clang::reinitializes]] void foo();
+ __attribute__((reinitializes)) void gnu_foo();
+ [[clang::reinitializes]] void bar() const; // expected-error {{only applies
to}}
+ [[clang::reinitializes]] static void baz(); // expected-error {{only applies
to}}
+ [[clang::reinitializes]] int a; // expected-error {{only applies to}}
+
+ [[clang::reinitializes("arg")]] void qux(); // expected-error
{{'reinitia
[PATCH] D49911: Summary:Add clang::reinitializes attribute
This revision was automatically updated to reflect the committed changes.
Closed by commit rC339569: Summary:Add clang::reinitializes attribute (authored
by mboehme, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D49911?vs=160348&id=160351#toc
Repository:
rC Clang
https://reviews.llvm.org/D49911
Files:
include/clang/Basic/Attr.td
include/clang/Basic/AttrDocs.td
lib/Sema/SemaDeclAttr.cpp
test/SemaCXX/attr-reinitializes.cpp
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -90,6 +90,11 @@
[{!S->isBitField()}],
"non-bit-field non-static data members">;
+def NonStaticNonConstCXXMethod
+: SubsetSubjectisStatic() && !S->isConst()}],
+"non-static non-const member functions">;
+
def ObjCInstanceMethod : SubsetSubjectisInstanceMethod()}],
"Objective-C instance methods">;
@@ -2974,3 +2979,9 @@
let Subjects = SubjectList<[Var, Function, CXXRecord]>;
let Documentation = [InternalLinkageDocs];
}
+
+def Reinitializes : InheritableAttr {
+ let Spellings = [Clang<"reinitializes", 0>];
+ let Subjects = SubjectList<[NonStaticNonConstCXXMethod], ErrorDiag>;
+ let Documentation = [ReinitializesDocs];
+}
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -3458,3 +3458,31 @@
corresponding line within the inlined callee.
}];
}
+
+def ReinitializesDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+The ``reinitializes`` attribute can be applied to a non-static, non-const C++
+member function to indicate that this member function reinitializes the entire
+object to a known state, independent of the previous state of the object.
+
+This attribute can be interpreted by static analyzers that warn about uses of
an
+object that has been left in an indeterminate state by a move operation. If a
+member function marked with the ``reinitializes`` attribute is called on a
+moved-from object, the analyzer can conclude that the object is no longer in an
+indeterminate state.
+
+A typical example where this attribute would be used is on functions that clear
+a container class:
+
+.. code-block:: c++
+
+ template
+ class Container {
+ public:
+...
+[[clang::reinitializes]] void Clear();
+...
+ };
+ }];
+}
Index: test/SemaCXX/attr-reinitializes.cpp
===
--- test/SemaCXX/attr-reinitializes.cpp
+++ test/SemaCXX/attr-reinitializes.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+[[clang::reinitializes]] int a; // expected-error {{'reinitializes' attribute
only applies to non-static non-const member functions}}
+
+[[clang::reinitializes]] void f(); // expected-error {{only applies to}}
+
+struct A {
+ [[clang::reinitializes]] void foo();
+ __attribute__((reinitializes)) void gnu_foo();
+ [[clang::reinitializes]] void bar() const; // expected-error {{only applies
to}}
+ [[clang::reinitializes]] static void baz(); // expected-error {{only applies
to}}
+ [[clang::reinitializes]] int a; // expected-error {{only applies to}}
+
+ [[clang::reinitializes("arg")]] void qux(); // expected-error
{{'reinitializes' attribute takes no arguments}}
+};
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6582,6 +6582,11 @@
case ParsedAttr::AT_XRayLogArgs:
handleXRayLogArgsAttr(S, D, AL);
break;
+
+ // Move semantics attribute.
+ case ParsedAttr::AT_Reinitializes:
+handleSimpleAttribute(S, D, AL);
+break;
}
}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -90,6 +90,11 @@
[{!S->isBitField()}],
"non-bit-field non-static data members">;
+def NonStaticNonConstCXXMethod
+: SubsetSubjectisStatic() && !S->isConst()}],
+"non-static non-const member functions">;
+
def ObjCInstanceMethod : SubsetSubjectisInstanceMethod()}],
"Objective-C instance methods">;
@@ -2974,3 +2979,9 @@
let Subjects = SubjectList<[Var, Function, CXXRecord]>;
let Documentation = [InternalLinkageDocs];
}
+
+def Reinitializes : InheritableAttr {
+ let Spellings = [Clang<"reinitializes", 0>];
+ let Subjects = SubjectList<[NonStaticNonConstCXXMethod], ErrorDiag>;
+ let Documentation = [ReinitializesDocs];
+}
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang
[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY
ldionne added a comment. In https://reviews.llvm.org/D49240#1197149, @hans wrote: > In https://reviews.llvm.org/D49240#1197052, @ldionne wrote: > > > In https://reviews.llvm.org/D49240#1196878, @hans wrote: > > > > > The reason we noticed this was that it caused a *50 GB* size increase of > > > the build output on our buildbots, which was enough to cause > > > infrastructure problems. > > > > > > This change was also committed shortly before the 7.0 branch, so it's > > > part of the 7.0.0 release candidates. > > > > > > Should we back it out until a solution is found? > > > > > > The thing is -- there's no solution without changing the guarantees that > > libc++ make. Today, libc++ guarantees that you can link TUs that were built > > with different versions of libc++ together. If we remove that guarantee, > > then we can use linkonce_odr and solve the problem that Chromium is having. > > > > Is that guarantee something that Chromium is relying upon? > > > I'm not sure (thakis can probably answer better), but isn't it enough to link > against some system libraries that might use libc++ for this to be true? One would have to link statically against a system library that was statically linked against libc++ -- I don't think this happens, at least not on Darwin AFAICT. As soon as we cross a dylib boundary, we're safe because those symbols hidden from the ABI are not exported from the dylib. The only problem is when distributing static archives using different versions of libc++, where we don't want presumably ODR-equivalent symbols to be deduplicated (because they might not actually be ODR-equivalent). > The previous solution, using inlining, while not very elegant didn't have > this giant binary size problem, so maybe it was better? Just to be clear, it's only about the number of symbols, not actual code size. > What should we put in the release notes, that folks should expect > significantly larger binaries using the 7.0.0 version? If the current state is not acceptable, we should roll back the change and only put it in once we _also_ know how to allow ODR-deduplicating across TUs. We don't have a solution for that right now -- this was a follow-up step that was planned in the future since this one was semantics-changing. If the current state is acceptable, we can put in the release notes that significant symbol size increases should be expected using LLVM 7.0.0. Repository: rCXX libc++ https://reviews.llvm.org/D49240 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r339571 - [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move
Author: mboehme
Date: Mon Aug 13 07:24:52 2018
New Revision: 339571
URL: http://llvm.org/viewvc/llvm-project?rev=339571&view=rev
Log:
[clang-tidy] Recognize [[clang::reinitializes]] attribute in
bugprone-use-after-move
Summary:
This allows member functions to be marked as reinitializing the object. After a
moved-from object has been reinitialized, the check will no longer consider it
to be in an indeterminate state.
The patch that adds the attribute itself is at https://reviews.llvm.org/D49911
Reviewers: ilya-biryukov, aaron.ballman, alexfh, hokein, rsmith
Reviewed By: aaron.ballman
Subscribers: dblaikie, xazax.hun, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D49910
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp?rev=339571&r1=339570&r2=339571&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp Mon Aug
13 07:24:52 2018
@@ -308,6 +308,10 @@ void UseAfterMoveFinder::getReinits(
cxxMemberCallExpr(
on(allOf(DeclRefMatcher, StandardSmartPointerTypeMatcher)),
callee(cxxMethodDecl(hasName("reset",
+ // Methods that have the [[clang::reinitializes]] attribute.
+ cxxMemberCallExpr(
+ on(DeclRefMatcher),
+ callee(cxxMethodDecl(hasAttr(clang::attr::Reinitializes,
// Passing variable to a function as a non-const pointer.
callExpr(forEachArgumentWithParam(
unaryOperator(hasOperatorName("&"),
Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst?rev=339571&r1=339570&r2=339571&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
(original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
Mon Aug 13 07:24:52 2018
@@ -178,6 +178,9 @@ The check considers a variable to be rei
- ``reset()`` is called on the variable and the variable is of type
``std::unique_ptr``, ``std::shared_ptr`` or ``std::weak_ptr``.
+ - A member function marked with the ``[[clang::reinitializes]]`` attribute is
+called on the variable.
+
If the variable in question is a struct and an individual member variable of
that struct is written to, the check does not consider this to be a
reinitialization -- even if, eventually, all member variables of the struct are
Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp?rev=339571&r1=339570&r2=339571&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp Mon Aug
13 07:24:52 2018
@@ -107,6 +107,15 @@ public:
int i;
};
+template
+class AnnotatedContainer {
+public:
+ AnnotatedContainer();
+
+ void foo() const;
+ [[clang::reinitializes]] void clear();
+};
+
// General tests.
@@ -898,6 +907,32 @@ void standardSmartPointerResetIsReinit()
}
}
+void reinitAnnotation() {
+ {
+AnnotatedContainer obj;
+std::move(obj);
+obj.foo();
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'obj' used after it was
+// CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+ }
+ {
+AnnotatedContainer obj;
+std::move(obj);
+obj.clear();
+obj.foo();
+ }
+ {
+// Calling clear() on a different object to the one that was moved is not
+// considered a reinitialization.
+AnnotatedContainer obj1, obj2;
+std::move(obj1);
+obj2.clear();
+obj1.foo();
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'obj1' used after it was
+// CHECK-MESSAGES: [[@LINE-4]]:5: note: move occurred here
+ }
+}
+
// Tests related to order of evaluation within expressions
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
[PATCH] D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339571: [clang-tidy] Recognize [[clang::reinitializes]]
attribute in bugprone-use-after… (authored by mboehme, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D49910
Files:
clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
Index: clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -308,6 +308,10 @@
cxxMemberCallExpr(
on(allOf(DeclRefMatcher, StandardSmartPointerTypeMatcher)),
callee(cxxMethodDecl(hasName("reset",
+ // Methods that have the [[clang::reinitializes]] attribute.
+ cxxMemberCallExpr(
+ on(DeclRefMatcher),
+ callee(cxxMethodDecl(hasAttr(clang::attr::Reinitializes,
// Passing variable to a function as a non-const pointer.
callExpr(forEachArgumentWithParam(
unaryOperator(hasOperatorName("&"),
Index:
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
@@ -178,6 +178,9 @@
- ``reset()`` is called on the variable and the variable is of type
``std::unique_ptr``, ``std::shared_ptr`` or ``std::weak_ptr``.
+ - A member function marked with the ``[[clang::reinitializes]]`` attribute is
+called on the variable.
+
If the variable in question is a struct and an individual member variable of
that struct is written to, the check does not consider this to be a
reinitialization -- even if, eventually, all member variables of the struct are
Index: clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-use-after-move.cpp
@@ -107,6 +107,15 @@
int i;
};
+template
+class AnnotatedContainer {
+public:
+ AnnotatedContainer();
+
+ void foo() const;
+ [[clang::reinitializes]] void clear();
+};
+
// General tests.
@@ -898,6 +907,32 @@
}
}
+void reinitAnnotation() {
+ {
+AnnotatedContainer obj;
+std::move(obj);
+obj.foo();
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'obj' used after it was
+// CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+ }
+ {
+AnnotatedContainer obj;
+std::move(obj);
+obj.clear();
+obj.foo();
+ }
+ {
+// Calling clear() on a different object to the one that was moved is not
+// considered a reinitialization.
+AnnotatedContainer obj1, obj2;
+std::move(obj1);
+obj2.clear();
+obj1.foo();
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: 'obj1' used after it was
+// CHECK-MESSAGES: [[@LINE-4]]:5: note: move occurred here
+ }
+}
+
// Tests related to order of evaluation within expressions
Index: clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -308,6 +308,10 @@
cxxMemberCallExpr(
on(allOf(DeclRefMatcher, StandardSmartPointerTypeMatcher)),
callee(cxxMethodDecl(hasName("reset",
+ // Methods that have the [[clang::reinitializes]] attribute.
+ cxxMemberCallExpr(
+ on(DeclRefMatcher),
+ callee(cxxMethodDecl(hasAttr(clang::attr::Reinitializes,
// Passing variable to a function as a non-const pointer.
callExpr(forEachArgumentWithParam(
unaryOperator(hasOperatorName("&"),
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-use-after-move.rst
@@ -178,6 +178,9 @@
- ``reset()`` is called on the variable and the variable is of type
``std::un
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. kindly ping! Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50635: Fix lint tests for D50449
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D50635 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r339572 - Fix lint tests for D50449
Author: kadircet
Date: Mon Aug 13 07:32:19 2018
New Revision: 339572
URL: http://llvm.org/viewvc/llvm-project?rev=339572&view=rev
Log:
Fix lint tests for D50449
Reviewers: ilya-biryukov, hokein
Reviewed By: hokein
Subscribers: hokein, ioeric, jkorous, arphaman, cfe-commits
Differential Revision: https://reviews.llvm.org/D50635
Modified:
clang-tools-extra/trunk/test/clangd/completion-snippets.test
clang-tools-extra/trunk/test/clangd/completion.test
Modified: clang-tools-extra/trunk/test/clangd/completion-snippets.test
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion-snippets.test?rev=339572&r1=339571&r2=339572&view=diff
==
--- clang-tools-extra/trunk/test/clangd/completion-snippets.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion-snippets.test Mon Aug 13
07:32:19 2018
@@ -34,6 +34,19 @@
# CHECK-NEXT: "kind": 3,
# CHECK-NEXT: "label": " func_with_args(int a, int b)",
# CHECK-NEXT: "sortText": "{{.*}}func_with_args"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "func_with_args(${1:int a}, ${2:int b})",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 0,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT:]
# CHECK-NEXT: }
Modified: clang-tools-extra/trunk/test/clangd/completion.test
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/completion.test?rev=339572&r1=339571&r2=339572&view=diff
==
--- clang-tools-extra/trunk/test/clangd/completion.test (original)
+++ clang-tools-extra/trunk/test/clangd/completion.test Mon Aug 13 07:32:19 2018
@@ -18,6 +18,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " a",
# CHECK-NEXT: "sortText": "{{.*}}a"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "a",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
@@ -38,6 +51,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " b",
# CHECK-NEXT: "sortText": "{{.*}}b"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "b",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r339573 - [clang-format] Fix comment, NFC
Author: krasimir Date: Mon Aug 13 07:32:29 2018 New Revision: 339573 URL: http://llvm.org/viewvc/llvm-project?rev=339573&view=rev Log: [clang-format] Fix comment, NFC Modified: cfe/trunk/unittests/Format/FormatTestTextProto.cpp Modified: cfe/trunk/unittests/Format/FormatTestTextProto.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestTextProto.cpp?rev=339573&r1=339572&r2=339573&view=diff == --- cfe/trunk/unittests/Format/FormatTestTextProto.cpp (original) +++ cfe/trunk/unittests/Format/FormatTestTextProto.cpp Mon Aug 13 07:32:29 2018 @@ -1,4 +1,4 @@ -//===- unittest/Format/FormatTestProto.cpp ===// +//===- unittest/Format/FormatTestTextProto.cpp ===// // // The LLVM Compiler Infrastructure // ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50635: Fix lint tests for D50449
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339572: Fix lint tests for D50449 (authored by kadircet,
committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50635
Files:
clang-tools-extra/trunk/test/clangd/completion-snippets.test
clang-tools-extra/trunk/test/clangd/completion.test
Index: clang-tools-extra/trunk/test/clangd/completion.test
===
--- clang-tools-extra/trunk/test/clangd/completion.test
+++ clang-tools-extra/trunk/test/clangd/completion.test
@@ -18,6 +18,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " a",
# CHECK-NEXT: "sortText": "{{.*}}a"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "a",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
@@ -38,6 +51,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " b",
# CHECK-NEXT: "sortText": "{{.*}}b"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "b",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
Index: clang-tools-extra/trunk/test/clangd/completion-snippets.test
===
--- clang-tools-extra/trunk/test/clangd/completion-snippets.test
+++ clang-tools-extra/trunk/test/clangd/completion-snippets.test
@@ -34,6 +34,19 @@
# CHECK-NEXT: "kind": 3,
# CHECK-NEXT: "label": " func_with_args(int a, int b)",
# CHECK-NEXT: "sortText": "{{.*}}func_with_args"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "func_with_args(${1:int a}, ${2:int b})",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 0,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT:]
# CHECK-NEXT: }
Index: clang-tools-extra/trunk/test/clangd/completion.test
===
--- clang-tools-extra/trunk/test/clangd/completion.test
+++ clang-tools-extra/trunk/test/clangd/completion.test
@@ -18,6 +18,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " a",
# CHECK-NEXT: "sortText": "{{.*}}a"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "a",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
@@ -38,6 +51,19 @@
# CHECK-NEXT: "kind": 5,
# CHECK-NEXT: "label": " b",
# CHECK-NEXT: "sortText": "{{.*}}b"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "b",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character": 4,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT:}
+# CHECK-NEXT: }
# CHECK-NEXT:}
# CHECK-NEXT: ]
---
Index: clang-tools-extra/trunk/test/clangd/completion-snippets.test
===
--- clang-tools-extra/trunk/test/clangd/completion-snippets.test
+++ clang-tools-extra/trunk/test/clangd/completion-snippets.test
@@ -34,6 +34,19 @@
# CHECK-NEXT: "kind": 3,
# CHECK-NEXT: "label": " func_with_args(int a, int b)",
# CHECK-NEXT: "sortText": "{{.*}}func_with_args"
+# CHECK-NEXT: "textEdit": {
+# CHECK-NEXT:"newText": "func_with_args(${1:int a}, ${2:int b})",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT: "end": {
+# CHECK-NEXT:"character": 7,
+# CHECK-NEXT:"line": 2
+# CHECK-NEXT: }
+# CHECK-NEXT: "start": {
+# CHECK-NEXT:"character":
r339574 - Revert "[OPENMP] Fix emission of the loop doacross constructs."
Author: abataev
Date: Mon Aug 13 07:42:18 2018
New Revision: 339574
URL: http://llvm.org/viewvc/llvm-project?rev=339574&view=rev
Log:
Revert "[OPENMP] Fix emission of the loop doacross constructs."
This reverts commit r339568 because of the problems with the buildbots.
Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/test/OpenMP/ordered_doacross_codegen.c
cfe/trunk/test/OpenMP/ordered_doacross_codegen.cpp
cfe/trunk/test/OpenMP/parallel_for_simd_ast_print.cpp
Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=339574&r1=339573&r2=339574&view=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Mon Aug 13 07:42:18 2018
@@ -930,11 +930,8 @@ public:
/// \endcode
/// In this example directive '#pragma omp for' has 'ordered' clause with
/// parameter 2.
-class OMPOrderedClause final
-: public OMPClause,
- private llvm::TrailingObjects {
+class OMPOrderedClause : public OMPClause {
friend class OMPClauseReader;
- friend TrailingObjects;
/// Location of '('.
SourceLocation LParenLoc;
@@ -942,26 +939,6 @@ class OMPOrderedClause final
/// Number of for-loops.
Stmt *NumForLoops = nullptr;
- /// Real number of loops.
- unsigned NumberOfLoops = 0;
-
- /// Build 'ordered' clause.
- ///
- /// \param Num Expression, possibly associated with this clause.
- /// \param NumLoops Number of loops, associated with this clause.
- /// \param StartLoc Starting location of the clause.
- /// \param LParenLoc Location of '('.
- /// \param EndLoc Ending location of the clause.
- OMPOrderedClause(Expr *Num, unsigned NumLoops, SourceLocation StartLoc,
- SourceLocation LParenLoc, SourceLocation EndLoc)
- : OMPClause(OMPC_ordered, StartLoc, EndLoc), LParenLoc(LParenLoc),
-NumForLoops(Num), NumberOfLoops(NumLoops) {}
-
- /// Build an empty clause.
- explicit OMPOrderedClause(unsigned NumLoops)
- : OMPClause(OMPC_ordered, SourceLocation(), SourceLocation()),
-NumberOfLoops(NumLoops) {}
-
/// Set the number of associated for-loops.
void setNumForLoops(Expr *Num) { NumForLoops = Num; }
@@ -969,17 +946,17 @@ public:
/// Build 'ordered' clause.
///
/// \param Num Expression, possibly associated with this clause.
- /// \param NumLoops Number of loops, associated with this clause.
/// \param StartLoc Starting location of the clause.
/// \param LParenLoc Location of '('.
/// \param EndLoc Ending location of the clause.
- static OMPOrderedClause *Create(const ASTContext &C, Expr *Num,
- unsigned NumLoops, SourceLocation StartLoc,
- SourceLocation LParenLoc,
- SourceLocation EndLoc);
+ OMPOrderedClause(Expr *Num, SourceLocation StartLoc,
+SourceLocation LParenLoc, SourceLocation EndLoc)
+ : OMPClause(OMPC_ordered, StartLoc, EndLoc), LParenLoc(LParenLoc),
+NumForLoops(Num) {}
/// Build an empty clause.
- static OMPOrderedClause* CreateEmpty(const ASTContext &C, unsigned NumLoops);
+ explicit OMPOrderedClause()
+ : OMPClause(OMPC_ordered, SourceLocation(), SourceLocation()) {}
/// Sets the location of '('.
void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
@@ -990,17 +967,6 @@ public:
/// Return the number of associated for-loops.
Expr *getNumForLoops() const { return cast_or_null(NumForLoops); }
- /// Set number of iterations for the specified loop.
- void setLoopNumIterations(unsigned NumLoop, Expr *NumIterations);
- /// Get number of iterations for all the loops.
- ArrayRef getLoopNumIterations() const;
-
- /// Set loop counter for the specified loop.
- void setLoopCounter(unsigned NumLoop, Expr *Counter);
- /// Get loops counter for the specified loop.
- Expr *getLoopCunter(unsigned NumLoop);
- const Expr *getLoopCunter(unsigned NumLoop) const;
-
child_range children() { return child_range(&NumForLoops, &NumForLoops + 1);
}
static bool classof(const OMPClause *T) {
@@ -3129,32 +3095,24 @@ class OMPDependClause final
/// Colon location.
SourceLocation ColonLoc;
- /// Number of loops, associated with the depend clause.
- unsigned NumLoops = 0;
-
/// Build clause with number of variables \a N.
///
/// \param StartLoc Starting location of the clause.
/// \param LParenLoc Location of '('.
/// \param EndLoc Ending location of the clause.
/// \param N Number of the variab
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hugoeg marked 8 inline comments as done.
hugoeg added inline comments.
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
hokein wrote:
> nit: please make sure the code follow LLVM code style, even for test code :)
what is this in reference too?
Will the test still work if I wrap the CHECK MESSAGE lines?
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:11
+
+namespace absl {
+std::string StringsFunction (std::string s1){
hokein wrote:
> Since we have multiple abseil checks that might use these fake abseil
> declarations, I'd suggest pull out these to a common header, and include it
> in this test file.
do I just put the header file in test/clang-tidy ?
https://reviews.llvm.org/D50542
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hugoeg updated this revision to Diff 160363.
hugoeg added a comment.
most corrections from comments have been applied
https://reviews.llvm.org/D50542
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoInternalDepsCheck.cpp
clang-tidy/abseil/NoInternalDepsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-internal-deps.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-no-internal-deps.cpp
Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+ return s1;
+}
+class SomeContainer{
+
+};
+
+namespace strings_internal{
+
+ void InternalFunction(){}
+
+ template
+ P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+ struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+ absl::strings_internal::InternalFunction();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+ absl::strings_internal::InternalTemplateFunction ("a");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class foo2{
+ friend struct absl::container_internal::InternalStruct;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl{
+ void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+ }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+ std::string str = absl::StringsFunction("a");
+ absl::SomeContainer b;
+}
+
+namespace absl {
+ SomeContainer b;
+ std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
.. toctree::
abseil-string-find-startswith
+ abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to
+depend upon it beaucse it’s an implementation detail. They cannot friend it,
+include it, you mention it or refer to it in any way. Doing so violates
+Abseil's compatibility guidelines and may result in breakage. See
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+ friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
Improvements to clang-rename
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+ ` check.
+
+ Gives a warning if code using Abseil depends on internal details.
Improvements to clang-tidy
--
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds instances
[PATCH] D50410: Removing -debug-info-macros from option suggestions test
acoomans added a comment. Can anyone confirm the `-debug-info-macro` command line option is unavailable with the PS4 SDK? Repository: rC Clang https://reviews.llvm.org/D50410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50640: Fix for bug 38508 - Don't do PCH processing when only generating preprocessor output
mikerice created this revision.
Herald added a subscriber: cfe-commits.
This clang-cl driver change removes the PCH options when we are only generating
preprocessed output. This is similar to the behavior of Y-.
Repository:
rC Clang
https://reviews.llvm.org/D50640
Files:
lib/Driver/Driver.cpp
test/Driver/cl-pch.cpp
Index: test/Driver/cl-pch.cpp
===
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -345,3 +345,24 @@
// CHECK-NoSourceTP: pchfile.pch
// CHECK-NoSourceTP: -x
// CHECK-NoSourceTP: "c++"
+
+// If only preprocessing, PCH options are ignored.
+// RUN: %clang_cl /P /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-YC-P %s
+// CHECK-YC-P-NOT: -emit-pch
+// CHECK-YC-P-NOT: -include-pch
+
+// RUN: %clang_cl /E /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-YC-E %s
+// CHECK-YC-E-NOT: -emit-pch
+// CHECK-YC-E-NOT: -include-pch
+
+// RUN: %clang_cl /P /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-YU-P %s
+// CHECK-YU-P-NOT: -emit-pch
+// CHECK-YU-P-NOT: -include-pch
+
+// RUN: %clang_cl /E /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-YU-E %s
+// CHECK-YU-E-NOT: -emit-pch
+// CHECK-YU-E-NOT: -include-pch
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2998,9 +2998,10 @@
Args.eraseArg(options::OPT__SLASH_Yc);
YcArg = nullptr;
}
- if (Args.hasArg(options::OPT__SLASH_Y_)) {
-// /Y- disables all pch handling. Rather than check for it everywhere,
-// just remove clang-cl pch-related flags here.
+ if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_))
{
+// If only preprocessing or /Y- is used, all pch handling is disabled.
+// Rather than check for it everywhere, just remove clang-cl pch-related
+// flags here.
Args.eraseArg(options::OPT__SLASH_Fp);
Args.eraseArg(options::OPT__SLASH_Yc);
Args.eraseArg(options::OPT__SLASH_Yu);
Index: test/Driver/cl-pch.cpp
===
--- test/Driver/cl-pch.cpp
+++ test/Driver/cl-pch.cpp
@@ -345,3 +345,24 @@
// CHECK-NoSourceTP: pchfile.pch
// CHECK-NoSourceTP: -x
// CHECK-NoSourceTP: "c++"
+
+// If only preprocessing, PCH options are ignored.
+// RUN: %clang_cl /P /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-YC-P %s
+// CHECK-YC-P-NOT: -emit-pch
+// CHECK-YC-P-NOT: -include-pch
+
+// RUN: %clang_cl /E /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-YC-E %s
+// CHECK-YC-E-NOT: -emit-pch
+// CHECK-YC-E-NOT: -include-pch
+
+// RUN: %clang_cl /P /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-YU-P %s
+// CHECK-YU-P-NOT: -emit-pch
+// CHECK-YU-P-NOT: -include-pch
+
+// RUN: %clang_cl /E /Ycpchfile.h /FIpchfile.h /c -### -- %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK-YU-E %s
+// CHECK-YU-E-NOT: -emit-pch
+// CHECK-YU-E-NOT: -include-pch
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2998,9 +2998,10 @@
Args.eraseArg(options::OPT__SLASH_Yc);
YcArg = nullptr;
}
- if (Args.hasArg(options::OPT__SLASH_Y_)) {
-// /Y- disables all pch handling. Rather than check for it everywhere,
-// just remove clang-cl pch-related flags here.
+ if (FinalPhase == phases::Preprocess || Args.hasArg(options::OPT__SLASH_Y_)) {
+// If only preprocessing or /Y- is used, all pch handling is disabled.
+// Rather than check for it everywhere, just remove clang-cl pch-related
+// flags here.
Args.eraseArg(options::OPT__SLASH_Fp);
Args.eraseArg(options::OPT__SLASH_Yc);
Args.eraseArg(options::OPT__SLASH_Yu);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50630: [AST] Update/correct the static_asserts for the bit-fields in Type
riccibruno updated this revision to Diff 160368.
riccibruno marked an inline comment as done.
Repository:
rC Clang
https://reviews.llvm.org/D50630
Files:
include/clang/AST/Type.h
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1557,8 +1557,6 @@
unsigned IsKindOf : 1;
};
- static_assert(NumTypeBits + 7 + 6 + 1 <= 32, "Does not fit in an unsigned");
-
class ReferenceTypeBitfields {
friend class ReferenceType;
@@ -1637,6 +1635,27 @@
ReferenceTypeBitfields ReferenceTypeBits;
TypeWithKeywordBitfields TypeWithKeywordBits;
VectorTypeBitfields VectorTypeBits;
+
+static_assert(sizeof(TypeBitfields) <= 8,
+ "TypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ArrayTypeBitfields) <= 8,
+ "ArrayTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AttributedTypeBitfields) <= 8,
+ "AttributedTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AutoTypeBitfields) <= 8,
+ "AutoTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(BuiltinTypeBitfields) <= 8,
+ "BuiltinTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(FunctionTypeBitfields) <= 8,
+ "FunctionTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ObjCObjectTypeBitfields) <= 8,
+ "ObjCObjectTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ReferenceTypeBitfields) <= 8,
+ "ReferenceTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(TypeWithKeywordBitfields) <= 8,
+ "TypeWithKeywordBitfields is larger than 8 bytes!");
+static_assert(sizeof(VectorTypeBitfields) <= 8,
+ "VectorTypeBitfields is larger than 8 bytes!");
};
private:
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1557,8 +1557,6 @@
unsigned IsKindOf : 1;
};
- static_assert(NumTypeBits + 7 + 6 + 1 <= 32, "Does not fit in an unsigned");
-
class ReferenceTypeBitfields {
friend class ReferenceType;
@@ -1637,6 +1635,27 @@
ReferenceTypeBitfields ReferenceTypeBits;
TypeWithKeywordBitfields TypeWithKeywordBits;
VectorTypeBitfields VectorTypeBits;
+
+static_assert(sizeof(TypeBitfields) <= 8,
+ "TypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ArrayTypeBitfields) <= 8,
+ "ArrayTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AttributedTypeBitfields) <= 8,
+ "AttributedTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AutoTypeBitfields) <= 8,
+ "AutoTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(BuiltinTypeBitfields) <= 8,
+ "BuiltinTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(FunctionTypeBitfields) <= 8,
+ "FunctionTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ObjCObjectTypeBitfields) <= 8,
+ "ObjCObjectTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ReferenceTypeBitfields) <= 8,
+ "ReferenceTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(TypeWithKeywordBitfields) <= 8,
+ "TypeWithKeywordBitfields is larger than 8 bytes!");
+static_assert(sizeof(VectorTypeBitfields) <= 8,
+ "VectorTypeBitfields is larger than 8 bytes!");
};
private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50641: [clangd][test] Fix exit messages in tests
jkorous created this revision.
jkorous added reviewers: sammccall, ilya-biryukov.
jkorous added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, arphaman, dexonsmith, MaskRay, ioeric.
There's a small typo in tests - causing that we aren't sending exit LSP message
to clangd but crashing it instead with the last message not being valid JSON
and clangd hitting unexpected EOF in runLanguageServerLoop() right after. Seems
like this bug even managed to spawn some offsprings via copy-pasting. I just
found it by accident since I am not closing stdin after the last message when
using XPC adapter and my test was hanging - waiting for clangd to exit.
It's not a big deal but I got two ideas.
1. Maybe we should somehow check stderr from clangd - in this case there was
JSON parse error present but tests didn't notice.
2. When fixing that I got surprised by shutdown-without-exit.test
implementation. It seems more like exit-without-shutdown to me and makes sense
that we still want to exit (with non-success return value) in such case. Was
this the intention?
What do you think?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50641
Files:
clangd/extra-flags.test
clangd/formatting.test
clangd/initialize-params.test
clangd/shutdown-with-exit.test
clangd/shutdown-without-exit.test
clangd/unsupported-method.test
Index: clangd/unsupported-method.test
===
--- clangd/unsupported-method.test
+++ clangd/unsupported-method.test
@@ -13,4 +13,4 @@
---
{"jsonrpc":"2.0","id":2,"method":"shutdown"}
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/shutdown-without-exit.test
===
--- clangd/shutdown-without-exit.test
+++ clangd/shutdown-without-exit.test
@@ -1,2 +1,2 @@
# RUN: not clangd -lit-test < %s
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/shutdown-with-exit.test
===
--- clangd/shutdown-with-exit.test
+++ clangd/shutdown-with-exit.test
@@ -1,4 +1,4 @@
# RUN: clangd -lit-test < %s
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/initialize-params.test
===
--- clangd/initialize-params.test
+++ clangd/initialize-params.test
@@ -46,4 +46,4 @@
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": null
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/formatting.test
===
--- clangd/formatting.test
+++ clangd/formatting.test
@@ -184,4 +184,4 @@
---
{"jsonrpc":"2.0","id":6,"method":"shutdown"}
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/extra-flags.test
===
--- clangd/extra-flags.test
+++ clangd/extra-flags.test
@@ -47,6 +47,6 @@
---
{"jsonrpc":"2.0","id":5,"method":"shutdown"}
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/unsupported-method.test
===
--- clangd/unsupported-method.test
+++ clangd/unsupported-method.test
@@ -13,4 +13,4 @@
---
{"jsonrpc":"2.0","id":2,"method":"shutdown"}
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/shutdown-without-exit.test
===
--- clangd/shutdown-without-exit.test
+++ clangd/shutdown-without-exit.test
@@ -1,2 +1,2 @@
# RUN: not clangd -lit-test < %s
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/shutdown-with-exit.test
===
--- clangd/shutdown-with-exit.test
+++ clangd/shutdown-with-exit.test
@@ -1,4 +1,4 @@
# RUN: clangd -lit-test < %s
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/initialize-params.test
===
--- clangd/initialize-params.test
+++ clangd/initialize-params.test
@@ -46,4 +46,4 @@
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": null
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/formatting.test
===
--- clangd/formatting.test
+++ clangd/formatting.test
@@ -184,4 +184,4 @@
---
{"jsonrpc":"2.0","id":6,"method":"shutdown"}
---
-{"jsonrpc":"2.0":"method":"exit"}
+{"jsonrpc":"2.0","method":"exit"}
Index: clangd/extra-flags.test
===
--- clangd/extra-flags.t
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
JonasToth added inline comments.
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
hugoeg wrote:
> hokein wrote:
> > nit: please make sure the code follow LLVM code style, even for test code :)
> what is this in reference too?
> Will the test still work if I wrap the CHECK MESSAGE lines?
CHECK-MESSAGE can be on one line, even if its longer (that is common in the
clang-tidy tests).
But dont use many empty lines and respect naming conventions and run
clang-format over the code (except there is a valid reason that the formatting
would infer with the tested logic).
Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:11
+
+namespace absl {
+std::string StringsFunction (std::string s1){
hugoeg wrote:
> hokein wrote:
> > Since we have multiple abseil checks that might use these fake abseil
> > declarations, I'd suggest pull out these to a common header, and include it
> > in this test file.
> do I just put the header file in test/clang-tidy ?
yes, there is one other similar case `hicpp-signed-bitwise-standard-types.h`
https://reviews.llvm.org/D50542
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
JonasToth added inline comments.
Comment at: clang-tidy/abseil/NoInternalDepsCheck.h:21
+/// against doing so. This check should not be run on internal Abseil files or
+///Abseil source code.
+///
double blank
Comment at: docs/clang-tidy/checks/abseil-no-internal-deps.rst:17
+
+absl::strings_internal::foo();
+class foo{
That codeblock needs to be indented, see other checks for reference
https://reviews.llvm.org/D50542
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hugoeg updated this revision to Diff 160371.
https://reviews.llvm.org/D50542
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoInternalDepsCheck.cpp
clang-tidy/abseil/NoInternalDepsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-internal-deps.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-no-internal-deps.cpp
Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+ return s1;
+}
+class SomeContainer{
+
+};
+
+namespace strings_internal{
+
+ void InternalFunction(){}
+
+ template
+ P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+ struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+ absl::strings_internal::InternalFunction();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+ absl::strings_internal::InternalTemplateFunction ("a");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class foo2{
+ friend struct absl::container_internal::InternalStruct;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl{
+ void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+ }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+ std::string str = absl::StringsFunction("a");
+ absl::SomeContainer b;
+}
+
+namespace absl {
+ SomeContainer b;
+ std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
.. toctree::
abseil-string-find-startswith
+ abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to
+depend upon it beaucse it’s an implementation detail. They cannot friend it,
+include it, you mention it or refer to it in any way. Doing so violates
+Abseil's compatibility guidelines and may result in breakage. See
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+ friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
Improvements to clang-rename
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+ ` check.
+
+ Gives a warning if code using Abseil depends on internal details.
Improvements to clang-tidy
--
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds instances where the user depends on internal details and warns them
+/// against doi
[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer
JonasToth added a comment.
Please add a test for the same usecase as in Sema.
template
struct SizeIndicator {
constexpr int value = 8;
};
template <>
struct SizeIndicator {
constexpr int value = 4;
};
template
void fooFunction() {
char Characters[SizeIndicator::value];
void ArrayToPointerDecay(Characters);
}
template <>
void fooFunction() {
char Character[SizeIndicator::value];
void ArrayToPointerDecay(Characters);
}
In both cases the mutation must be detected. I assume the function
specialization is diagnosed correctly, because everything is resolved.
Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:454
+
+ AST =
+ tooling::buildASTFromCode("template void f() { T x; x.y.z; }");
Is there already a test for a method from a templated type?
E.g.
```
template
struct Foo {
T x;
Foo() { int local_int; x(local_int); }
};
```
One can not say that `local_int` could be const or not.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50619
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50631: [AST] Stuff more data into FunctionTypeBitfields
riccibruno updated this revision to Diff 160369.
riccibruno marked 5 inline comments as done.
riccibruno edited the summary of this revision.
riccibruno added a comment.
Bumped the number of bits for parameters from 12 to 14,
stealing from NumExceptions. This means that now (unless
limited by something else in clang) the maximum number
of types in a dynamic exception spec is 127, and the
maximum number of function parameters is 16383.
Repository:
rC Clang
https://reviews.llvm.org/D50631
Files:
include/clang/AST/Type.h
lib/AST/Type.cpp
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2844,24 +2844,23 @@
FunctionProtoType::FunctionProtoType(QualType result, ArrayRef params,
QualType canonical,
const ExtProtoInfo &epi)
-: FunctionType(FunctionProto, result, canonical,
- result->isDependentType(),
+: FunctionType(FunctionProto, result, canonical, result->isDependentType(),
result->isInstantiationDependentType(),
result->isVariablyModifiedType(),
- result->containsUnexpandedParameterPack(), epi.ExtInfo),
- NumParams(params.size()),
- NumExceptions(epi.ExceptionSpec.Exceptions.size()),
- ExceptionSpecType(epi.ExceptionSpec.Type),
- HasExtParameterInfos(epi.ExtParameterInfos != nullptr),
- Variadic(epi.Variadic), HasTrailingReturn(epi.HasTrailingReturn) {
- assert(NumParams == params.size() && "function has too many parameters");
-
+ result->containsUnexpandedParameterPack(), epi.ExtInfo) {
FunctionTypeBits.TypeQuals = epi.TypeQuals;
FunctionTypeBits.RefQualifier = epi.RefQualifier;
+ FunctionTypeBits.NumParams = params.size();
+ assert(getNumParams() == params.size() && "function w. too many parameters!");
+ FunctionTypeBits.NumExceptions = epi.ExceptionSpec.Exceptions.size();
+ FunctionTypeBits.ExceptionSpecType = epi.ExceptionSpec.Type;
+ FunctionTypeBits.HasExtParameterInfos = !!epi.ExtParameterInfos;
+ FunctionTypeBits.Variadic = epi.Variadic;
+ FunctionTypeBits.HasTrailingReturn = epi.HasTrailingReturn;
// Fill in the trailing argument array.
auto *argSlot = reinterpret_cast(this+1);
- for (unsigned i = 0; i != NumParams; ++i) {
+ for (unsigned i = 0; i != getNumParams(); ++i) {
if (params[i]->isDependentType())
setDependent();
else if (params[i]->isInstantiationDependentType())
@@ -2875,7 +2874,7 @@
if (getExceptionSpecType() == EST_Dynamic) {
// Fill in the exception array.
-QualType *exnSlot = argSlot + NumParams;
+QualType *exnSlot = argSlot + getNumParams();
unsigned I = 0;
for (QualType ExceptionType : epi.ExceptionSpec.Exceptions) {
// Note that, before C++17, a dependent exception specification does
@@ -2895,7 +2894,7 @@
epi.ExceptionSpec.NoexceptExpr->isValueDependent());
// Store the noexcept expression and context.
-auto **noexSlot = reinterpret_cast(argSlot + NumParams);
+auto **noexSlot = reinterpret_cast(argSlot + getNumParams());
*noexSlot = epi.ExceptionSpec.NoexceptExpr;
if (epi.ExceptionSpec.NoexceptExpr->isValueDependent() ||
@@ -2907,15 +2906,15 @@
} else if (getExceptionSpecType() == EST_Uninstantiated) {
// Store the function decl from which we will resolve our
// exception specification.
-auto **slot = reinterpret_cast(argSlot + NumParams);
+auto **slot = reinterpret_cast(argSlot + getNumParams());
slot[0] = epi.ExceptionSpec.SourceDecl;
slot[1] = epi.ExceptionSpec.SourceTemplate;
// This exception specification doesn't make the type dependent, because
// it's not instantiated as part of instantiating the type.
} else if (getExceptionSpecType() == EST_Unevaluated) {
// Store the function decl from which we will resolve our
// exception specification.
-auto **slot = reinterpret_cast(argSlot + NumParams);
+auto **slot = reinterpret_cast(argSlot + getNumParams());
slot[0] = epi.ExceptionSpec.SourceDecl;
}
@@ -2935,7 +2934,7 @@
if (epi.ExtParameterInfos) {
auto *extParamInfos =
const_cast(getExtParameterInfosBuffer());
-for (unsigned i = 0; i != NumParams; ++i)
+for (unsigned i = 0; i != getNumParams(); ++i)
extParamInfos[i] = epi.ExtParameterInfos[i];
}
}
@@ -2981,7 +2980,7 @@
case EST_Dynamic:
// A dynamic exception specification is throwing unless every exception
// type is an (unexpanded) pack expansion type.
-for (unsigned I = 0, N = NumExceptions; I != N; ++I)
+for (unsigned I = 0; I != getNumExceptions(); ++I)
if (!getExceptionType(I)->getAs())
return CT_Can;
return CT_Dependent;
@@ -3056,7 +3055,8 @@
void FunctionProtoType::Profile(llvm::FoldingSetNodeID &ID,
const ASTContext &Ctx) {
- P
[PATCH] D50619: [clang-tidy] Handle unresolved expressions in ExprMutationAnalyzer
JonasToth added inline comments.
Comment at: unittests/clang-tidy/ExprMutationAnalyzerTest.cpp:454
+
+ AST =
+ tooling::buildASTFromCode("template void f() { T x; x.y.z; }");
JonasToth wrote:
> Is there already a test for a method from a templated type?
>
> E.g.
> ```
> template
> struct Foo {
> T x;
> Foo() { int local_int; x(local_int); }
> };
> ```
> One can not say that `local_int` could be const or not.
`x.method(local_int);` would be interesting, just for security, given that the
first case is an operator overload and the second one a classical method call.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50619
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
jfb added a comment. CC some sanitizer folks. https://reviews.llvm.org/D50549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hugoeg added inline comments. Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:2 +// RUN: %check_clang_tidy %s abseil-no-internal-deps %t + + JonasToth wrote: > hugoeg wrote: > > hokein wrote: > > > nit: please make sure the code follow LLVM code style, even for test code > > > :) > > what is this in reference too? > > Will the test still work if I wrap the CHECK MESSAGE lines? > CHECK-MESSAGE can be on one line, even if its longer (that is common in the > clang-tidy tests). > > But dont use many empty lines and respect naming conventions and run > clang-format over the code (except there is a valid reason that the > formatting would infer with the tested logic). Do my function names have to be verbs, they're not doing anything. I could rename InternalFunction to something like InternallyProcessString annd StringFunction to process String Would this be preferred? https://reviews.llvm.org/D50542 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxxabi] r339580 - [itanium demangler] Add llvm::itaniumFindTypesInMangledName()
Author: epilk
Date: Mon Aug 13 09:37:47 2018
New Revision: 339580
URL: http://llvm.org/viewvc/llvm-project?rev=339580&view=rev
Log:
[itanium demangler] Add llvm::itaniumFindTypesInMangledName()
This function calls a callback whenever a is parsed.
This is necessary to implement FindAlternateFunctionManglings in LLDB, which
uses a similar hack in FastDemangle. Once that function has been updated to use
this version, FastDemangle can finally be removed.
Differential revision: https://reviews.llvm.org/D50586
Modified:
libcxxabi/trunk/src/cxa_demangle.cpp
Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=339580&r1=339579&r2=339580&view=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Mon Aug 13 09:37:47 2018
@@ -1976,6 +1976,9 @@ struct Db {
// conversion operator's type, and are resolved in the enclosing .
PODSmallVector ForwardTemplateRefs;
+ void (*TypeCallback)(void *, const char *) = nullptr;
+ void *TypeCallbackContext = nullptr;
+
bool TryToParseTemplateArgs = true;
bool PermitForwardTemplateReferences = false;
bool ParsingLambdaParams = false;
@@ -3218,6 +3221,9 @@ Node *Db::parseQualifiedType() {
Node *Db::parseType() {
Node *Result = nullptr;
+ if (TypeCallback != nullptr)
+TypeCallback(TypeCallbackContext, First);
+
switch (look()) {
// ::=
case 'r':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339581: [SEMA] add more -Wfloat-conversion to compound
assigment analysis (authored by nickdesaulniers, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D50467
Files:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -10282,33 +10282,6 @@
DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
}
-/// Analyze the given compound assignment for the possible losing of
-/// floating-point precision.
-static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
- assert(isa(E) &&
- "Must be compound assignment operation");
- // Recurse on the LHS and RHS in here
- AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
- AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
-
- // Now check the outermost expression
- const auto *ResultBT = E->getLHS()->getType()->getAs();
- const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
- // If both source and target are floating points.
- if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
-// Builtin FP kinds are ordered by increasing FP rank.
-if (ResultBT->getKind() < RBT->getKind())
- // We don't want to warn for system macro.
- if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
-// warn about dropping FP rank.
-DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(),
-E->getOperatorLoc(),
-diag::warn_impcast_float_result_precision);
-}
-
/// Diagnose an implicit cast from a floating point value to an integer value.
static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
SourceLocation CContext) {
@@ -10411,6 +10384,39 @@
}
}
+/// Analyze the given compound assignment for the possible losing of
+/// floating-point precision.
+static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
+ assert(isa(E) &&
+ "Must be compound assignment operation");
+ // Recurse on the LHS and RHS in here
+ AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+ AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+
+ // Now check the outermost expression
+ const auto *ResultBT = E->getLHS()->getType()->getAs();
+ const auto *RBT = cast(E)
+->getComputationResultType()
+->getAs();
+
+ // The below checks assume source is floating point.
+ if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+
+ // If source is floating point but target is not.
+ if (!ResultBT->isFloatingPoint())
+return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
+ E->getExprLoc());
+
+ // If both source and target are floating points.
+ // Builtin FP kinds are ordered by increasing FP rank.
+ if (ResultBT->getKind() < RBT->getKind() &&
+ // We don't want to warn for system macro.
+ !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+// warn about dropping FP rank.
+DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(),
+diag::warn_impcast_float_result_precision);
+}
+
static std::string PrettyPrintInRange(const llvm::APSInt &Value,
IntRange Range) {
if (!Range.Width) return "0";
Index: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
===
--- cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
+++ cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
@@ -41,6 +41,32 @@
l = ld; //expected-warning{{conversion}}
}
+void CompoundAssignment() {
+ int x = 3;
+
+ x += 1.234; //expected-warning{{conversion}}
+ x -= -0.0; //expected-warning{{conversion}}
+ x *= 1.1f; //expected-warning{{conversion}}
+ x /= -2.2f; //expected-warning{{conversion}}
+
+ int y = x += 1.4f; //expected-warning{{conversion}}
+
+ float z = 1.1f;
+ double w = -2.2;
+
+ y += z + w; //expected-warning{{conversion}}
+}
+
+# 1 "foo.h" 3
+// ^ the following text comes from a system header file.
+#define SYSTEM_MACRO_FLOAT(x) do { (x) += 1.1; } while(0)
+# 1 "warn-float-conversion.cpp" 1
+// ^ start of a new file.
+void SystemMacro() {
+ float x = 0.0f;
+ SYSTEM_MACRO_FLOAT(x);
+}
+
void Test() {
int a1 = 10.0/2.0; //expected-warning{{conversion}}
int a2 = 1.0/2.0; //expected-warning{{conversion}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.
r339582 - [AST] Update/correct the static_asserts for the bit-fields in Type
Author: brunoricci
Date: Mon Aug 13 09:40:57 2018
New Revision: 339582
URL: http://llvm.org/viewvc/llvm-project?rev=339582&view=rev
Log:
[AST] Update/correct the static_asserts for the bit-fields in Type
The current static_assert only checks that ObjCObjectTypeBitfields
fits into an unsigned. However it turns out that FunctionTypeBitfields
do not currently fits into an unsigned. Therefore the anonymous
union containing the bit-fields always use 8 bytes instead of 4.
This patch removes the lone misguided static_assert and systematically
checks the size of each bit-field.
Reviewed By: erichkeane
Differential Revision: https://reviews.llvm.org/D50630
Modified:
cfe/trunk/include/clang/AST/Type.h
Modified: cfe/trunk/include/clang/AST/Type.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=339582&r1=339581&r2=339582&view=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Mon Aug 13 09:40:57 2018
@@ -1539,8 +1539,6 @@ protected:
unsigned IsKindOf : 1;
};
- static_assert(NumTypeBits + 7 + 6 + 1 <= 32, "Does not fit in an unsigned");
-
class ReferenceTypeBitfields {
friend class ReferenceType;
@@ -1619,6 +1617,27 @@ protected:
ReferenceTypeBitfields ReferenceTypeBits;
TypeWithKeywordBitfields TypeWithKeywordBits;
VectorTypeBitfields VectorTypeBits;
+
+static_assert(sizeof(TypeBitfields) <= 8,
+ "TypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ArrayTypeBitfields) <= 8,
+ "ArrayTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AttributedTypeBitfields) <= 8,
+ "AttributedTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AutoTypeBitfields) <= 8,
+ "AutoTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(BuiltinTypeBitfields) <= 8,
+ "BuiltinTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(FunctionTypeBitfields) <= 8,
+ "FunctionTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ObjCObjectTypeBitfields) <= 8,
+ "ObjCObjectTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ReferenceTypeBitfields) <= 8,
+ "ReferenceTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(TypeWithKeywordBitfields) <= 8,
+ "TypeWithKeywordBitfields is larger than 8 bytes!");
+static_assert(sizeof(VectorTypeBitfields) <= 8,
+ "VectorTypeBitfields is larger than 8 bytes!");
};
private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50630: [AST] Update/correct the static_asserts for the bit-fields in Type
This revision was automatically updated to reflect the committed changes.
Closed by commit rC339582: [AST] Update/correct the static_asserts for the
bit-fields in Type (authored by brunoricci, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D50630
Files:
include/clang/AST/Type.h
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1539,8 +1539,6 @@
unsigned IsKindOf : 1;
};
- static_assert(NumTypeBits + 7 + 6 + 1 <= 32, "Does not fit in an unsigned");
-
class ReferenceTypeBitfields {
friend class ReferenceType;
@@ -1619,6 +1617,27 @@
ReferenceTypeBitfields ReferenceTypeBits;
TypeWithKeywordBitfields TypeWithKeywordBits;
VectorTypeBitfields VectorTypeBits;
+
+static_assert(sizeof(TypeBitfields) <= 8,
+ "TypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ArrayTypeBitfields) <= 8,
+ "ArrayTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AttributedTypeBitfields) <= 8,
+ "AttributedTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AutoTypeBitfields) <= 8,
+ "AutoTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(BuiltinTypeBitfields) <= 8,
+ "BuiltinTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(FunctionTypeBitfields) <= 8,
+ "FunctionTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ObjCObjectTypeBitfields) <= 8,
+ "ObjCObjectTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ReferenceTypeBitfields) <= 8,
+ "ReferenceTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(TypeWithKeywordBitfields) <= 8,
+ "TypeWithKeywordBitfields is larger than 8 bytes!");
+static_assert(sizeof(VectorTypeBitfields) <= 8,
+ "VectorTypeBitfields is larger than 8 bytes!");
};
private:
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1539,8 +1539,6 @@
unsigned IsKindOf : 1;
};
- static_assert(NumTypeBits + 7 + 6 + 1 <= 32, "Does not fit in an unsigned");
-
class ReferenceTypeBitfields {
friend class ReferenceType;
@@ -1619,6 +1617,27 @@
ReferenceTypeBitfields ReferenceTypeBits;
TypeWithKeywordBitfields TypeWithKeywordBits;
VectorTypeBitfields VectorTypeBits;
+
+static_assert(sizeof(TypeBitfields) <= 8,
+ "TypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ArrayTypeBitfields) <= 8,
+ "ArrayTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AttributedTypeBitfields) <= 8,
+ "AttributedTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(AutoTypeBitfields) <= 8,
+ "AutoTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(BuiltinTypeBitfields) <= 8,
+ "BuiltinTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(FunctionTypeBitfields) <= 8,
+ "FunctionTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ObjCObjectTypeBitfields) <= 8,
+ "ObjCObjectTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(ReferenceTypeBitfields) <= 8,
+ "ReferenceTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(TypeWithKeywordBitfields) <= 8,
+ "TypeWithKeywordBitfields is larger than 8 bytes!");
+static_assert(sizeof(VectorTypeBitfields) <= 8,
+ "VectorTypeBitfields is larger than 8 bytes!");
};
private:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis
nickdesaulniers added a comment. Thank you for the code review. Repository: rL LLVM https://reviews.llvm.org/D50467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r339581 - [SEMA] add more -Wfloat-conversion to compound assigment analysis
Author: nickdesaulniers
Date: Mon Aug 13 09:38:07 2018
New Revision: 339581
URL: http://llvm.org/viewvc/llvm-project?rev=339581&view=rev
Log:
[SEMA] add more -Wfloat-conversion to compound assigment analysis
Summary: Fixes Bug: https://bugs.llvm.org/show_bug.cgi?id=27061
Reviewers: aaron.ballman, acoomans
Reviewed By: aaron.ballman, acoomans
Subscribers: acoomans, cfe-commits, srhines, pirama
Differential Revision: https://reviews.llvm.org/D50467
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=339581&r1=339580&r2=339581&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Aug 13 09:38:07 2018
@@ -10282,33 +10282,6 @@ static void DiagnoseImpCast(Sema &S, Exp
DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
}
-/// Analyze the given compound assignment for the possible losing of
-/// floating-point precision.
-static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
- assert(isa(E) &&
- "Must be compound assignment operation");
- // Recurse on the LHS and RHS in here
- AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
- AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
-
- // Now check the outermost expression
- const auto *ResultBT = E->getLHS()->getType()->getAs();
- const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
- // If both source and target are floating points.
- if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
-// Builtin FP kinds are ordered by increasing FP rank.
-if (ResultBT->getKind() < RBT->getKind())
- // We don't want to warn for system macro.
- if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
-// warn about dropping FP rank.
-DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(),
-E->getOperatorLoc(),
-diag::warn_impcast_float_result_precision);
-}
-
/// Diagnose an implicit cast from a floating point value to an integer value.
static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
SourceLocation CContext) {
@@ -10411,6 +10384,39 @@ static void DiagnoseFloatingImpCast(Sema
}
}
+/// Analyze the given compound assignment for the possible losing of
+/// floating-point precision.
+static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
+ assert(isa(E) &&
+ "Must be compound assignment operation");
+ // Recurse on the LHS and RHS in here
+ AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+ AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+
+ // Now check the outermost expression
+ const auto *ResultBT = E->getLHS()->getType()->getAs();
+ const auto *RBT = cast(E)
+->getComputationResultType()
+->getAs();
+
+ // The below checks assume source is floating point.
+ if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+
+ // If source is floating point but target is not.
+ if (!ResultBT->isFloatingPoint())
+return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
+ E->getExprLoc());
+
+ // If both source and target are floating points.
+ // Builtin FP kinds are ordered by increasing FP rank.
+ if (ResultBT->getKind() < RBT->getKind() &&
+ // We don't want to warn for system macro.
+ !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+// warn about dropping FP rank.
+DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(),
E->getOperatorLoc(),
+diag::warn_impcast_float_result_precision);
+}
+
static std::string PrettyPrintInRange(const llvm::APSInt &Value,
IntRange Range) {
if (!Range.Width) return "0";
Modified: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-float-conversion.cpp?rev=339581&r1=339580&r2=339581&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-float-conversion.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-float-conversion.cpp Mon Aug 13 09:38:07 2018
@@ -41,6 +41,32 @@ void Convert(float f, double d, long dou
l = ld; //expected-warning{{conversion}}
}
+void CompoundAssignment() {
+ int x = 3;
+
+ x += 1.234; //expected-warning{{conversion}}
+ x -= -0.0; //expected-warning{{conversion}}
+ x *= 1.1f; //expected-warning{{conversion}}
+ x /= -2.2f; //expected-warning{{conversion}}
+
+ int y = x += 1.4f; //expected-warning{{conversion}}
+
+ float z = 1
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
dvyukov added inline comments.
Comment at:
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
assert(!t0.joinable());
while (!done) {}
assert(G::op_run);
I don't immediately see how the race on n_alive/op_run happens. It seems that
the modifications in the thread happen before this line, and modifications in
main happen after this line. How can both of them modify the variables at the
same time?
https://reviews.llvm.org/D50549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
riccibruno created this revision.
riccibruno added a reviewer: erichkeane.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
Type has enough space for two members of
TemplateSpecializationType. Mechanical patch.
Repository:
rC Clang
https://reviews.llvm.org/D50643
Files:
include/clang/AST/Type.h
lib/AST/Type.cpp
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -3335,8 +3335,10 @@
Canon.isNull()? true : Canon->isDependentType(),
Canon.isNull()? true : Canon->isInstantiationDependentType(),
false,
- T.containsUnexpandedParameterPack()),
-Template(T), NumArgs(Args.size()), TypeAlias(!AliasedType.isNull()) {
+ T.containsUnexpandedParameterPack()), Template(T) {
+ TemplateSpecializationTypeBits.NumArgs = Args.size();
+ TemplateSpecializationTypeBits.TypeAlias = !AliasedType.isNull();
+
assert(!T.getAsDependentTemplateName() &&
"Use DependentTemplateSpecializationType for dependent template-name");
assert((T.getKind() == TemplateName::Template ||
@@ -3365,7 +3367,7 @@
}
// Store the aliased type if this is a type alias template specialization.
- if (TypeAlias) {
+ if (isTypeAlias()) {
auto *Begin = reinterpret_cast(this + 1);
*reinterpret_cast(Begin + getNumArgs()) = AliasedType;
}
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1624,6 +1624,20 @@
unsigned Keyword : 2;
};
+ class TemplateSpecializationTypeBitfields {
+friend class TemplateSpecializationType;
+
+unsigned : NumTypeBits;
+
+/// Whether this template specialization type is a substituted type alias.
+unsigned TypeAlias : 1;
+
+/// The number of template arguments named in this class template
+/// specialization. Intentionally not a bitfield since we have
+/// plenty of space left.
+unsigned NumArgs;
+ };
+
union {
TypeBitfields TypeBits;
ArrayTypeBitfields ArrayTypeBits;
@@ -1635,6 +1649,7 @@
ReferenceTypeBitfields ReferenceTypeBits;
TypeWithKeywordBitfields TypeWithKeywordBits;
VectorTypeBitfields VectorTypeBits;
+TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits;
static_assert(sizeof(TypeBitfields) <= 8,
"TypeBitfields is larger than 8 bytes!");
@@ -1656,6 +1671,9 @@
"TypeWithKeywordBitfields is larger than 8 bytes!");
static_assert(sizeof(VectorTypeBitfields) <= 8,
"VectorTypeBitfields is larger than 8 bytes!");
+static_assert(sizeof(TemplateSpecializationTypeBitfields) <= 8,
+ "TemplateSpecializationTypeBitfields is larger"
+ " than 8 bytes!");
};
private:
@@ -4673,13 +4691,6 @@
/// replacement must, recursively, be one of these).
TemplateName Template;
- /// The number of template arguments named in this class template
- /// specialization.
- unsigned NumArgs : 31;
-
- /// Whether this template specialization type is a substituted type alias.
- unsigned TypeAlias : 1;
-
TemplateSpecializationType(TemplateName T,
ArrayRef Args,
QualType Canon,
@@ -4714,7 +4725,7 @@
/// typedef A type; // not a type alias
/// };
/// \endcode
- bool isTypeAlias() const { return TypeAlias; }
+ bool isTypeAlias() const { return TemplateSpecializationTypeBits.TypeAlias; }
/// Get the aliased type, if this is a specialization of a type alias
/// template.
@@ -4737,14 +4748,16 @@
}
/// Retrieve the number of template arguments.
- unsigned getNumArgs() const { return NumArgs; }
+ unsigned getNumArgs() const {
+return TemplateSpecializationTypeBits.NumArgs;
+ }
/// Retrieve a specific template argument as a type.
/// \pre \c isArgType(Arg)
const TemplateArgument &getArg(unsigned Idx) const; // in TemplateBase.h
ArrayRef template_arguments() const {
-return {getArgs(), NumArgs};
+return {getArgs(), getNumArgs()};
}
bool isSugared() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check
hugoeg updated this revision to Diff 160380.
hugoeg marked 3 inline comments as done.
https://reviews.llvm.org/D50542
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/NoInternalDepsCheck.cpp
clang-tidy/abseil/NoInternalDepsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-no-internal-deps.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-fake-declarations.h
test/clang-tidy/abseil-no-internal-deps.cpp
Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,34 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+#include "abseil-fake-declarations.h"
+
+void foo(){
+ absl::strings_internal::InternalFunction();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+ absl::strings_internal::InternalTemplateFunction ("a");
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class foo2{
+ friend struct absl::container_internal::InternalStruct;
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl{
+ void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+ }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+ std::string Str = absl::StringsFunction("a");
+ absl::SomeContainer b;
+}
+
+namespace absl {
+ SomeContainer b;
+ std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/abseil-fake-declarations.h
===
--- test/clang-tidy/abseil-fake-declarations.h
+++ test/clang-tidy/abseil-fake-declarations.h
@@ -0,0 +1,24 @@
+namespace std {
+struct string {
+ string(const char *);
+ ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+ return s1;
+}
+class SomeContainer{
+
+};
+namespace strings_internal{
+ void InternalFunction(){}
+ template
+ P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+ struct InternalStruct{};
+} // namespace container_internal
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
.. toctree::
abseil-string-find-startswith
+ abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,21 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something
+is in a namespace that includes the word “internal”, code is not allowed to
+depend upon it beaucse it’s an implementation detail. They cannot friend it,
+include it, you mention it or refer to it in any way. Doing so violates
+Abseil's compatibility guidelines and may result in breakage. See
+https://abseil.io/about/compatibility for more information.
+
+The following cases will result in warnings:
+
+.. code-block:: c++
+
+ absl::strings_internal::foo();
+ class foo{
+friend struct absl::container_internal::faa;
+ };
+ absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
Improvements to clang-rename
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+ ` check.
+
+ Gives a warning if code using Abseil depends on internal details.
Improvements to clang-tidy
--
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===---
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
erichkeane added a comment. I think that this causes UB. The Type baseclass will use the TypeBitfields active member, but this uses the TemplateSpecializationTypeBits, right? I know we take advantage of this elsewhere, but I'm tentative about this one... Repository: rC Clang https://reviews.llvm.org/D50643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
BillyONeal added inline comments.
Comment at:
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
assert(!t0.joinable());
while (!done) {}
assert(G::op_run);
dvyukov wrote:
> I don't immediately see how the race on n_alive/op_run happens. It seems that
> the modifications in the thread happen before this line, and modifications in
> main happen after this line. How can both of them modify the variables at the
> same time?
The destruction of g here races with the destruction of the DECAY_COPY'd copy
of G used as the parameter of operator(). That is, line 69 creates a copy of g,
passes that to the started thread, the started thread calls gCopy(). gCopy()
doesn't return until the done flag is set, but the destruction of the object on
which op() is being called is not so synchronized. Most of the other thread
tests avoid this problem by joining with the thread; joining waits for the
destruction of the DECAY_COPY'd parameters, but this does not.
(This is one of the reasons detach() should basically never be called anywhere)
https://reviews.llvm.org/D50549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50645: [clangd] Show non-instantiated decls in signatureHelp
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay.
To avoid producing very verbose output in substitutions involving
typedefs, e.g.
T -> std::vector::iterator
gets turned into an unreadable mess when printed out for libstdc++,
result contains internal types (std::__Vector_iterator<...>) and
expanded well-defined typedefs (std::basic_string).
Until we improve the presentation code in clang, going with
non-instantiated decls looks like a better UX trade-off.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50645
Files:
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1537,6 +1537,59 @@
EXPECT_EQ(0, Results.activeParameter);
}
+TEST(SignatureHelpTest, InstantiatedSignatures) {
+ EXPECT_THAT(signatures(R"cpp(
+template
+void foo(T, T, T);
+
+int main() {
+ foo(^);
+}
+ )cpp")
+ .signatures,
+ ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+ EXPECT_THAT(signatures(R"cpp(
+template
+void foo(T, T, T);
+
+int main() {
+ foo(10, ^);
+})cpp")
+ .signatures,
+ ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+ EXPECT_THAT(signatures(R"cpp(
+template
+void foo(T...);
+
+int main() {
+ foo(^);
+}
+ )cpp")
+ .signatures,
+ ElementsAre(Sig("foo(T...) -> void", {"T..."})));
+
+ // It is debatable whether we should substitute the outer template parameter
+ // ('T') in that case. Currently we don't substitute it in signature help,
but
+ // do substitute in code complete.
+ // FIXME: make code complete and signature help consistent, figure out which
+ // way is better.
+ EXPECT_THAT(signatures(R"cpp(
+template
+struct X {
+ template
+ void foo(T, U);
+};
+
+int main() {
+ X().foo(^)
+}
+ )cpp")
+ .signatures,
+ ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -714,7 +714,15 @@
"too many arguments");
SigHelp.activeParameter = static_cast(CurrentArg);
for (unsigned I = 0; I < NumCandidates; ++I) {
- const auto &Candidate = Candidates[I];
+ OverloadCandidate Candidate = Candidates[I];
+ // We want to avoid showing instantiated signatures, because they may be
+ // long in some cases (e.g. when 'T' is substituted with 'std::string',
we
+ // would get 'std::basic_string').
+ if (auto Func = Candidate.getFunction()) {
+if (auto Pattern = Func->getTemplateInstantiationPattern())
+ Candidate = OverloadCandidate(Pattern);
+ }
+
const auto *CCS = Candidate.CreateSignatureString(
CurrentArg, S, *Allocator, CCTUInfo, true);
assert(CCS && "Expected the CodeCompletionString to be non-null");
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1537,6 +1537,59 @@
EXPECT_EQ(0, Results.activeParameter);
}
+TEST(SignatureHelpTest, InstantiatedSignatures) {
+ EXPECT_THAT(signatures(R"cpp(
+template
+void foo(T, T, T);
+
+int main() {
+ foo(^);
+}
+ )cpp")
+ .signatures,
+ ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+ EXPECT_THAT(signatures(R"cpp(
+template
+void foo(T, T, T);
+
+int main() {
+ foo(10, ^);
+})cpp")
+ .signatures,
+ ElementsAre(Sig("foo(T, T, T) -> void", {"T", "T", "T"})));
+
+ EXPECT_THAT(signatures(R"cpp(
+template
+void foo(T...);
+
+int main() {
+ foo(^);
+}
+ )cpp")
+ .signatures,
+ ElementsAre(Sig("foo(T...) -> void", {"T..."})));
+
+ // It is debatable whether we should substitute the outer template parameter
+ // ('T') in that case. Currently we don't substitute it in signature help, but
+ // do substitute in code complete.
+ // FIXME: make code complete and signature help consistent, figure out which
+ // way is better.
+ EXPECT_THAT(signatures(R"cpp(
+template
+struct X {
+ template
+ void foo(T, U);
+};
+
+int main() {
+ X().foo(^)
+}
+ )cpp")
+ .signatures,
+ ElementsAre(Sig("foo(T, U) -> void", {"T", "U"})));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
BillyONeal added inline comments.
Comment at:
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
assert(!t0.joinable());
while (!done) {}
assert(G::op_run);
BillyONeal wrote:
> dvyukov wrote:
> > I don't immediately see how the race on n_alive/op_run happens. It seems
> > that the modifications in the thread happen before this line, and
> > modifications in main happen after this line. How can both of them modify
> > the variables at the same time?
> The destruction of g here races with the destruction of the DECAY_COPY'd copy
> of G used as the parameter of operator(). That is, line 69 creates a copy of
> g, passes that to the started thread, the started thread calls gCopy().
> gCopy() doesn't return until the done flag is set, but the destruction of the
> object on which op() is being called is not so synchronized. Most of the
> other thread tests avoid this problem by joining with the thread; joining
> waits for the destruction of the DECAY_COPY'd parameters, but this does not.
>
> (This is one of the reasons detach() should basically never be called
> anywhere)
>
(That is to say, there's nothing to prevent both threads from executing G::!G()
on the two different copies of g... making op_run atomic is probably avoidable
but I'm being paranoid given that there was already thread unsafety here...)
https://reviews.llvm.org/D50549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
riccibruno added a comment. All of these bitfields (ab)use are already UB I think... I don't see what is special in this case... Repository: rC Clang https://reviews.llvm.org/D50643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
dvyukov added inline comments.
Comment at:
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
assert(!t0.joinable());
while (!done) {}
assert(G::op_run);
BillyONeal wrote:
> BillyONeal wrote:
> > dvyukov wrote:
> > > I don't immediately see how the race on n_alive/op_run happens. It seems
> > > that the modifications in the thread happen before this line, and
> > > modifications in main happen after this line. How can both of them modify
> > > the variables at the same time?
> > The destruction of g here races with the destruction of the DECAY_COPY'd
> > copy of G used as the parameter of operator(). That is, line 69 creates a
> > copy of g, passes that to the started thread, the started thread calls
> > gCopy(). gCopy() doesn't return until the done flag is set, but the
> > destruction of the object on which op() is being called is not so
> > synchronized. Most of the other thread tests avoid this problem by joining
> > with the thread; joining waits for the destruction of the DECAY_COPY'd
> > parameters, but this does not.
> >
> > (This is one of the reasons detach() should basically never be called
> > anywhere)
> >
> (That is to say, there's nothing to prevent both threads from executing
> G::!G() on the two different copies of g... making op_run atomic is probably
> avoidable but I'm being paranoid given that there was already thread unsafety
> here...)
What is gCopy? I don't see anything named gCopy in this file...
Do we care about completion of destruction? Why? We wait for done to be set,
and other variables are already updated at that point. Why does it matter that
"the destruction of the object on which op() is being called is not so
synchronized."?
https://reviews.llvm.org/D50549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50647: [Sema] fix -Wfloat-conversion test case.
nickdesaulniers created this revision.
nickdesaulniers added reviewers: aaron.ballman, gkistanova.
Herald added a subscriber: cfe-commits.
Fixes commit 6bbde717f7fb ("[SEMA] add more -Wfloat-conversion to
compound assigment analysis").
This test case was caught in postsubmit testing.
Repository:
rC Clang
https://reviews.llvm.org/D50647
Files:
test/Sema/conversion.c
Index: test/Sema/conversion.c
===
--- test/Sema/conversion.c
+++ test/Sema/conversion.c
@@ -359,7 +359,7 @@
void test_7676608(void) {
float q = 0.7f;
char c = 5;
- f7676608(c *= q);
+ f7676608(c *= q); // expected-warning {{conversion}}
}
//
Index: test/Sema/conversion.c
===
--- test/Sema/conversion.c
+++ test/Sema/conversion.c
@@ -359,7 +359,7 @@
void test_7676608(void) {
float q = 0.7f;
char c = 5;
- f7676608(c *= q);
+ f7676608(c *= q); // expected-warning {{conversion}}
}
//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D50647: [Sema] fix -Wfloat-conversion test case.
On Mon, Aug 13, 2018 at 1:08 PM, Nick Desaulniers via Phabricator
wrote:
> nickdesaulniers created this revision.
> nickdesaulniers added reviewers: aaron.ballman, gkistanova.
> Herald added a subscriber: cfe-commits.
>
> Fixes commit 6bbde717f7fb ("[SEMA] add more -Wfloat-conversion to
> compound assigment analysis").
Thank you for fixing this, but in the future, can you please put the
SVN revision number into the commit message rather than a git hash
(for code archaeology purposes)? Not everyone is on git yet. :-)
r339581 is the amended revision in this case.
~Aaron
>
> This test case was caught in postsubmit testing.
>
>
> Repository:
> rC Clang
>
> https://reviews.llvm.org/D50647
>
> Files:
> test/Sema/conversion.c
>
>
> Index: test/Sema/conversion.c
> ===
> --- test/Sema/conversion.c
> +++ test/Sema/conversion.c
> @@ -359,7 +359,7 @@
> void test_7676608(void) {
>float q = 0.7f;
>char c = 5;
> - f7676608(c *= q);
> + f7676608(c *= q); // expected-warning {{conversion}}
> }
>
> //
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50643: [AST] Pack the bits of TemplateSpecializationType into Type
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Ah, right. I missed that the others already do it. Fine I guess... Repository: rC Clang https://reviews.llvm.org/D50643 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
BillyONeal added inline comments.
Comment at:
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
assert(!t0.joinable());
while (!done) {}
assert(G::op_run);
dvyukov wrote:
> BillyONeal wrote:
> > BillyONeal wrote:
> > > dvyukov wrote:
> > > > I don't immediately see how the race on n_alive/op_run happens. It
> > > > seems that the modifications in the thread happen before this line, and
> > > > modifications in main happen after this line. How can both of them
> > > > modify the variables at the same time?
> > > The destruction of g here races with the destruction of the DECAY_COPY'd
> > > copy of G used as the parameter of operator(). That is, line 69 creates a
> > > copy of g, passes that to the started thread, the started thread calls
> > > gCopy(). gCopy() doesn't return until the done flag is set, but the
> > > destruction of the object on which op() is being called is not so
> > > synchronized. Most of the other thread tests avoid this problem by
> > > joining with the thread; joining waits for the destruction of the
> > > DECAY_COPY'd parameters, but this does not.
> > >
> > > (This is one of the reasons detach() should basically never be called
> > > anywhere)
> > >
> > (That is to say, there's nothing to prevent both threads from executing
> > G::!G() on the two different copies of g... making op_run atomic is
> > probably avoidable but I'm being paranoid given that there was already
> > thread unsafety here...)
> What is gCopy? I don't see anything named gCopy in this file...
>
> Do we care about completion of destruction? Why? We wait for done to be set,
> and other variables are already updated at that point. Why does it matter
> that "the destruction of the object on which op() is being called is not so
> synchronized."?
Because the destructor does `--n_alive;`
https://reviews.llvm.org/D50549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
BillyONeal added inline comments.
Comment at:
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
assert(!t0.joinable());
while (!done) {}
assert(G::op_run);
BillyONeal wrote:
> dvyukov wrote:
> > BillyONeal wrote:
> > > BillyONeal wrote:
> > > > dvyukov wrote:
> > > > > I don't immediately see how the race on n_alive/op_run happens. It
> > > > > seems that the modifications in the thread happen before this line,
> > > > > and modifications in main happen after this line. How can both of
> > > > > them modify the variables at the same time?
> > > > The destruction of g here races with the destruction of the
> > > > DECAY_COPY'd copy of G used as the parameter of operator(). That is,
> > > > line 69 creates a copy of g, passes that to the started thread, the
> > > > started thread calls gCopy(). gCopy() doesn't return until the done
> > > > flag is set, but the destruction of the object on which op() is being
> > > > called is not so synchronized. Most of the other thread tests avoid
> > > > this problem by joining with the thread; joining waits for the
> > > > destruction of the DECAY_COPY'd parameters, but this does not.
> > > >
> > > > (This is one of the reasons detach() should basically never be called
> > > > anywhere)
> > > >
> > > (That is to say, there's nothing to prevent both threads from executing
> > > G::!G() on the two different copies of g... making op_run atomic is
> > > probably avoidable but I'm being paranoid given that there was already
> > > thread unsafety here...)
> > What is gCopy? I don't see anything named gCopy in this file...
> >
> > Do we care about completion of destruction? Why? We wait for done to be
> > set, and other variables are already updated at that point. Why does it
> > matter that "the destruction of the object on which op() is being called is
> > not so synchronized."?
> Because the destructor does `--n_alive;`
>What is gCopy? I don't see anything named gCopy in this file...
The copy is made in the constructor of std::thread. std::thread makes a copy of
all the input parameters, gives the copy to the started thread, and then
std::invoke is called there.
>Why does it matter that "the destruction of the object on which op() is being
>called is not so synchronized."?
Because the two dtors race on `--n_alive;` when `n_alive` is not atomic.
https://reviews.llvm.org/D50549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests
dvyukov added inline comments.
Comment at:
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
assert(!t0.joinable());
while (!done) {}
assert(G::op_run);
BillyONeal wrote:
> BillyONeal wrote:
> > dvyukov wrote:
> > > BillyONeal wrote:
> > > > BillyONeal wrote:
> > > > > dvyukov wrote:
> > > > > > I don't immediately see how the race on n_alive/op_run happens. It
> > > > > > seems that the modifications in the thread happen before this line,
> > > > > > and modifications in main happen after this line. How can both of
> > > > > > them modify the variables at the same time?
> > > > > The destruction of g here races with the destruction of the
> > > > > DECAY_COPY'd copy of G used as the parameter of operator(). That is,
> > > > > line 69 creates a copy of g, passes that to the started thread, the
> > > > > started thread calls gCopy(). gCopy() doesn't return until the done
> > > > > flag is set, but the destruction of the object on which op() is being
> > > > > called is not so synchronized. Most of the other thread tests avoid
> > > > > this problem by joining with the thread; joining waits for the
> > > > > destruction of the DECAY_COPY'd parameters, but this does not.
> > > > >
> > > > > (This is one of the reasons detach() should basically never be called
> > > > > anywhere)
> > > > >
> > > > (That is to say, there's nothing to prevent both threads from executing
> > > > G::!G() on the two different copies of g... making op_run atomic is
> > > > probably avoidable but I'm being paranoid given that there was already
> > > > thread unsafety here...)
> > > What is gCopy? I don't see anything named gCopy in this file...
> > >
> > > Do we care about completion of destruction? Why? We wait for done to be
> > > set, and other variables are already updated at that point. Why does it
> > > matter that "the destruction of the object on which op() is being called
> > > is not so synchronized."?
> > Because the destructor does `--n_alive;`
> >What is gCopy? I don't see anything named gCopy in this file...
>
> The copy is made in the constructor of std::thread. std::thread makes a copy
> of all the input parameters, gives the copy to the started thread, and then
> std::invoke is called there.
>
> >Why does it matter that "the destruction of the object on which op() is
> >being called is not so synchronized."?
>
> Because the two dtors race on `--n_alive;` when `n_alive` is not atomic.
But the first dtor runs before "while (!done) {}" line and the second dtor runs
after "while (!done) {}" line, no?
Or there is third object involved? But then I don't see how joining the thread
would help either.
https://reviews.llvm.org/D50549
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
