[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.
CarlosAlbertoEnciso added a comment. Hi, It seems that your patch is causing an error in the tests for http://lab.llvm.org:8011/builders/clang-x64-ninja-win7 http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11510/steps/ninja%20check%201/logs/FAIL%3A%20Extra%20Tools%20Unit%20Tests%3A%3AFileDistanceTests.URI Thanks very much Repository: rL LLVM https://reviews.llvm.org/D48441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48916: Fix setting of empty implicit-section-name attribute for functions affected by '#pragma clang section'
petpav01 created this revision.
petpav01 added reviewers: javed.absar, chill, rnk.
Herald added subscribers: cfe-commits, kristof.beyls.
Code in `CodeGenModule::SetFunctionAttributes()` can set an empty attribute
`implicit-section-name` on a function that is affected by `#pragma clang
text="section"`. This is incorrect because the attribute should contain a valid
section name. If the function additionally also uses
`__attribute__((section("section")))` then this results in emitting the
function in a section with an empty name.
Example:
$ cat repr.c
#pragma clang section text=".text_pragma"
void f(void) __attribute__((section(".text_attr")));
void f(void) {}
$ clang -target arm-none-eabi -march=armv7-a -c repr.c
$ llvm-objdump -h repr.o
repr.o: file format ELF32-arm-little
Sections:
Idx Name Size Address Type
0
1 .strtab 005d
2 .text TEXT
3 0004 TEXT
4 .ARM.exidx0008
5 .rel.ARM.exidx 0008
6 .comment 00b2
7 .note.GNU-stack
8 .ARM.attributes 003a
9 .symtab 0050
Section with index 3 contains the code for function `f()`. The name of the
section should be `.text_attr` but is instead empty.
The following happens in this example:
- `CodeGenModule::EmitGlobalFunctionDefinition()` is called to emit IR for
`f()`. It invokes `GetAddrOfFunction()` -> `GetOrCreateLLVMFunction()` ->
`SetFunctionAttributes()`. The last method notices that the `FunctionDecl` has
the `PragmaClangTextSectionAttr` attribute and wrongly sets empty
`implicit-section-name` attribute on the resulting `llvm::Function`.
- `EmitGlobalFunctionDefinition()` calls `setNonAliasAttributes()` which
normally also sets the `implicit-section-name` attribute but because the `Decl`
has `SectionAttr` it gets skipped.
The patch fixes the issue by removing the problematic code that sets empty
`implicit-section-name` from `CodeGenModule::SetFunctionAttributes()`. The idea
is that it is sufficient to set this attribute only from
`setNonAliasAttributes()` when a function is emitted.
Repository:
rC Clang
https://reviews.llvm.org/D48916
Files:
lib/CodeGen/CodeGenModule.cpp
test/CodeGen/clang-sections-attribute.c
Index: test/CodeGen/clang-sections-attribute.c
===
--- /dev/null
+++ test/CodeGen/clang-sections-attribute.c
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -emit-llvm -triple arm-none-eabi -o - %s | FileCheck %s
+
+// Test interaction between __attribute__((section())) and '#pragma clang
+// section' directives. The attribute should always have higher priority than
+// the pragma.
+
+// Text tests.
+#pragma clang section text=".ext_fun_pragma"
+void ext_fun(void) __attribute__((section(".ext_fun_attr")));
+void ext_fun(void) {}
+#pragma clang section text=""
+
+void ext_fun2(void) __attribute__((section(".ext_fun2_attr")));
+#pragma clang section text=".ext_fun2_pragma"
+void ext_fun2(void) {}
+#pragma clang section text=""
+
+#pragma clang section text=".int_fun_pragma"
+static void int_fun(void) __attribute__((section(".int_fun_attr"), used));
+static void int_fun(void) {}
+#pragma clang section text=""
+
+static void int_fun2(void) __attribute__((section(".int_fun2_attr"), used));
+#pragma clang section text=".int_fun2_pragma"
+static void int_fun2(void) {}
+#pragma clang section text=""
+
+// Rodata tests.
+#pragma clang section rodata=".ext_const_pragma"
+__attribute__((section(".ext_const_attr")))
+const int ext_const = 1;
+#pragma clang section rodata=""
+
+#pragma clang section rodata=".int_const_pragma"
+__attribute__((section(".int_const_attr"), used))
+static const int int_const = 1;
+#pragma clang section rodata=""
+
+// Data tests.
+#pragma clang section data=".ext_var_pragma"
+__attribute__((section(".ext_var_attr")))
+int ext_var = 1;
+#pragma clang section data=""
+
+#pragma clang section data=".int_var_pragma"
+__attribute__((section(".int_var_attr"), used))
+static int int_var = 1;
+#pragma clang section data=""
+
+// Bss tests.
+#pragma clang section bss=".ext_zvar_pragma"
+__attribute__((section(".ext_zvar_attr")))
+int ext_zvar;
+#pragma clang section bss=""
+
+#pragma clang section bss=".int_zvar_pragma"
+__attribute__((section(".int_zvar_attr"), used))
+static int int_zvar;
+#pragma clang section bss=""
+
+// CHECK: @ext_const = constant i32 1, section ".ext_const_attr", align 4{{$}}
+// CHECK: @int_const = internal constant i32 1, section ".int_const_attr",
align 4{{$}}
+// CHECK: @ext_var = global i32 1, section ".ext_var_attr", align 4{{$}}
+// CHECK: @int_var = internal global i32 1, section ".int_var_attr", align
4{{$}}
+// CHECK: @ext_zvar = global i32 0, section ".ext_zvar_attr",
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D48917
Files:
lib/Sema/SemaCodeComplete.cpp
unittests/Sema/CodeCompleteTest.cpp
Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -131,4 +131,15 @@
EXPECT_TRUE(VisitedNS.empty());
}
+TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {
+ auto VisitedNS = runCodeCompleteOnCode(R"cpp(
+namespace n1 {
+namespace n2 {
+ void f(^) {}
+}
+}
+ )cpp");
+ EXPECT_THAT(VisitedNS, UnorderedElementsAre("n1", "n1::n2"));
+}
+
} // namespace
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3741,10 +3741,12 @@
if (CodeCompleter->includeMacros())
AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext(CodeCompletionContext::CCC_Expression,
- Data.PreferredType),
-Results.data(),Results.size());
+ CodeCompletionContext CC(CodeCompletionContext::CCC_Expression,
+ Data.PreferredType);
+ for (auto *Visited : Results.getCompletionContext().getVisitedContexts())
+CC.addVisitedContext(Visited);
+ HandleCodeCompleteResults(this, CodeCompleter, CC, Results.data(),
+Results.size());
}
void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4362,11 @@
}
Results.ExitScope();
- //We need to make sure we're setting the right context,
- //so only say we include macros if the code completer says we do
- enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
if (CodeCompleter->includeMacros()) {
AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
}
-
- HandleCodeCompleteResults(this, CodeCompleter,
-kind,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4769,9 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4790,8 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteNamespaceDecl(Scope *S) {
@@ -4893,10 +4887,9 @@
// Add any type specifiers
AddTypeSpecifierResults(getLangOpts(), Results);
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5170,8 @@
else
AddObjCTopLevelResults(Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static void AddObjCExpressionResults(ResultBuilder &Results, bool NeedAt) {
@@ -5311,9 +5303,8 @@
Results.EnterNewScope();
AddObjCVisibilityResults(getLangOpts(), Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
ebevhan added inline comments.
Comment at: lib/Basic/FixedPoint.cpp:40
+ if (DstWidth > Val.getBitWidth())
+Val = Val.extend(DstWidth);
+ if (Upscaling)
leonardchan wrote:
> ebevhan wrote:
> > It should be possible to replace this with `extOrTrunc` and move it below
> > the saturation.
> I don't think we can move the extension below saturation since saturation
> requires checking the bits above the dst width which may require extending
> and shifting beforehand.
>
> Still leaving it above saturation may require checking for the width over
> using `extOrTrunc` to prevent truncating early before right shifting.
I think the cases where saturation takes effect are limited to simply
truncation, since when we upscale we automatically extend first to avoid
shifting out bits. This means that the only situation where we need to check
for saturation is right before we truncate, and we don't have to worry about
anything happening when we upscale.
Do you have an example of when the extension is needed? (In case it wasn't
clear, I mean the `NewVal = NewVal.extend(DstWidth);` extension, not the
extension that is part of the upscale)
Comment at: lib/Basic/FixedPoint.cpp:58
+ if (!DstSema.isSigned && Val.isNegative()) {
+Val = 0;
+ } else if (!(Masked == Mask || Masked == 0)) {
leonardchan wrote:
> ebevhan wrote:
> > If the value is unsigned and negative, we always zero it?
> >
> > Also, this code always saturates the value regardless of whether the
> > destination semantics call for it, no?
> You're right. I did not test for this since I thought this would fall under
> undefined behavior converting a signed negative number to an unsigned number.
> I'll add a check for saturation since I imagine most people would expect
> modular wrap around, but do you think I should add a test case for this?
Yes, even though it's undefined behavior to wrap, I think it's a good idea to
do as little work as possible for each operation and keep every operation as
simple as possible. This is to make it easier to codegen later, since it will
mean fewer operations, it will be simpler to understand the connection between
the semantics of the operations and the code emission and there will be better
'alignment' between generated code and constant evaluation results.
Obviously, if you invoke undefined behavior in runtime anything can happen, but
I think that the operations should behave "sensibly" in isolation.
Comment at: lib/Basic/FixedPoint.cpp:50
+else
+ NewVal = 0;
+ } else {
If you move the `!DstSema.isSigned() && NewVal.isNegative()` block after the
mask check, you can avoid doing `= 0` twice.
Comment at: lib/Basic/FixedPoint.cpp:58
+ NewVal = NewVal.extOrTrunc(DstWidth);
+ return APFixedPoint(NewVal, DstSema);
+}
The signedness of NewVal is not being set here.
Comment at: unittests/Basic/FixedPointTest.cpp:380
+}
+
+// Check the value in a given fixed point sema overflows to the saturated max
Technically, neither CheckSaturatedConversion function checks whether or not
the result was correct, only that it saturated.
Comment at: unittests/Basic/FixedPointTest.cpp:506
+TEST(FixedPoint, AccConversionOverflow) {
+ // To SAcc max limit (65536)
+ CheckSaturatedConversionMax(getLAccSema(), Saturated(getAccSema()),
Acc?
Repository:
rC Clang
https://reviews.llvm.org/D48661
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336242 - [clangd] FileDistance: don't add duplicate edges
Author: sammccall
Date: Wed Jul 4 01:27:28 2018
New Revision: 336242
URL: http://llvm.org/viewvc/llvm-project?rev=336242&view=rev
Log:
[clangd] FileDistance: don't add duplicate edges
Modified:
clang-tools-extra/trunk/clangd/FileDistance.cpp
Modified: clang-tools-extra/trunk/clangd/FileDistance.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FileDistance.cpp?rev=336242&r1=336241&r2=336242&view=diff
==
--- clang-tools-extra/trunk/clangd/FileDistance.cpp (original)
+++ clang-tools-extra/trunk/clangd/FileDistance.cpp Wed Jul 4 01:27:28 2018
@@ -72,7 +72,9 @@ FileDistance::FileDistance(StringMap S.getValue().MaxUpTraversals) {
if (Cache.find(Hash) != Cache.end())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48916: Fix setting of empty implicit-section-name attribute for functions affected by '#pragma clang section'
chill added inline comments. Comment at: test/CodeGen/clang-sections-attribute.c:1 +// RUN: %clang_cc1 -emit-llvm -triple arm-none-eabi -o - %s | FileCheck %s + Isn't it possible for the test to fail if the Arm target is not configured? Repository: rC Clang https://reviews.llvm.org/D48916 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336243 - NFC - typo fix in test/CodeGen/avx512f-builtins.c
Author: gbuella
Date: Wed Jul 4 01:32:02 2018
New Revision: 336243
URL: http://llvm.org/viewvc/llvm-project?rev=336243&view=rev
Log:
NFC - typo fix in test/CodeGen/avx512f-builtins.c
Modified:
cfe/trunk/test/CodeGen/avx512f-builtins.c
Modified: cfe/trunk/test/CodeGen/avx512f-builtins.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512f-builtins.c?rev=336243&r1=336242&r2=336243&view=diff
==
--- cfe/trunk/test/CodeGen/avx512f-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx512f-builtins.c Wed Jul 4 01:32:02 2018
@@ -2757,12 +2757,12 @@ __m512d test_mm512_maskz_div_round_pd(__
return _mm512_maskz_div_round_pd(__U,__A,__B,_MM_FROUND_TO_NEAREST_INT |
_MM_FROUND_NO_EXC);
}
__m512d test_mm512_div_pd(__m512d __a, __m512d __b) {
- // CHECK-LABLE: @test_mm512_div_pd
+ // CHECK-LABEL: @test_mm512_div_pd
// CHECK: fdiv <8 x double>
return _mm512_div_pd(__a,__b);
}
__m512d test_mm512_mask_div_pd(__m512d __w, __mmask8 __u, __m512d __a, __m512d
__b) {
- // CHECK-LABLE: @test_mm512_mask_div_pd
+ // CHECK-LABEL: @test_mm512_mask_div_pd
// CHECK: fdiv <8 x double> %{{.*}}, %{{.*}}
// CHECK: select <8 x i1> %{{.*}}, <8 x double> %{{.*}}, <8 x double> %{{.*}}
return _mm512_mask_div_pd(__w,__u,__a,__b);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48880: [Sema] Fix crash in getConstructorName.
ilya-biryukov updated this revision to Diff 154063.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Better recovery for invalid decls that do have an injected class name
- Move the test to SemaCXX
Repository:
rC Clang
https://reviews.llvm.org/D48880
Files:
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/injected-class-name-crash.cpp
Index: test/SemaCXX/injected-class-name-crash.cpp
===
--- /dev/null
+++ test/SemaCXX/injected-class-name-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct X : public Foo
+X::X() {
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -113,6 +113,8 @@
break;
}
}
+ if (!InjectedClassName && CurClass->isInvalidDecl())
+return ParsedType();
assert(InjectedClassName && "couldn't find injected class name");
QualType T = Context.getTypeDeclType(InjectedClassName);
Index: test/SemaCXX/injected-class-name-crash.cpp
===
--- /dev/null
+++ test/SemaCXX/injected-class-name-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct X : public Foo
+X::X() {
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -113,6 +113,8 @@
break;
}
}
+ if (!InjectedClassName && CurClass->isInvalidDecl())
+return ParsedType();
assert(InjectedClassName && "couldn't find injected class name");
QualType T = Context.getTypeDeclType(InjectedClassName);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48880: [Sema] Fix crash in getConstructorName.
ilya-biryukov added a comment. Thanks for taking a look! Repository: rC Clang https://reviews.llvm.org/D48880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ilya-biryukov added a comment. Generally LG, just one comment. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we pass in the context with `PreferedType` there instead of reconstructing it later? To make sure we don't miss other things (incl. any future additions) that `ResultsBuilder` puts into the context. Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336244 - [Sema] Fix crash in getConstructorName.
Author: ibiryukov
Date: Wed Jul 4 01:50:12 2018
New Revision: 336244
URL: http://llvm.org/viewvc/llvm-project?rev=336244&view=rev
Log:
[Sema] Fix crash in getConstructorName.
Summary:
Can happen when getConstructorName is called on invalid decls,
specifically the ones that do not have the injected class name.
Reviewers: bkramer, rsmith
Reviewed By: rsmith
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D48880
Added:
cfe/trunk/test/SemaCXX/injected-class-name-crash.cpp
Modified:
cfe/trunk/lib/Sema/SemaExprCXX.cpp
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=336244&r1=336243&r2=336244&view=diff
==
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jul 4 01:50:12 2018
@@ -113,6 +113,8 @@ ParsedType Sema::getConstructorName(Iden
break;
}
}
+ if (!InjectedClassName && CurClass->isInvalidDecl())
+return ParsedType();
assert(InjectedClassName && "couldn't find injected class name");
QualType T = Context.getTypeDeclType(InjectedClassName);
Added: cfe/trunk/test/SemaCXX/injected-class-name-crash.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/injected-class-name-crash.cpp?rev=336244&view=auto
==
--- cfe/trunk/test/SemaCXX/injected-class-name-crash.cpp (added)
+++ cfe/trunk/test/SemaCXX/injected-class-name-crash.cpp Wed Jul 4 01:50:12
2018
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct X : public Foo
+X::X() {
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48880: [Sema] Fix crash in getConstructorName.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336244: [Sema] Fix crash in getConstructorName. (authored by
ibiryukov, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48880?vs=154063&id=154064#toc
Repository:
rC Clang
https://reviews.llvm.org/D48880
Files:
lib/Sema/SemaExprCXX.cpp
test/SemaCXX/injected-class-name-crash.cpp
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -113,6 +113,8 @@
break;
}
}
+ if (!InjectedClassName && CurClass->isInvalidDecl())
+return ParsedType();
assert(InjectedClassName && "couldn't find injected class name");
QualType T = Context.getTypeDeclType(InjectedClassName);
Index: test/SemaCXX/injected-class-name-crash.cpp
===
--- test/SemaCXX/injected-class-name-crash.cpp
+++ test/SemaCXX/injected-class-name-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct X : public Foo
+X::X() {
+}
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -113,6 +113,8 @@
break;
}
}
+ if (!InjectedClassName && CurClass->isInvalidDecl())
+return ParsedType();
assert(InjectedClassName && "couldn't find injected class name");
QualType T = Context.getTypeDeclType(InjectedClassName);
Index: test/SemaCXX/injected-class-name-crash.cpp
===
--- test/SemaCXX/injected-class-name-crash.cpp
+++ test/SemaCXX/injected-class-name-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct X : public Foo
+X::X() {
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336246 - [clangd] FileDistance: missing constexpr
Author: sammccall Date: Wed Jul 4 01:52:13 2018 New Revision: 336246 URL: http://llvm.org/viewvc/llvm-project?rev=336246&view=rev Log: [clangd] FileDistance: missing constexpr Modified: clang-tools-extra/trunk/clangd/FileDistance.cpp Modified: clang-tools-extra/trunk/clangd/FileDistance.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FileDistance.cpp?rev=336246&r1=336245&r2=336246&view=diff == --- clang-tools-extra/trunk/clangd/FileDistance.cpp (original) +++ clang-tools-extra/trunk/clangd/FileDistance.cpp Wed Jul 4 01:52:13 2018 @@ -54,7 +54,7 @@ static SmallString<128> canonicalize(Str return Result; } -const unsigned FileDistance::kUnreachable; +constexpr const unsigned FileDistance::kUnreachable; FileDistance::FileDistance(StringMap Sources, const FileDistanceOptions &Opts) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48427: [Analyzer] Fix for D47417 to make the tests pass
baloghadamsoftware updated this revision to Diff 154066.
baloghadamsoftware added a comment.
Adding of transition removed since it is part of
https://reviews.llvm.org/D47417.
https://reviews.llvm.org/D48427
Files:
lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
const ContainerData &CData);
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
bool isZero(ProgramStateRef State, const NonLoc &Val);
} // namespace
@@ -532,7 +533,9 @@
auto ContMap = State->get();
for (const auto Cont : ContMap) {
if (!SR.isLiveRegion(Cont.first)) {
- State = State->remove(Cont.first);
+ if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+ }
}
}
@@ -1174,6 +1177,22 @@
return State;
}
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+ auto RegionMap = State->get();
+ for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+ return true;
+ }
+
+ auto SymbolMap = State->get();
+ for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+ return true;
+ }
+
+ return false;
+}
+
bool isZero(ProgramStateRef State, const NonLoc &Val) {
auto &BVF = State->getBasicVals();
return compare(State, Val,
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
const MemRegion *Cont);
ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
const ContainerData &CData);
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
bool isZero(ProgramStateRef State, const NonLoc &Val);
} // namespace
@@ -532,7 +533,9 @@
auto ContMap = State->get();
for (const auto Cont : ContMap) {
if (!SR.isLiveRegion(Cont.first)) {
- State = State->remove(Cont.first);
+ if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+ }
}
}
@@ -1174,6 +1177,22 @@
return State;
}
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+ auto RegionMap = State->get();
+ for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+ return true;
+ }
+
+ auto SymbolMap = State->get();
+ for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+ return true;
+ }
+
+ return false;
+}
+
bool isZero(ProgramStateRef State, const NonLoc &Val) {
auto &BVF = State->getBasicVals();
return compare(State, Val,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
hokein added subscribers: bkramer, hokein.
hokein added a comment.
I'm not familiar with this part of code, but the change looks fine to me. I
think @bkramer is the right person to review it.
Please make sure the style align with LLVM code style.
Comment at: lib/Basic/VirtualFileSystem.cpp:507
/// Adapt a InMemoryFile for VFS' File interface.
class InMemoryFileAdaptor : public File {
I think we should have a comment saying the InMemoryFile has the same behavior
as the real file system.
Repository:
rC Clang
https://reviews.llvm.org/D48903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336248 - [clangd] FileDistance: temporarily disable in CodeComplete, it's behaving badly
Author: sammccall Date: Wed Jul 4 02:01:04 2018 New Revision: 336248 URL: http://llvm.org/viewvc/llvm-project?rev=336248&view=rev Log: [clangd] FileDistance: temporarily disable in CodeComplete, it's behaving badly Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=336248&r1=336247&r2=336248&view=diff == --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Jul 4 02:01:04 2018 @@ -1137,7 +1137,8 @@ private: SymbolQualitySignals Quality; SymbolRelevanceSignals Relevance; Relevance.Query = SymbolRelevanceSignals::CodeComplete; -Relevance.FileProximityMatch = FileProximity.getPointer(); +// FIXME: re-enable this after working out why it eats memory. +// Relevance.FileProximityMatch = FileProximity.getPointer(); auto &First = Bundle.front(); if (auto FuzzyScore = fuzzyScore(First)) Relevance.NameMatch = *FuzzyScore; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48921: NFC - type fix in test/CodeGenCXX/runtime-dllstorage.cpp
GBuella created this revision. GBuella added reviewers: compnerd, espindola. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D48921 Files: test/CodeGenCXX/runtime-dllstorage.cpp Index: test/CodeGenCXX/runtime-dllstorage.cpp === --- test/CodeGenCXX/runtime-dllstorage.cpp +++ test/CodeGenCXX/runtime-dllstorage.cpp @@ -1,13 +1,13 @@ // RUN: %clang_cc1 -triple i686-windows-msvc -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-MS -check-prefix CHECK-MS-DYNAMIC // RUN: %clang_cc1 -triple i686-windows-msvc -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-MS -check-prefix CHECK-MS-STATIC -// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-NODECL-IA -check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT +// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-NODECL-IA -check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-STATIC-IA -check-prefix CHECK-STATIC-NODECL-IA -check-prefix CHECK-IA-STATIC-CXA-ATEXIT -// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -DIMPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IMPORT-IA -check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT +// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -DIMPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IMPORT-IA -check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std -DIMPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-STATIC-IA -check-prefix CHECK-STATIC-IMPORT-IA -check-prefix CHECK-IA-STATIC-CXA-ATEXIT -// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -DEXPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT +// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -DEXPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std -DEXPORT_DECLARATIONS -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-STATIC-IA -check-prefix CHECK-IA-STATIC-CXA-ATEXIT -// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -DDECL -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-DECL-IA -check-prefix CHECK-DYANMIC-IA-CXA-ATEXIT +// RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -DDECL -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-DECL-IA -check-prefix CHECK-DYNAMIC-IA-CXA-ATEXIT // RUN: %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -flto-visibility-public-std -DDECL -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-STATIC-IA -check-prefix CHECK-STATIC-DECL-IA -check-prefix CHECK-IA-STATIC-CXA-ATEXIT // %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -fno-use-cxa-atexit -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-DYNAMIC-IA -check-prefix CHECK-DYNAMIC-IA-ATEXIT // %clang_cc1 -triple i686-windows-itanium -std=c++11 -fdeclspec -fms-compatibility -fexceptions -fcxx-exceptions -fno-use-cxa-atexit -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK-IA -check-prefix CHECK-STA
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
ilya-biryukov added a reviewer: ilya-biryukov.
ilya-biryukov added a comment.
Mimicing RealFS seems like the right thing to do here, so I would vouch for
checking this change in.
I'm a little worried that there are tests/clients relying on the old behavior,
have you run all the tests?
Also, could you please run `git-clang-format` to fix the formatting issues?
Comment at: lib/Basic/VirtualFileSystem.cpp:516
+ explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string RequestedName)
+ : Node(Node), RequestedName (std::move (RequestedName))
+ {}
NIT: The formatting is broken here.
Comment at: lib/Basic/VirtualFileSystem.cpp:520
+ llvm::ErrorOr status() override {
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
Maybe add a `RequestedName` parameter to the `InMemoryNode` instead to make
sure it's not misused?
It looks like all the clients calling it have to change the name and some are
not doing it now, e.g. the directory iterator will use statuses with full paths.
Comment at: lib/Basic/VirtualFileSystem.cpp:521
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
+ }
Maybe add a comment that this matches the real file system behavior?
Comment at: lib/Basic/VirtualFileSystem.cpp:722
if (Node)
-return (*Node)->getStatus();
+{
+ Status St = (*Node)->getStatus();
NIT: The indent is incorrect here.
Comment at: lib/Basic/VirtualFileSystem.cpp:724
+ Status St = (*Node)->getStatus();
+ return Status::copyWithNewName(St, Path.str());
+}
NIT: we don't need `str()` here
Repository:
rC Clang
https://reviews.llvm.org/D48903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response
ilya-biryukov added a comment. > I think the fixing way is to normalize the file path from AST (making it > absolute). Totally agree. Could we run the code used to get the URI to store in the dynamic index? Should we expose and reuse code in `getSymbolLocation()` from `SymbolCollector.cpp`? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336249 - Try to fix FileDistance test on windows.
Author: ioeric
Date: Wed Jul 4 02:08:40 2018
New Revision: 336249
URL: http://llvm.org/viewvc/llvm-project?rev=336249&view=rev
Log:
Try to fix FileDistance test on windows.
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11510/steps/ninja%20check%201/logs/FAIL%3A%20Extra%20Tools%20Unit%20Tests%3A%3AFileDistanceTests.URI
Modified:
clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
Modified: clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp?rev=336249&r1=336248&r2=336249&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp Wed Jul 4
02:08:40 2018
@@ -67,9 +67,15 @@ TEST(FileDistanceTests, URI) {
SourceParams CostLots;
CostLots.Cost = 1000;
- URIDistance D(
- {{testPath("foo"), CostLots}, {"/not/a/testpath", SourceParams()}},
Opts);
+ URIDistance D({{testPath("foo"), CostLots},
+ {"/not/a/testpath", SourceParams()},
+ {"C:\\not\\a\\testpath", SourceParams()}},
+Opts);
+#ifdef _WIN32
+ EXPECT_EQ(D.distance("file:///C%3a/not/a/testpath/either"), 3u);
+#else
EXPECT_EQ(D.distance("file:///not/a/testpath/either"), 3u);
+#endif
EXPECT_EQ(D.distance("unittest:foo"), 1000u);
EXPECT_EQ(D.distance("unittest:bar"), 1008u);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.
Hi Carlos, thanks for letting us know! I sent r336249 to fix the windows test. On Wed, Jul 4, 2018 at 9:51 AM Carlos Alberto Enciso via Phabricator < revi...@reviews.llvm.org> wrote: > CarlosAlbertoEnciso added a comment. > > Hi, > > It seems that your patch is causing an error in the tests for > > http://lab.llvm.org:8011/builders/clang-x64-ninja-win7 > > > http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11510/steps/ninja%20check%201/logs/FAIL%3A%20Extra%20Tools%20Unit%20Tests%3A%3AFileDistanceTests.URI > > Thanks very much > > > Repository: > rL LLVM > > https://reviews.llvm.org/D48441 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.
ioeric added a subscriber: sammccall. ioeric added a comment. Hi Carlos, thanks for letting us know! I sent r336249 to fix the windows test. Repository: rL LLVM https://reviews.llvm.org/D48441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
ilya-biryukov added a comment. Sorry, have missed the @hokein 's comments, so one of mine seems like a duplicate. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48916: Fix setting of empty implicit-section-name attribute for functions affected by '#pragma clang section'
petpav01 added inline comments. Comment at: test/CodeGen/clang-sections-attribute.c:1 +// RUN: %clang_cc1 -emit-llvm -triple arm-none-eabi -o - %s | FileCheck %s + chill wrote: > Isn't it possible for the test to fail if the Arm target is not configured? I think it should not be necessary to add a REQUIRES line because the test emits and checks only LLVM IR but no target codegen is done. An experiment without having the ARM target configured shows that the test still works in such a setting. Note also that the two already present tests for `#pragma clang section` (`clang-sections.cpp` and `clang-sections-tentative.c`) have a similar header as this new test as well. Repository: rC Clang https://reviews.llvm.org/D48916 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ioeric added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, ilya-biryukov wrote: > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we pass > in the context with `PreferedType` there instead of reconstructing it later? > To make sure we don't miss other things (incl. any future additions) that > `ResultsBuilder` puts into the context. The `PreferedType` should actually already be set in the `ResultsBuilder` (line 3715). In thoery, `Results.getCompletionContext()` should work fine here as well, but it would break some Index tests - it introduced some inconsistency in sema scoring when running with and without result caching (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35). This is probably a bug, but I'm not sure if I'm the right person to chase it :( Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48628: [AST] Structural equivalence of methods
balazske updated this revision to Diff 154073.
balazske added a comment.
- Updates according to review comments
Repository:
rC Clang
https://reviews.llvm.org/D48628
Files:
lib/AST/ASTImporter.cpp
lib/AST/ASTStructuralEquivalence.cpp
unittests/AST/ASTImporterTest.cpp
unittests/AST/StructuralEquivalenceTest.cpp
Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -18,13 +18,13 @@
std::unique_ptr AST0, AST1;
std::string Code0, Code1; // Buffers for SourceManager
- // Get a pair of Decl pointers to the synthetised declarations from the given
- // code snipets. By default we search for the unique Decl with name 'foo' in
- // both snippets.
- std::tuple
- makeNamedDecls(const std::string &SrcCode0, const std::string &SrcCode1,
- Language Lang, const char *const Identifier = "foo") {
-
+ // Get a pair of node pointers into the synthesized AST from the given code
+ // snippets. To determine the returned node, a separate matcher is specified
+ // for both snippets. The first matching node is returned.
+ template
+ std::tuple makeDecls(
+ const std::string &SrcCode0, const std::string &SrcCode1, Language Lang,
+ const MatcherType &Matcher0, const MatcherType &Matcher1) {
this->Code0 = SrcCode0;
this->Code1 = SrcCode1;
ArgVector Args = getBasicRunOptionsForLanguage(Lang);
@@ -34,28 +34,32 @@
AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args, InputFileName);
AST1 = tooling::buildASTFromCodeWithArgs(Code1, Args, InputFileName);
-ASTContext &Ctx0 = AST0->getASTContext(), &Ctx1 = AST1->getASTContext();
-
-auto getDecl = [](ASTContext &Ctx, const std::string &Name) -> NamedDecl * {
- IdentifierInfo *SearchedII = &Ctx.Idents.get(Name);
- assert(SearchedII && "Declaration with the identifier "
- "should be specified in test!");
- DeclarationName SearchDeclName(SearchedII);
- SmallVector FoundDecls;
- Ctx.getTranslationUnitDecl()->localUncachedLookup(SearchDeclName,
-FoundDecls);
+NodeType *D0 = FirstDeclMatcher().match(
+AST0->getASTContext().getTranslationUnitDecl(), Matcher0);
+NodeType *D1 = FirstDeclMatcher().match(
+AST1->getASTContext().getTranslationUnitDecl(), Matcher1);
- // We should find one Decl but one only.
- assert(FoundDecls.size() == 1);
+return std::make_tuple(D0, D1);
+ }
- return FoundDecls[0];
-};
+ // Get a pair of node pointers into the synthesized AST from the given code
+ // snippets. The same matcher is used for both snippets.
+ template
+ std::tuple makeDecls(
+ const std::string &SrcCode0, const std::string &SrcCode1, Language Lang,
+ const MatcherType &AMatcher) {
+return makeDecls(
+ SrcCode0, SrcCode1, Lang, AMatcher, AMatcher);
+ }
-NamedDecl *D0 = getDecl(Ctx0, Identifier);
-NamedDecl *D1 = getDecl(Ctx1, Identifier);
-assert(D0);
-assert(D1);
-return std::make_tuple(D0, D1);
+ // Get a pair of Decl pointers to the synthesized declarations from the given
+ // code snippets. We search for the first NamedDecl with given name in both
+ // snippets.
+ std::tuple makeNamedDecls(
+ const std::string &SrcCode0, const std::string &SrcCode1,
+ Language Lang, const char *const Identifier = "foo") {
+auto Matcher = namedDecl(hasName(Identifier));
+return makeDecls(SrcCode0, SrcCode1, Lang, Matcher);
}
bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
@@ -110,35 +114,29 @@
}
TEST_F(StructuralEquivalenceTest, IntVsSignedIntTemplateSpec) {
- auto Decls = makeNamedDecls(
- "template struct foo; template<> struct foo{};",
- "template struct foo; template<> struct foo{};",
- Lang_CXX);
- ClassTemplateSpecializationDecl *Spec0 =
- *cast(get<0>(Decls))->spec_begin();
- ClassTemplateSpecializationDecl *Spec1 =
- *cast(get<1>(Decls))->spec_begin();
- ASSERT_TRUE(Spec0 != nullptr);
- ASSERT_TRUE(Spec1 != nullptr);
+ auto Decls = makeDecls(
+ R"(template struct foo; template<> struct foo{};)",
+ R"(template struct foo; template<> struct foo{};)",
+ Lang_CXX,
+ classTemplateSpecializationDecl());
+ auto Spec0 = get<0>(Decls);
+ auto Spec1 = get<1>(Decls);
EXPECT_TRUE(testStructuralMatch(Spec0, Spec1));
}
TEST_F(StructuralEquivalenceTest, CharVsSignedCharTemplateSpec) {
- auto Decls = makeNamedDecls(
- "template struct foo; template<> struct foo{};",
- "template struct foo; template<> struct foo{};",
- Lang_CXX);
- ClassTemplateSpecializationDecl *Spec0 =
- *cast(get<0>(Decls))->spec_begin();
- ClassTemplateSpecializationDecl *Spec1 =
- *cast(get<1>(Decls))->spec_begin();
- ASSERT_TRUE(Spec0 != nullptr);
- ASSERT_TRUE
[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM with a minor NIT
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:88
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+ llvm::errs() << "Found errors in the translation unit. Igoring "
+ "collected symbols...\n";
NIT: Maybe use clangd's `log` here? To get timestamps in the output, etc.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types
martong marked an inline comment as done.
martong added inline comments.
Comment at: lib/AST/ASTImporter.cpp:2085
}
+ } else {
+if (!IsStructuralMatch(D, FoundRecord, false))
balazske wrote:
> martong wrote:
> > a_sidorin wrote:
> > > Is it possible to use the added code for the entire condition `if (auto
> > > *FoundRecord = dyn_cast(Found))`, replacing its body? Our
> > > structural matching already knows how to handle unnamed structures, and
> > > the upper code partially duplicates
> > > `IsStructurallyEquivalent(StructuralEquivalenceContext
> > > &Context,RecordDecl *D1, RecordDecl *D2)`. Can we change structural
> > > matching to handle this stuff instead of doing it in ASTNodeImporter?
> > Yes, this is a good point, updated the patch accordingly.
> My idea was to remove this whole `if (!SearchName)` branch, but some
> restructuring of the next conditions may be needed. Setting of `PrevDecl` in
> the `if` branch does not look correct (if `!SearchName` is false it is never
> set, unlike in the old code).
I tried what you mentioned Balazs.
It turned out, we can't just skip the whole `if (!SearchName)` branch. Because
in the below case we would match the first unnamed union with the second,
therefore we will break functionality (and some lit tests). The `continue` at
line 2078 is very important in this edge case. (I agree that this code is very
messy and would be good if we could refactor that to be cleaner, particularly
if we could remove the `SearchName` variable would make it way better.)
```
// Matches
struct Unnamed {
union {
struct {
int i;
} S;
struct {
float i;
} R;
} U;
} x14;
```
The failing lit test and it's output:
```
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:84:5:
warning: type 'struct Unnamed::(anonymous at
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:84:5)'
has incompatible definitions in different translation units
struct {
^
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:85:11:
note: field 'i' has type 'int' here
int i;
^
/home/egbomrt/WORK/llvm3/git/llvm/tools/clang/test/ASTMerge/struct/Inputs/struct1.c:88:13:
note: field 'i' has type 'float' here
float i;
^
```
Repository:
rC Clang
https://reviews.llvm.org/D48773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, ioeric wrote: > ilya-biryukov wrote: > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we > > pass in the context with `PreferedType` there instead of reconstructing it > > later? > > To make sure we don't miss other things (incl. any future additions) that > > `ResultsBuilder` puts into the context. > The `PreferedType` should actually already be set in the `ResultsBuilder` > (line 3715). In thoery, `Results.getCompletionContext()` should work fine > here as well, but it would break some Index tests - it introduced some > inconsistency in sema scoring when running with and without result caching > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35). > This is probably a bug, but I'm not sure if I'm the right person to chase it > :( What kind of inconsistencies? Maybe we should just update the CHECKS in the test? Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.
ioeric added inline comments.
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:88
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+ llvm::errs() << "Found errors in the translation unit. Igoring "
+ "collected symbols...\n";
ilya-biryukov wrote:
> NIT: Maybe use clangd's `log` here? To get timestamps in the output, etc.
The default logger doesn't seem to add timestamp? I'll keep `llvm::errs()` here
for consistency in this file.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added a comment.
In https://reviews.llvm.org/D46190#1135688, @rsmith wrote:
> The right way to handle this is to pass both the ultimately-selected
> declaration and the declaration found by name lookup into the calls to
> `MarkAnyDeclReferenced` and friends. We should mark the selected declaration
> as `Used` (when appropriate), and mark the found declaration as `Referenced`.
>
> We should not be trying to reconstruct which using declarations are in scope
> after the fact. Only declarations actually found by name lookup and then
> selected by the context performing the lookup should be marked referenced.
> (There may be cases where name lookup discards equivalent lookup results that
> are not redeclarations of one another, though, and to handle such cases
> properly, you may need to teach name lookup to track a list of found
> declarations rather than only one.)
Following your comments, I have replaced the scope traversal with `LookupName`
in order to find the declarations.
But there are 2 specific cases:
- using-directives
The `LookupName` function does not record the using-directives. With this
patch, there is an extra option to store those directives in the lookup
results, to be processed during the setting of the 'Referenced' bit.
- namespace-alias
I am aware of your comment on the function that recursively traverse the
namespace alias, but I can't see another way to do it. If you have a more
specific idea, I am happy to explore it.
For both cases, may be `LookupName` function can have that functionality and
store in the results any namespace-directives and namespace-alias associated
with the given declaration.
Comment at: lib/Sema/Sema.cpp:1879
+void Sema::MarkNamespaceAliasReferenced(NamedDecl *ND) {
+ if (ND && !ND->isReferenced()) {
+NamespaceAliasDecl *NA = nullptr;
probinson wrote:
> You could do this as an early return and reduce indentation in the rest of
> the method.
> ```
> if (!ND || ND->isReferenced())
> return;
> ```
>
Corrected to
```
if (!ND || ND->isReferenced())
return;
```
Comment at: lib/Sema/Sema.cpp:1880
+ if (ND && !ND->isReferenced()) {
+NamespaceAliasDecl *NA = nullptr;
+while ((NA = dyn_cast(ND)) && !NA->isReferenced()) {
probinson wrote:
> Initializing this to nullptr is redundant, as you set NA in the while-loop
> expression.
Removed the `nullptr` initialization.
Comment at: lib/Sema/Sema.cpp:1891
+/// \brief Mark as referenced any 'using declaration' that have introduced
+/// the given declaration in the current context.
+void Sema::MarkUsingReferenced(Decl *D, CXXScopeSpec *SS, Expr *E) {
probinson wrote:
> `\brief` is unnecessary, as we have auto-brief turned on.
Removed the `\brief` format.
Comment at: lib/Sema/Sema.cpp:1903
+ if (auto *NNS = SS ? SS->getScopeRep()
+ : E ? dyn_cast(E)->getQualifier()
+ : nullptr) {
probinson wrote:
> This dyn_cast<> can be simply a cast<>.
Changed the dyn_cast<> to cast<>
Comment at: lib/Sema/Sema.cpp:1916
+ if ((USD = dyn_cast(DR)) && !USD->isReferenced()) {
+if (USD->getTargetDecl() == D) {
+ USD->setReferenced();
probinson wrote:
> You could sink the declaration of USD like so:
> ```
> for (auto *DR : S->decls())
> if (auto *USD = dyn_cast(DR))
> if (!USD->isReferenced() && USD->getTargetDecl() == D) {
> ```
> Also I would put braces around the 'for' loop body, even though it is
> technically one statement.
Not available in the new patch.
Comment at: lib/Sema/Sema.cpp:1927
+// Check if the declaration was introduced by a 'using-directive'.
+auto *Target = dyn_cast(DC);
+for (auto *UD : S->using_directives())
probinson wrote:
> You verified that his is a namespace earlier, so use cast<> not dyn_cast<>.
Not available in the new patch.
https://reviews.llvm.org/D46190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a referenced declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added a comment. In https://reviews.llvm.org/D46190#1135295, @probinson wrote: > Style comments. > The two new Sema methods (for namespaces and using references) are C++ > specific, so SemaDeclCXX.cpp would seem like a more appropriate home for them. Both functions have been moved to SemaDeclCXX.cpp. https://reviews.llvm.org/D46190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ilya-biryukov added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can we > > > pass in the context with `PreferedType` there instead of reconstructing > > > it later? > > > To make sure we don't miss other things (incl. any future additions) that > > > `ResultsBuilder` puts into the context. > > The `PreferedType` should actually already be set in the `ResultsBuilder` > > (line 3715). In thoery, `Results.getCompletionContext()` should work fine > > here as well, but it would break some Index tests - it introduced some > > inconsistency in sema scoring when running with and without result caching > > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35). > > This is probably a bug, but I'm not sure if I'm the right person to chase > > it :( > What kind of inconsistencies? Maybe we should just update the CHECKS in the > test? After chatting offline: it seems passing in the context that has the preferred type set into `ResultsBuilder` saves us from check failures. The fact that `ResultsBuilder` also has `setPreferrredType`, which can be different from the one in the `CodeCompletionContext` is still confusing, but that's unrelated to the problem at hand. Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ioeric updated this revision to Diff 154077.
ioeric added a comment.
- Addressed review comment.
Repository:
rC Clang
https://reviews.llvm.org/D48917
Files:
lib/Sema/SemaCodeComplete.cpp
unittests/Sema/CodeCompleteTest.cpp
Index: unittests/Sema/CodeCompleteTest.cpp
===
--- unittests/Sema/CodeCompleteTest.cpp
+++ unittests/Sema/CodeCompleteTest.cpp
@@ -131,4 +131,15 @@
EXPECT_TRUE(VisitedNS.empty());
}
+TEST(SemaCodeCompleteTest, VisitedNSWithoutQualifier) {
+ auto VisitedNS = runCodeCompleteOnCode(R"cpp(
+namespace n1 {
+namespace n2 {
+ void f(^) {}
+}
+}
+ )cpp");
+ EXPECT_THAT(VisitedNS, UnorderedElementsAre("n1", "n1::n2"));
+}
+
} // namespace
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3700,9 +3700,11 @@
/// type we're looking for.
void Sema::CodeCompleteExpression(Scope *S,
const CodeCompleteExpressionData &Data) {
- ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Expression);
+ ResultBuilder Results(
+ *this, CodeCompleter->getAllocator(),
+ CodeCompleter->getCodeCompletionTUInfo(),
+ CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+Data.PreferredType));
if (Data.ObjCCollection)
Results.setFilter(&ResultBuilder::IsObjCCollection);
else if (Data.IntegralConstantExpression)
@@ -3741,10 +3743,8 @@
if (CodeCompleter->includeMacros())
AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext(CodeCompletionContext::CCC_Expression,
- Data.PreferredType),
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4360,11 @@
}
Results.ExitScope();
- //We need to make sure we're setting the right context,
- //so only say we include macros if the code completer says we do
- enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
if (CodeCompleter->includeMacros()) {
AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
}
-
- HandleCodeCompleteResults(this, CodeCompleter,
-kind,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4767,9 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4788,8 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteNamespaceDecl(Scope *S) {
@@ -4893,10 +4885,9 @@
// Add any type specifiers
AddTypeSpecifierResults(getLangOpts(), Results);
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5168,8 @@
else
AddObjCTopLevelResults(Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
ioeric added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:3744 AddMacroResults(PP, Results, false, PreferredTypeIsPointer); - HandleCodeCompleteResults(this, CodeCompleter, -CodeCompletionContext(CodeCompletionContext::CCC_Expression, ilya-biryukov wrote: > ilya-biryukov wrote: > > ioeric wrote: > > > ilya-biryukov wrote: > > > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can > > > > we pass in the context with `PreferedType` there instead of > > > > reconstructing it later? > > > > To make sure we don't miss other things (incl. any future additions) > > > > that `ResultsBuilder` puts into the context. > > > The `PreferedType` should actually already be set in the `ResultsBuilder` > > > (line 3715). In thoery, `Results.getCompletionContext()` should work fine > > > here as well, but it would break some Index tests - it introduced some > > > inconsistency in sema scoring when running with and without result > > > caching > > > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35). > > > This is probably a bug, but I'm not sure if I'm the right person to > > > chase it :( > > What kind of inconsistencies? Maybe we should just update the CHECKS in the > > test? > After chatting offline: it seems passing in the context that has the > preferred type set into `ResultsBuilder` saves us from check failures. > The fact that `ResultsBuilder` also has `setPreferrredType`, which can be > different from the one in the `CodeCompletionContext` is still confusing, but > that's unrelated to the problem at hand. Sounds good. Thanks! Repository: rC Clang https://reviews.llvm.org/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336252 - [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.
Author: ioeric
Date: Wed Jul 4 02:43:35 2018
New Revision: 336252
URL: http://llvm.org/viewvc/llvm-project?rev=336252&view=rev
Log:
[clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.
Summary:
For example, template parameter might not be resolved in a broken TU,
which can result in wrong USR/SymbolID.
Reviewers: ilya-biryukov
Subscribers: MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D48881
Modified:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Modified:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=336252&r1=336251&r2=336252&view=diff
==
---
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
(original)
+++
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Wed Jul 4 02:43:35 2018
@@ -82,6 +82,14 @@ public:
void EndSourceFileAction() override {
WrapperFrontendAction::EndSourceFileAction();
+const auto &CI = getCompilerInstance();
+if (CI.hasDiagnostics() &&
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+ llvm::errs() << "Found errors in the translation unit. Igoring "
+ "collected symbols...\n";
+ return;
+}
+
auto Symbols = Collector->takeSymbols();
for (const auto &Sym : Symbols) {
Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336252: [clangd] Avoid collecting symbols from broken TUs in
global-symbol-builder. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D48881
Files:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Index:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
---
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -82,6 +82,14 @@
void EndSourceFileAction() override {
WrapperFrontendAction::EndSourceFileAction();
+const auto &CI = getCompilerInstance();
+if (CI.hasDiagnostics() &&
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+ llvm::errs() << "Found errors in the translation unit. Igoring "
+ "collected symbols...\n";
+ return;
+}
+
auto Symbols = Collector->takeSymbols();
for (const auto &Sym : Symbols) {
Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));
Index: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===
--- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -82,6 +82,14 @@
void EndSourceFileAction() override {
WrapperFrontendAction::EndSourceFileAction();
+const auto &CI = getCompilerInstance();
+if (CI.hasDiagnostics() &&
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+ llvm::errs() << "Found errors in the translation unit. Igoring "
+ "collected symbols...\n";
+ return;
+}
+
auto Symbols = Collector->takeSymbols();
for (const auto &Sym : Symbols) {
Ctx->reportResult(Sym.ID.str(), SymbolToYAML(Sym));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48881: [clangd] Avoid collecting symbols from broken TUs in global-symbol-builder.
ilya-biryukov added inline comments.
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:88
+(CI.getDiagnosticClient().getNumErrors() > 0)) {
+ llvm::errs() << "Found errors in the translation unit. Igoring "
+ "collected symbols...\n";
ioeric wrote:
> ilya-biryukov wrote:
> > NIT: Maybe use clangd's `log` here? To get timestamps in the output, etc.
> The default logger doesn't seem to add timestamp? I'll keep `llvm::errs()`
> here for consistency in this file.
> The default logger doesn't seem to add timestamp?
Ah, yes, it's the `JSONOutput` that adds timestamps Sorry.
> I'll keep llvm::errs() here for consistency in this file.
We write to `stderr` at the bottom of the file to show errors to the user.
This instance looks much more like a logging routine.
Since our dependencies (e.g. `SymbolCollector`) use logs, I'd argue it's
actually **more** consistent across the program to use `log` here as well. E.g.
if someone adds a `LoggingSession` that writes timestamps, not only
SymbolCollector logs should start showing those, but this one as well.
Repository:
rL LLVM
https://reviews.llvm.org/D48881
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336253 - [clangd] Cleanup unittest: URIs. NFC.
Author: ioeric
Date: Wed Jul 4 02:54:23 2018
New Revision: 336253
URL: http://llvm.org/viewvc/llvm-project?rev=336253&view=rev
Log:
[clangd] Cleanup unittest: URIs. NFC.
Modified:
clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
Modified: clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp?rev=336253&r1=336252&r2=336253&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FileDistanceTests.cpp Wed Jul 4
02:54:23 2018
@@ -76,8 +76,8 @@ TEST(FileDistanceTests, URI) {
#else
EXPECT_EQ(D.distance("file:///not/a/testpath/either"), 3u);
#endif
- EXPECT_EQ(D.distance("unittest:foo"), 1000u);
- EXPECT_EQ(D.distance("unittest:bar"), 1008u);
+ EXPECT_EQ(D.distance("unittest:///foo"), 1000u);
+ EXPECT_EQ(D.distance("unittest:///bar"), 1008u);
}
TEST(FileDistance, LimitUpTraversals) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
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/D48917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336255 - [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
Author: ioeric
Date: Wed Jul 4 03:01:18 2018
New Revision: 336255
URL: http://llvm.org/viewvc/llvm-project?rev=336255&view=rev
Log:
[SemaCodeComplete] Make sure visited contexts are passed to completion results
handler.
Reviewers: ilya-biryukov
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D48917
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=336255&r1=336254&r2=336255&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Wed Jul 4 03:01:18 2018
@@ -3700,9 +3700,11 @@ struct Sema::CodeCompleteExpressionData
/// type we're looking for.
void Sema::CodeCompleteExpression(Scope *S,
const CodeCompleteExpressionData &Data) {
- ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Expression);
+ ResultBuilder Results(
+ *this, CodeCompleter->getAllocator(),
+ CodeCompleter->getCodeCompletionTUInfo(),
+ CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+Data.PreferredType));
if (Data.ObjCCollection)
Results.setFilter(&ResultBuilder::IsObjCCollection);
else if (Data.IntegralConstantExpression)
@@ -3741,10 +3743,8 @@ void Sema::CodeCompleteExpression(Scope
if (CodeCompleter->includeMacros())
AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext(CodeCompletionContext::CCC_Expression,
- Data.PreferredType),
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter,
Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4360,11 @@ void Sema::CodeCompleteCase(Scope *S) {
}
Results.ExitScope();
- //We need to make sure we're setting the right context,
- //so only say we include macros if the code completer says we do
- enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
if (CodeCompleter->includeMacros()) {
AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
}
-
- HandleCodeCompleteResults(this, CodeCompleter,
-kind,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter,
Results.getCompletionContext(),
+Results.data(), Results.size());
}
static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4767,9 @@ void Sema::CodeCompleteUsing(Scope *S) {
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-
CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter,
Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4788,8 @@ void Sema::CodeCompleteUsingDirective(Sc
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter,
Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteNamespaceDecl(Scope *S) {
@@ -4893,10 +4885,9 @@ void Sema::CodeCompleteOperatorName(Scop
// Add any type specifiers
AddTypeSpecifierResults(getLangOpts(), Results);
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter,
Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5168,8 @@ void Sema::CodeCompleteObjCAtDirective(S
else
AddObjCTopLevelResults(Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other
[PATCH] D48917: [SemaCodeComplete] Make sure visited contexts are passed to completion results handler.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336255: [SemaCodeComplete] Make sure visited contexts are
passed to completion results… (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48917?vs=154077&id=154080#toc
Repository:
rC Clang
https://reviews.llvm.org/D48917
Files:
lib/Sema/SemaCodeComplete.cpp
unittests/Sema/CodeCompleteTest.cpp
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3700,9 +3700,11 @@
/// type we're looking for.
void Sema::CodeCompleteExpression(Scope *S,
const CodeCompleteExpressionData &Data) {
- ResultBuilder Results(*this, CodeCompleter->getAllocator(),
-CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Expression);
+ ResultBuilder Results(
+ *this, CodeCompleter->getAllocator(),
+ CodeCompleter->getCodeCompletionTUInfo(),
+ CodeCompletionContext(CodeCompletionContext::CCC_Expression,
+Data.PreferredType));
if (Data.ObjCCollection)
Results.setFilter(&ResultBuilder::IsObjCCollection);
else if (Data.IntegralConstantExpression)
@@ -3741,10 +3743,8 @@
if (CodeCompleter->includeMacros())
AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext(CodeCompletionContext::CCC_Expression,
- Data.PreferredType),
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompletePostfixExpression(Scope *S, ExprResult E) {
@@ -4360,17 +4360,11 @@
}
Results.ExitScope();
- //We need to make sure we're setting the right context,
- //so only say we include macros if the code completer says we do
- enum CodeCompletionContext::Kind kind = CodeCompletionContext::CCC_Other;
if (CodeCompleter->includeMacros()) {
AddMacroResults(PP, Results, false);
-kind = CodeCompletionContext::CCC_OtherWithMacros;
}
-
- HandleCodeCompleteResults(this, CodeCompleter,
-kind,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static bool anyNullArguments(ArrayRef Args) {
@@ -4773,10 +4767,9 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteUsingDirective(Scope *S) {
@@ -4795,9 +4788,8 @@
CodeCompleter->includeGlobals(),
CodeCompleter->loadExternal());
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Namespace,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteNamespaceDecl(Scope *S) {
@@ -4893,10 +4885,9 @@
// Add any type specifiers
AddTypeSpecifierResults(getLangOpts(), Results);
Results.ExitScope();
-
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Type,
-Results.data(),Results.size());
+
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
void Sema::CodeCompleteConstructorInitializer(
@@ -5177,9 +5168,8 @@
else
AddObjCTopLevelResults(Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-CodeCompletionContext::CCC_Other,
-Results.data(),Results.size());
+ HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
+Results.data(), Results.size());
}
static void AddObjCExpressionResults(ResultBuilder &Results, bool NeedAt) {
@@ -5311,9 +5301,8 @@
Results.EnterNewScope();
AddObjCVisibilityResults(getLangOpts(), Results, false);
Results.ExitScope();
- HandleCodeCompleteResults(this, CodeCompleter,
-
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
deepak2427 added a comment. I encountered the issue while working with the unroller and found that it was not following the pragma info, and traced it back to the issue with metadata. As far as I understood, for for-loops and while-loops, we add the metadata only to the loop back-edge. So it would make sense to keep them consistent. I'm not an expert in clang, and do not know how we can detect such problems. Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336257 - [clang-tools-extra] Cleanup documentation routine
Author: omtcyfz
Date: Wed Jul 4 03:18:03 2018
New Revision: 336257
URL: http://llvm.org/viewvc/llvm-project?rev=336257&view=rev
Log:
[clang-tools-extra] Cleanup documentation routine
The following issues are resolved:
* Doxygen didn't generate documentation for a bunch of existing tools
due to the absence of their directories in the doxygen configuration
file. This patch adds all relevant directories to the appropriate list.
* clang-tools-extra/docs/Doxyfile seems to be unused and irrelevant,
doxygen.cfg.in is passed to the CMake's Doxygen invocation, hence
Doxyfile is removed.
The validity of proposed changes was manually checked by building
doxygen-clang-tools and making sure that clangd and other tools are
present in Doxygen-generated docs of clang-tools-extra.
Reviewers: ioeric
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D47537
Removed:
clang-tools-extra/trunk/docs/Doxyfile
Modified:
clang-tools-extra/trunk/docs/doxygen.cfg.in
Removed: clang-tools-extra/trunk/docs/Doxyfile
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/Doxyfile?rev=336256&view=auto
==
--- clang-tools-extra/trunk/docs/Doxyfile (original)
+++ clang-tools-extra/trunk/docs/Doxyfile (removed)
@@ -1,1808 +0,0 @@
-# Doxyfile 1.7.6.1
-
-# This file describes the settings to be used by the documentation system
-# doxygen (www.doxygen.org) for a project
-#
-# All text after a hash (#) is considered a comment and will be ignored
-# The format is:
-# TAG = value [value, ...]
-# For lists items can also be appended using:
-# TAG += value [value, ...]
-# Values that contain spaces should be placed between quotes (" ")
-
-#---
-# Project related configuration options
-#---
-
-# This tag specifies the encoding used for all characters in the config file
-# that follow. The default is UTF-8 which is also the encoding used for all
-# text before the first occurrence of this tag. Doxygen uses libiconv (or the
-# iconv built into libc) for the transcoding. See
-# http://www.gnu.org/software/libiconv for the list of possible encodings.
-
-DOXYFILE_ENCODING = UTF-8
-
-# The PROJECT_NAME tag is a single word (or sequence of words) that should
-# identify the project. Note that if you do not use Doxywizard you need
-# to put quotes around the project name if it contains spaces.
-
-PROJECT_NAME = clang-tools-extra
-
-# The PROJECT_NUMBER tag can be used to enter a project or revision number.
-# This could be handy for archiving the generated documentation or
-# if some version control system is used.
-
-PROJECT_NUMBER =
-
-# Using the PROJECT_BRIEF tag one can provide an optional one line description
-# for a project that appears at the top of each page and should give viewer
-# a quick idea about the purpose of the project. Keep the description short.
-
-PROJECT_BRIEF =
-
-# With the PROJECT_LOGO tag one can specify an logo or icon that is
-# included in the documentation. The maximum height of the logo should not
-# exceed 55 pixels and the maximum width should not exceed 200 pixels.
-# Doxygen will copy the logo to the output directory.
-
-PROJECT_LOGO =
-
-# The OUTPUT_DIRECTORY tag is used to specify the (relative or absolute)
-# base path where the generated documentation will be put.
-# If a relative path is entered, it will be relative to the location
-# where doxygen was started. If left blank the current directory will be used.
-
-# Same directory that Sphinx uses.
-OUTPUT_DIRECTORY = ./_build/
-
-# If the CREATE_SUBDIRS tag is set to YES, then doxygen will create
-# 4096 sub-directories (in 2 levels) under the output directory of each output
-# format and will distribute the generated files over these directories.
-# Enabling this option can be useful when feeding doxygen a huge amount of
-# source files, where putting all generated files in the same directory would
-# otherwise cause performance problems for the file system.
-
-CREATE_SUBDIRS = NO
-
-# The OUTPUT_LANGUAGE tag is used to specify the language in which all
-# documentation generated by doxygen is written. Doxygen will use this
-# information to generate all constant output in the proper language.
-# The default language is English, other supported languages are:
-# Afrikaans, Arabic, Brazilian, Catalan, Chinese, Chinese-Traditional,
-# Croatian, Czech, Danish, Dutch, Esperanto, Farsi, Finnish, French, German,
-# Greek, Hungarian, Italian, Japanese, Japanese-en (Japanese with English
-# messages), Korean, Korean-en, Lithuanian, Norwegian, Macedonian, Persian,
-# Polish, Portuguese, Romanian, Russian, Serbian, Serbian-Cyrillic, Slovak,
-# Slovene, Spanish, Swedish, Ukrainian, and Vietnamese.
-
-OUTPUT_LANGUAGE= English
-
-# If
[PATCH] D48314: [Frontend] Cache preamble-related data
ilya-biryukov added a comment. I would argue this should be handled by the clients instead. Adding global state and locking is complicated. (And ASTUnit is complicated enough). What are the use-cases of creating multiple `ASTUnit` inside the same process for the same file? Which clients do that and why they can't have a single ASTUnit per file? https://reviews.llvm.org/D48314 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47537: [clang-tools-extra] Cleanup documentation routine
This revision was automatically updated to reflect the committed changes.
Closed by commit rL336257: [clang-tools-extra] Cleanup documentation routine
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47537?vs=150083&id=154086#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47537
Files:
clang-tools-extra/trunk/docs/Doxyfile
clang-tools-extra/trunk/docs/doxygen.cfg.in
Index: clang-tools-extra/trunk/docs/doxygen.cfg.in
===
--- clang-tools-extra/trunk/docs/doxygen.cfg.in
+++ clang-tools-extra/trunk/docs/doxygen.cfg.in
@@ -743,15 +743,20 @@
# spaces.
# Note: If this tag is empty the current directory is searched.
-INPUT = \
- @abs_srcdir@/../clang-tidy \
- @abs_srcdir@/../clang-apply-replacements \
- @abs_srcdir@/../clang-query \
- @abs_srcdir@/../clang-rename \
- @abs_srcdir@/../modularize \
- @abs_srcdir@/../pp-trace \
- @abs_srcdir@/../tool-template \
- @abs_srcdir@/doxygen-mainpage.dox
+INPUT = \
+ @abs_srcdir@/../change-namespace \
+ @abs_srcdir@/../clang-apply-replacements \
+ @abs_srcdir@/../clang-doc \
+ @abs_srcdir@/../clang-move \
+ @abs_srcdir@/../clang-query \
+ @abs_srcdir@/../clang-reorder-fields \
+ @abs_srcdir@/../clang-tidy \
+ @abs_srcdir@/../clangd \
+ @abs_srcdir@/../include-fixer \
+ @abs_srcdir@/../modularize \
+ @abs_srcdir@/../pp-trace \
+ @abs_srcdir@/../tool-template \
+ @abs_srcdir@/doxygen-mainpage.dox
# This tag can be used to specify the character encoding of the source files
# that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses
Index: clang-tools-extra/trunk/docs/Doxyfile
===
--- clang-tools-extra/trunk/docs/Doxyfile
+++ clang-tools-extra/trunk/docs/Doxyfile
@@ -1,1808 +0,0 @@
-# Doxyfile 1.7.6.1
-
-# This file describes the settings to be used by the documentation system
-# doxygen (www.doxygen.org) for a project
-#
-# All text after a hash (#) is considered a comment and will be ignored
-# The format is:
-# TAG = value [value, ...]
-# For lists items can also be appended using:
-# TAG += value [value, ...]
-# Values that contain spaces should be placed between quotes (" ")
-
-#---
-# Project related configuration options
-#---
-
-# This tag specifies the encoding used for all characters in the config file
-# that follow. The default is UTF-8 which is also the encoding used for all
-# text before the first occurrence of this tag. Doxygen uses libiconv (or the
-# iconv built into libc) for the transcoding. See
-# http://www.gnu.org/software/libiconv for the list of possible encodings.
-
-DOXYFILE_ENCODING = UTF-8
-
-# The PROJECT_NAME tag is a single word (or sequence of words) that should
-# identify the project. Note that if you do not use Doxywizard you need
-# to put quotes around the project name if it contains spaces.
-
-PROJECT_NAME = clang-tools-extra
-
-# The PROJECT_NUMBER tag can be used to enter a project or revision number.
-# This could be handy for archiving the generated documentation or
-# if some version control system is used.
-
-PROJECT_NUMBER =
-
-# Using the PROJECT_BRIEF tag one can provide an optional one line description
-# for a project that appears at the top of each page and should give viewer
-# a quick idea about the purpose of the project. Keep the description short.
-
-PROJECT_BRIEF =
-
-# With the PROJECT_LOGO tag one can specify an logo or icon that is
-# included in the documentation. The maximum height of the logo should not
-# exceed 55 pixels and the maximum width should not exceed 200 pixels.
-# Doxygen will copy the logo to the output directory.
-
-PROJECT_LOGO =
-
-# The OUTPUT_DIRECTORY tag is used to specify the (relative or absolute)
-# base path where the generated documentation will be put.
-# If a relative path is entered, it will be relative to the location
-# where doxygen was started. If left blank the current directory will be used.
-
-# Same directory that Sphinx uses.
-OUTPUT_DIRECTORY = ./_build/
-
-# If the CREATE_SUBDIRS tag is set to YES, then doxygen will create
-# 4096 sub-directories (in 2 levels) under the output directory of
[PATCH] D48928: [ms] Fix mangling of string literals used to initialize arrays larger or smaller than the literal
hans created this revision.
hans added reviewers: thakis, majnemer.
A Chromium developer reported a bug which turned out to be a mangling collision
between these two literals:
char s[] = "foo";
char t[32] = "foo";
They may look the same, but for the initialization of t we will (under some
circumstances) use a literal that's extended with zeros, and both the length
and those zeros should be accounted for by the mangling.
This actually makes the mangling code simpler: where it previously had special
logic for null terminators, which are not part of the StringLiteral, that is
now covered by the general algorithm.
https://reviews.llvm.org/D48928
Files:
lib/AST/MicrosoftMangle.cpp
test/CodeGen/mangle-ms-string-literals.c
Index: test/CodeGen/mangle-ms-string-literals.c
===
--- /dev/null
+++ test/CodeGen/mangle-ms-string-literals.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c -emit-llvm %s -o - -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -x c -emit-llvm %s -o - -triple=x86_64-pc-win32 | FileCheck %s
+
+void crbug857442(int x) {
+ // Make sure to handle truncated or padded literals. The truncation is only valid in C.
+ struct {int x; char s[2]; } truncatedAscii = {x, "hello"};
+ // CHECK: "??_C@_01CONKJJHI@he@"
+ struct {int x; char s[16]; } paddedAscii = {x, "hello"};
+ // CHECK: "??_C@_0BA@EAAINDNC@hello?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA?$AA@"
+}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -3171,7 +3171,7 @@
// ::= # the length of the literal
//
// ::= + @ # crc of the literal including
- // # null-terminator
+ // # trailing null bytes
//
// ::=# uninteresting character
// ::= '?$' # these two nibbles
@@ -3186,44 +3186,50 @@
MicrosoftCXXNameMangler Mangler(*this, Out);
Mangler.getStream() << "??_C@_";
+ // The actual string length might be different from that of the string literal
+ // in cases like:
+ // char foo[3] = "foobar";
+ // char bar[42] = "foobar";
+ // Where it is truncated or zero-padded to fit the array. This is the length
+ // used for mangling, and any trailing null-bytes also need to be mangled.
+ unsigned StringLength = getASTContext()
+ .getAsConstantArrayType(SL->getType())
+ ->getSize()
+ .getZExtValue();
+
// : The "kind" of string literal is encoded into the mangled name.
if (SL->isWide())
Mangler.getStream() << '1';
else
Mangler.getStream() << '0';
// : The next part of the mangled name consists of the length
- // of the string.
- // The StringLiteral does not consider the NUL terminator byte(s) but the
- // mangling does.
- // N.B. The length is in terms of bytes, not characters.
- Mangler.mangleNumber(SL->getByteLength() + SL->getCharByteWidth());
+ // of the string in bytes.
+ Mangler.mangleNumber(StringLength * SL->getCharByteWidth());
- auto GetLittleEndianByte = [&SL](unsigned Index) {
+ auto GetLittleEndianByte = [&SL, StringLength](unsigned Index) {
unsigned CharByteWidth = SL->getCharByteWidth();
+if (Index / CharByteWidth >= SL->getLength())
+ return static_cast(0);
uint32_t CodeUnit = SL->getCodeUnit(Index / CharByteWidth);
unsigned OffsetInCodeUnit = Index % CharByteWidth;
return static_cast((CodeUnit >> (8 * OffsetInCodeUnit)) & 0xff);
};
- auto GetBigEndianByte = [&SL](unsigned Index) {
+ auto GetBigEndianByte = [&SL, StringLength](unsigned Index) {
unsigned CharByteWidth = SL->getCharByteWidth();
+if (Index / CharByteWidth >= SL->getLength())
+ return static_cast(0);
uint32_t CodeUnit = SL->getCodeUnit(Index / CharByteWidth);
unsigned OffsetInCodeUnit = (CharByteWidth - 1) - (Index % CharByteWidth);
return static_cast((CodeUnit >> (8 * OffsetInCodeUnit)) & 0xff);
};
// CRC all the bytes of the StringLiteral.
llvm::JamCRC JC;
- for (unsigned I = 0, E = SL->getByteLength(); I != E; ++I)
+ for (unsigned I = 0, E = StringLength * SL->getCharByteWidth(); I != E; ++I)
JC.update(GetLittleEndianByte(I));
- // The NUL terminator byte(s) were not present earlier,
- // we need to manually process those bytes into the CRC.
- for (unsigned NullTerminator = 0; NullTerminator < SL->getCharByteWidth();
- ++NullTerminator)
-JC.update('\x00');
-
// : The CRC is encoded utilizing the standard number mangling
// scheme.
Mangler.mangleNumber(JC.getCRC());
@@ -3260,18 +3266,14 @@
// Enforce our 32 bytes max, except wchar_t which gets 32 chars instead.
unsigned MaxBytesToMangle = SL->isWide() ? 64U : 32U;
- unsigned NumBytesToMangle = std::min(MaxBytesToMangle, SL->g
[PATCH] D48628: [AST] Structural equivalence of methods
balazske added inline comments.
Comment at: lib/AST/ASTImporter.cpp:2454
+ if (IsStructuralMatch(D, FoundFunction)) {
+const FunctionDecl *Definition = nullptr;
+if (D->doesThisDeclarationHaveABody() &&
This change with `Definition` is needed to make the test
ImportOfEquivalentMethod work. But this is a general problem with importing, I
have a new test that fails and is not related to CXXMethodDecl. Add this new
(not directly related) test or remove this change and disable the failing test?
Repository:
rC Clang
https://reviews.llvm.org/D48628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48928: [ms] Fix mangling of string literals used to initialize arrays larger or smaller than the literal
hans added a comment. I couldn't get MSVC to create a symbol with padding at the end; they seem to always insert the padding with a separate loop during initialization, but this does show the truncated case: https://godbolt.org/g/B8ktA3 https://reviews.llvm.org/D48928 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336260 - [clangd] only ignore collected symbols if TU has uncompilable errors.
Author: ioeric
Date: Wed Jul 4 03:39:48 2018
New Revision: 336260
URL: http://llvm.org/viewvc/llvm-project?rev=336260&view=rev
Log:
[clangd] only ignore collected symbols if TU has uncompilable errors.
Modified:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Modified:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=336260&r1=336259&r2=336260&view=diff
==
---
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
(original)
+++
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Wed Jul 4 03:39:48 2018
@@ -84,9 +84,10 @@ public:
const auto &CI = getCompilerInstance();
if (CI.hasDiagnostics() &&
-(CI.getDiagnosticClient().getNumErrors() > 0)) {
- llvm::errs() << "Found errors in the translation unit. Igoring "
- "collected symbols...\n";
+CI.getDiagnostics().hasUncompilableErrorOccurred()) {
+ llvm::errs()
+ << "Found uncompilable errors in the translation unit. Igoring "
+ "collected symbols...\n";
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48342: [libcxx] Optimize vectors construction of trivial types from an iterator range with const-ness mismatch.
EricWF added a comment. Are there any tests which actually exercise the new behavior? Comment at: libcxx/include/memory:1665 +(is_same::type> >::value +|| is_same >::value +|| !__has_construct::value) && I'm not sure we should care about allocators for `T const`. The're all but an abomination. Comment at: libcxx/test/std/containers/sequences/vector/vector.cons/construct_iter_iter_different_value_type.pass.cpp:13 +// template vector(InputIter first, InputIter last); + +// Initialize a vector with a different value type. Make sure initialization Can this be folded into an existing test file for the constructor it's targeting? https://reviews.llvm.org/D48342 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.
CarlosAlbertoEnciso added a comment. In https://reviews.llvm.org/D48441#1151889, @ioeric wrote: > Hi Carlos, thanks for letting us know! I sent r336249 to fix the windows > test. Hi @ioeric, Thanks very much. Happy to help. Repository: rL LLVM https://reviews.llvm.org/D48441 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336261 - Add missing include for size_t
Author: ericwf
Date: Wed Jul 4 04:14:18 2018
New Revision: 336261
URL: http://llvm.org/viewvc/llvm-project?rev=336261&view=rev
Log:
Add missing include for size_t
Modified:
cfe/trunk/include/clang/Basic/Stack.h
Modified: cfe/trunk/include/clang/Basic/Stack.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Stack.h?rev=336261&r1=336260&r2=336261&view=diff
==
--- cfe/trunk/include/clang/Basic/Stack.h (original)
+++ cfe/trunk/include/clang/Basic/Stack.h Wed Jul 4 04:14:18 2018
@@ -15,6 +15,8 @@
#ifndef LLVM_CLANG_BASIC_STACK_H
#define LLVM_CLANG_BASIC_STACK_H
+#include
+
namespace clang {
/// The amount of stack space that Clang would like to be provided with.
/// If less than this much is available, we may be unable to reach our
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48314: [Frontend] Cache preamble-related data
yvvan added a comment. @ilya-biryukov Sorry. I didn't have time to post comments here. The usecase that we have is a supportive translation unit for code completion. Probably you use something similar in clangd not to wait for the TU to be reparsed after a change? The gain from this change is both performance and memory but I don't have measurements under my hand right now. And of course they are only valid for this usecase. https://reviews.llvm.org/D48314 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336262 - NFC - Fix typo in test/Layout/itanium-pack-and-align.cpp
Author: gbuella Date: Wed Jul 4 04:21:44 2018 New Revision: 336262 URL: http://llvm.org/viewvc/llvm-project?rev=336262&view=rev Log: NFC - Fix typo in test/Layout/itanium-pack-and-align.cpp Modified: cfe/trunk/test/Layout/itanium-pack-and-align.cpp Modified: cfe/trunk/test/Layout/itanium-pack-and-align.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/itanium-pack-and-align.cpp?rev=336262&r1=336261&r2=336262&view=diff == --- cfe/trunk/test/Layout/itanium-pack-and-align.cpp (original) +++ cfe/trunk/test/Layout/itanium-pack-and-align.cpp Wed Jul 4 04:21:44 2018 @@ -23,4 +23,4 @@ T t; // CHECK-NEXT: 0 | char x // CHECK-NEXT: 1 | int y // CHECK-NEXT:| [sizeof=8, dsize=8, align=8, -// CHECK-NETX:| nvsize=8, nvalign=8] +// CHECK-NEXT:| nvsize=8, nvalign=8] ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336263 - NFC - Fix typo in test/CodeGenObjC/gnustep2-class.m
Author: gbuella Date: Wed Jul 4 04:26:09 2018 New Revision: 336263 URL: http://llvm.org/viewvc/llvm-project?rev=336263&view=rev Log: NFC - Fix typo in test/CodeGenObjC/gnustep2-class.m Modified: cfe/trunk/test/CodeGenObjC/gnustep2-class.m Modified: cfe/trunk/test/CodeGenObjC/gnustep2-class.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/gnustep2-class.m?rev=336263&r1=336262&r2=336263&view=diff == --- cfe/trunk/test/CodeGenObjC/gnustep2-class.m (original) +++ cfe/trunk/test/CodeGenObjC/gnustep2-class.m Wed Jul 4 04:26:09 2018 @@ -51,5 +51,5 @@ // And check that we get a pointer to it in the right place // CHECK: @._OBJC_REF_CLASS_X = global // CHECK-SAME: @._OBJC_CLASS_X -// CHECK-SAMEsection "__objc_class_refs" +// CHECK-SAME: section "__objc_class_refs" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336264 - NFC - Fix type in builtins-ppc-p9vector.c test
Author: gbuella
Date: Wed Jul 4 04:29:21 2018
New Revision: 336264
URL: http://llvm.org/viewvc/llvm-project?rev=336264&view=rev
Log:
NFC - Fix type in builtins-ppc-p9vector.c test
Modified:
cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c
Modified: cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c?rev=336264&r1=336263&r2=336264&view=diff
==
--- cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c (original)
+++ cfe/trunk/test/CodeGen/builtins-ppc-p9vector.c Wed Jul 4 04:29:21 2018
@@ -983,7 +983,7 @@ vector bool int test86(void) {
}
vector bool long long test87(void) {
// CHECK-BE: @llvm.ppc.vsx.xvtstdcdp(<2 x double> {{.+}}, i32 127)
-// CHECK-BE_NEXT: ret <2 x i64
+// CHECK-BE-NEXT: ret <2 x i64>
// CHECK: @llvm.ppc.vsx.xvtstdcdp(<2 x double> {{.+}}, i32 127)
// CHECK-NEXT: ret <2 x i64>
return vec_test_data_class(vda, __VEC_CLASS_FP_NOT_NORMAL);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso updated this revision to Diff 154091.
CarlosAlbertoEnciso added a comment.
Update the patch to address the comments from the reviewers.
https://reviews.llvm.org/D46190
Files:
include/clang/Sema/Lookup.h
include/clang/Sema/Sema.h
include/clang/Sema/SemaInternal.h
lib/Sema/SemaCXXScopeSpec.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/SemaLookup.cpp
lib/Sema/SemaTemplate.cpp
test/PCH/cxx-templates.cpp
test/SemaCXX/referenced_alias_declaration_1.cpp
test/SemaCXX/referenced_alias_declaration_2.cpp
test/SemaCXX/referenced_using_all.cpp
test/SemaCXX/referenced_using_declaration_1.cpp
test/SemaCXX/referenced_using_declaration_2.cpp
test/SemaCXX/referenced_using_directive.cpp
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+ typedef int Integer;
+ int var;
+}
+
+void Fa() {
+ using namespace N; // Referenced
+ var = 1;
+}
+
+void Fb() {
+ using namespace N;
+ N::var = 1;
+}
+
+void Fc() {
+ using namespace N; // Referenced
+ Integer var = 1;
+}
+
+void Fd() {
+ using namespace N;
+ N::Integer var = 1;
+}
+
+//CHECK: |-FunctionDecl {{.*}} Fa 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fb 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'int' lvalue Var {{.*}} 'var' 'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: |-FunctionDecl {{.*}} Fc 'void ()'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-UsingDirectiveDecl {{.*}} referenced Namespace {{.*}} 'N'
+//CHECK-NEXT: | `-DeclStmt {{.*}}
+//CHECK-NEXT: | `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1
+//CHECK-NEXT: `-FunctionDecl {{.*}} Fd 'void ()'
+//CHECK-NEXT: `-CompoundStmt {{.*}}
+//CHECK-NEXT: |-DeclStmt {{.*}}
+//CHECK-NEXT: | `-UsingDirectiveDecl {{.*}} Namespace {{.*}} 'N'
+//CHECK-NEXT: `-DeclStmt {{.*}}
+//CHECK-NEXT: `-VarDecl {{.*}} var 'N::Integer':'int' cinit
+//CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 1
Index: test/SemaCXX/referenced_using_declaration_2.cpp
===
--- test/SemaCXX/referenced_using_declaration_2.cpp
+++ test/SemaCXX/referenced_using_declaration_2.cpp
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -std=c++11 -ast-dump %s | FileCheck %s --strict-whitespace
+
+namespace N {
+ typedef int Integer;
+ typedef char Char;
+}
+
+using N::Integer;
+using N::Char; // Referenced
+
+void Foo(int p1, N::Integer p2, Char p3) {
+ N::Integer var;
+ var = 0;
+}
+
+using N::Integer; // Referenced
+Integer Bar() {
+ using N::Char;
+ return 0;
+}
+
+//CHECK: |-UsingDecl {{.*}} N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Char
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Char' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Char'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'char'
+//CHECK-NEXT: |-FunctionDecl {{.*}} Foo 'void (int, N::Integer, N::Char)'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p1 'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p2 'N::Integer':'int'
+//CHECK-NEXT: | |-ParmVarDecl {{.*}} p3 'N::Char':'char'
+//CHECK-NEXT: | `-CompoundStmt {{.*}}
+//CHECK-NEXT: | |-DeclStmt {{.*}}
+//CHECK-NEXT: | | `-VarDecl {{.*}} used var 'N::Integer':'int'
+//CHECK-NEXT: | `-BinaryOperator {{.*}} 'N::Integer':'int' lvalue '='
+//CHECK-NEXT: | |-DeclRefExpr {{.*}} 'N::Integer':'int' lvalue Var {{.*}} 'var' 'N::Integer':'int'
+//CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 0
+//CHECK-NEXT: |-UsingDecl {{.*}} referenced N::Integer
+//CHECK-NEXT: |-UsingShadowDecl {{.*}} implicit referenced Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-TypedefType {{.*}} 'N::Integer' sugar
+//CHECK-NEXT: | |-Typedef {{.*}} 'Integer'
+//CHECK-NEXT: | `-BuiltinType {{.*}} 'int'
+//
[PATCH] D46190: For a used declaration, mark any associated usings as referenced.
CarlosAlbertoEnciso added inline comments.
Comment at: lib/Sema/Sema.cpp:1884-1885
+ ND = NA->getAliasedNamespace();
+ if (auto *NNS = NA->getQualifier())
+MarkNamespaceAliasReferenced(NNS->getAsNamespaceAlias());
+}
rsmith wrote:
> The loop and recursion here -- and indeed this whole function -- seem
> unnecessary.
>
> ```
> namespace A {} // #1
> namespace X {
> namespace B = A; // #2
> }
> namespace Y = X; // #3
> namespace C = Y::B; // #4
> ```
>
> Declaration `#4` should mark `#2` and `#3` referenced. So the only
> 'unreferenced namespace alias' warning we should get here is for `#4` -- and
> there is no need for marking `#4` as referenced to affect `#2` or `#3`.
The function and recursion is to be able to handle cases like:
```
namespace N {
int var;
}
void Foo() {
namespace NA = N;
namespace NB = NA;
NB::var = 4;
}
```
'var' is referenced and N, NA and NB should be marked as 'referenced' as well.
Comment at: lib/Sema/Sema.cpp:1890-1891
+
+/// \brief Mark as referenced any 'using declaration' that have introduced
+/// the given declaration in the current context.
+void Sema::MarkUsingReferenced(Decl *D, CXXScopeSpec *SS, Expr *E) {
rsmith wrote:
> Why is this ever appropriate? I would think we should just mark the named
> declaration from the relevant lookup as referenced in all cases.
My intention is to detect cases where the using statements do not affect other
declarations.
```
namespace N {
int var;
}
void Foo() {
using N::var;<- No Referenced
N::var = 1; <- Used
}
```
https://reviews.llvm.org/D46190
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48314: [Frontend] Cache preamble-related data
ilya-biryukov added a comment. I agree that the optimization is compelling, we do also share preamble in clangd and run code completion in parallel with other actions that need an AST. However, I would argue that a proper way to introduce the optimization would be to change interface of `ASTUnit` that would somehow allow reusing the PCHs between different `ASTUnit`s. Having the cache is fine, but I suggest we: 1. Make it explicit in the interface, e.g. `ASTUnit` ctor will accept an extra `PreambleCache` parameter. 2. Have it off by default. Whenever ASTUnit needs to rebuild a preamble, it can look into the cache and check there's a cached preamble and whether it matches the one that we're building. After the rebuild is finished, it will try to push the built preamble into the cache. ASTUnit can store the shared reference to a const preamble, i.e. `std::shared_ptr`. The cache itself will store the `weak_ptr`, thus allowing the preambles to be removed when they're not used anymore. It would allow to: 1. avoid global state in ASTUnit, the cache will have to be created somewhere up the stack and passed to ASTUnit explicitly. 2. avoid complicating ASTUnit with synchronization code, it can be moved to preamble cache. 3. keep changes to the ASTUnit minimal. The only function that needs to be changed is the one building the preamble. Given how complicated ASTUnit is already, I think that's a big win. https://reviews.llvm.org/D48314 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48933: [clangd] Treat class constructor as in the same scope as the class in ranking.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48933
Files:
clangd/Quality.cpp
unittests/clangd/QualityTests.cpp
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -21,6 +21,8 @@
#include "Quality.h"
#include "TestFS.h"
#include "TestTU.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/Support/Casting.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -199,6 +201,39 @@
EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
}
+TEST(QualityTests, NoBoostForClassConstructor) {
+ auto Header = TestTU::withHeaderCode(R"cpp(
+class Foo {
+public:
+ Foo(int);
+};
+ )cpp");
+ auto Symbols = Header.headerSymbols();
+ auto AST = Header.build();
+
+ const NamedDecl *Foo = &findDecl(AST, "Foo");
+ SymbolRelevanceSignals Cls;
+ Cls.merge(CodeCompletionResult(Foo, /*Priority=*/0));
+
+ // `findDecl` would return the implicit injected class for "Foo::Foo"; simply
+ // look for a constructor decl under the class.
+ const DeclContext *FooCtx = llvm::dyn_cast(Foo);
+ assert(FooCtx);
+ const NamedDecl *CtorDecl = nullptr;
+ for (const auto *D : FooCtx->decls())
+if (const auto *ND = llvm::dyn_cast(D))
+ if (llvm::isa(D)) {
+CtorDecl = ND;
+break;
+ }
+ assert(CtorDecl);
+ SymbolRelevanceSignals Ctor;
+ Ctor.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));
+
+ EXPECT_EQ(Cls.Scope, SymbolRelevanceSignals::GlobalScope);
+ EXPECT_EQ(Ctor.Scope, SymbolRelevanceSignals::GlobalScope);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -11,10 +11,12 @@
#include "URI.h"
#include "index/Index.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclVisitor.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
@@ -195,6 +197,9 @@
if (auto *R = dyn_cast_or_null(D))
if (R->isInjectedClassName())
DC = DC->getParent();
+ // Class constructor should have the same scope as the class.
+ if (const auto *Ctor = llvm::dyn_cast(D))
+DC = DC->getParent();
bool InClass = false;
for (; !DC->isFileContext(); DC = DC->getParent()) {
if (DC->isFunctionOrMethod())
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -21,6 +21,8 @@
#include "Quality.h"
#include "TestFS.h"
#include "TestTU.h"
+#include "clang/AST/DeclCXX.h"
+#include "llvm/Support/Casting.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -199,6 +201,39 @@
EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
}
+TEST(QualityTests, NoBoostForClassConstructor) {
+ auto Header = TestTU::withHeaderCode(R"cpp(
+class Foo {
+public:
+ Foo(int);
+};
+ )cpp");
+ auto Symbols = Header.headerSymbols();
+ auto AST = Header.build();
+
+ const NamedDecl *Foo = &findDecl(AST, "Foo");
+ SymbolRelevanceSignals Cls;
+ Cls.merge(CodeCompletionResult(Foo, /*Priority=*/0));
+
+ // `findDecl` would return the implicit injected class for "Foo::Foo"; simply
+ // look for a constructor decl under the class.
+ const DeclContext *FooCtx = llvm::dyn_cast(Foo);
+ assert(FooCtx);
+ const NamedDecl *CtorDecl = nullptr;
+ for (const auto *D : FooCtx->decls())
+if (const auto *ND = llvm::dyn_cast(D))
+ if (llvm::isa(D)) {
+CtorDecl = ND;
+break;
+ }
+ assert(CtorDecl);
+ SymbolRelevanceSignals Ctor;
+ Ctor.merge(CodeCompletionResult(CtorDecl, /*Priority=*/0));
+
+ EXPECT_EQ(Cls.Scope, SymbolRelevanceSignals::GlobalScope);
+ EXPECT_EQ(Ctor.Scope, SymbolRelevanceSignals::GlobalScope);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -11,10 +11,12 @@
#include "URI.h"
#include "index/Index.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclVisitor.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Sema/CodeCompleteConsumer.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"
@@ -195,6 +197,9 @@
if (auto *R = dyn_cast_or_null(D))
if (R->isIn
r336239 - [Sema] Consider all format_arg attributes.
Author: meinersbur
Date: Tue Jul 3 18:37:11 2018
New Revision: 336239
URL: http://llvm.org/viewvc/llvm-project?rev=336239&view=rev
Log:
[Sema] Consider all format_arg attributes.
If a function has multiple format_arg attributes, clang only considers
the first it finds (because AttributeLists are in reverse order, not
necessarily the textually first) and ignores all others.
Loop over all FormatArgAttr to print warnings for all declared
format_arg attributes.
For instance, libintl's ngettext (select plural or singular version of
format string) has two __format_arg__ attributes.
Differential Revision: https://reviews.llvm.org/D48734
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/attr-format_arg.c
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=336239&r1=336238&r2=336239&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Jul 3 18:37:11 2018
@@ -5518,13 +5518,22 @@ checkFormatStringExpr(Sema &S, const Exp
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND =
dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst = false;
+}
+ }
+ if (!IsFirst)
+return CommonResult;
+
+ if (const auto *FD = dyn_cast(ND)) {
unsigned BuiltinID = FD->getBuiltinID();
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
Modified: cfe/trunk/test/Sema/attr-format_arg.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-format_arg.c?rev=336239&r1=336238&r2=336239&view=diff
==
--- cfe/trunk/test/Sema/attr-format_arg.c (original)
+++ cfe/trunk/test/Sema/attr-format_arg.c Tue Jul 3 18:37:11 2018
@@ -4,10 +4,27 @@ int printf(const char *, ...);
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data
arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data
arguments}}
+
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47946: [ASTmporter] Fix infinite recursion on function import with struct definition in parameters
gerazo added a comment. @a.sidorin what do you think? https://reviews.llvm.org/D47946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336269 - [ASTImporter] import macro source locations
Author: r.stahl
Date: Wed Jul 4 06:34:05 2018
New Revision: 336269
URL: http://llvm.org/viewvc/llvm-project?rev=336269&view=rev
Log:
[ASTImporter] import macro source locations
Summary: Implement full import of macro expansion info with spelling and
expansion locations.
Reviewers: a.sidorin, klimek, martong, balazske, xazax.hun
Reviewed By: martong
Subscribers: thakis, xazax.hun, balazske, rnkovacs, cfe-commits
Differential Revision: https://reviews.llvm.org/D47698
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=336269&r1=336268&r2=336269&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Wed Jul 4 06:34:05 2018
@@ -7164,19 +7164,13 @@ SourceLocation ASTImporter::Import(Sourc
return {};
SourceManager &FromSM = FromContext.getSourceManager();
-
- // For now, map everything down to its file location, so that we
- // don't have to import macro expansions.
- // FIXME: Import macro expansions!
- FromLoc = FromSM.getFileLoc(FromLoc);
+
std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
- SourceManager &ToSM = ToContext.getSourceManager();
FileID ToFileID = Import(Decomposed.first);
if (ToFileID.isInvalid())
return {};
- SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
- .getLocWithOffset(Decomposed.second);
- return ret;
+ SourceManager &ToSM = ToContext.getSourceManager();
+ return ToSM.getComposedLoc(ToFileID, Decomposed.second);
}
SourceRange ASTImporter::Import(SourceRange FromRange) {
@@ -7184,41 +7178,56 @@ SourceRange ASTImporter::Import(SourceRa
}
FileID ASTImporter::Import(FileID FromID) {
- llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+ llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
if (Pos != ImportedFileIDs.end())
return Pos->second;
-
+
SourceManager &FromSM = FromContext.getSourceManager();
SourceManager &ToSM = ToContext.getSourceManager();
const SrcMgr::SLocEntry &FromSLoc = FromSM.getSLocEntry(FromID);
- assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-
- // Include location of this file.
- SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-
- // Map the FileID for to the "to" source manager.
+
+ // Map the FromID to the "to" source manager.
FileID ToID;
- const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
- if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-// FIXME: We probably want to use getVirtualFile(), so we don't hit the
-// disk again
-// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-// than mmap the files several times.
-const FileEntry *Entry =
ToFileManager.getFile(Cache->OrigEntry->getName());
-if (!Entry)
- return {};
-ToID = ToSM.createFileID(Entry, ToIncludeLoc,
- FromSLoc.getFile().getFileCharacteristic());
+ if (FromSLoc.isExpansion()) {
+const SrcMgr::ExpansionInfo &FromEx = FromSLoc.getExpansion();
+SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc());
+SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());
+unsigned TokenLen = FromSM.getFileIDSize(FromID);
+SourceLocation MLoc;
+if (FromEx.isMacroArgExpansion()) {
+ MLoc = ToSM.createMacroArgExpansionLoc(ToSpLoc, ToExLocS, TokenLen);
+} else {
+ SourceLocation ToExLocE = Import(FromEx.getExpansionLocEnd());
+ MLoc = ToSM.createExpansionLoc(ToSpLoc, ToExLocS, ToExLocE, TokenLen,
+ FromEx.isExpansionTokenRange());
+}
+ToID = ToSM.getFileID(MLoc);
} else {
-// FIXME: We want to re-use the existing MemoryBuffer!
-const llvm::MemoryBuffer *
-FromBuf = Cache->getBuffer(FromContext.getDiagnostics(), FromSM);
-std::unique_ptr ToBuf
- = llvm::MemoryBuffer::getMemBufferCopy(FromBuf->getBuffer(),
- FromBuf->getBufferIdentifier());
-ToID = ToSM.createFileID(std::move(ToBuf),
- FromSLoc.getFile().getFileCharacteristic());
+// Include location of this file.
+SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
+
+const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
+if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+ // FIXME: We probably want to use getVirtualFile(), so we don't hit the
+ // disk again
+ // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
+ // than mmap the files several times.
+ const FileEntry *Entry =
+ ToFileManager.getFile(Cache->OrigEntry->getName());
+ if (!Entry)
+return {};
+
[PATCH] D47698: [ASTImporter] import macro source locations
This revision was automatically updated to reflect the committed changes.
r.stahl marked an inline comment as done.
Closed by commit rC336269: [ASTImporter] import macro source locations
(authored by r.stahl, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47698?vs=152633&id=154103#toc
Repository:
rC Clang
https://reviews.llvm.org/D47698
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -7164,61 +7164,70 @@
return {};
SourceManager &FromSM = FromContext.getSourceManager();
-
- // For now, map everything down to its file location, so that we
- // don't have to import macro expansions.
- // FIXME: Import macro expansions!
- FromLoc = FromSM.getFileLoc(FromLoc);
+
std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
- SourceManager &ToSM = ToContext.getSourceManager();
FileID ToFileID = Import(Decomposed.first);
if (ToFileID.isInvalid())
return {};
- SourceLocation ret = ToSM.getLocForStartOfFile(ToFileID)
- .getLocWithOffset(Decomposed.second);
- return ret;
+ SourceManager &ToSM = ToContext.getSourceManager();
+ return ToSM.getComposedLoc(ToFileID, Decomposed.second);
}
SourceRange ASTImporter::Import(SourceRange FromRange) {
return SourceRange(Import(FromRange.getBegin()), Import(FromRange.getEnd()));
}
FileID ASTImporter::Import(FileID FromID) {
- llvm::DenseMap::iterator Pos
-= ImportedFileIDs.find(FromID);
+ llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
if (Pos != ImportedFileIDs.end())
return Pos->second;
-
+
SourceManager &FromSM = FromContext.getSourceManager();
SourceManager &ToSM = ToContext.getSourceManager();
const SrcMgr::SLocEntry &FromSLoc = FromSM.getSLocEntry(FromID);
- assert(FromSLoc.isFile() && "Cannot handle macro expansions yet");
-
- // Include location of this file.
- SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-
- // Map the FileID for to the "to" source manager.
+
+ // Map the FromID to the "to" source manager.
FileID ToID;
- const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
- if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-// FIXME: We probably want to use getVirtualFile(), so we don't hit the
-// disk again
-// FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-// than mmap the files several times.
-const FileEntry *Entry = ToFileManager.getFile(Cache->OrigEntry->getName());
-if (!Entry)
- return {};
-ToID = ToSM.createFileID(Entry, ToIncludeLoc,
- FromSLoc.getFile().getFileCharacteristic());
+ if (FromSLoc.isExpansion()) {
+const SrcMgr::ExpansionInfo &FromEx = FromSLoc.getExpansion();
+SourceLocation ToSpLoc = Import(FromEx.getSpellingLoc());
+SourceLocation ToExLocS = Import(FromEx.getExpansionLocStart());
+unsigned TokenLen = FromSM.getFileIDSize(FromID);
+SourceLocation MLoc;
+if (FromEx.isMacroArgExpansion()) {
+ MLoc = ToSM.createMacroArgExpansionLoc(ToSpLoc, ToExLocS, TokenLen);
+} else {
+ SourceLocation ToExLocE = Import(FromEx.getExpansionLocEnd());
+ MLoc = ToSM.createExpansionLoc(ToSpLoc, ToExLocS, ToExLocE, TokenLen,
+ FromEx.isExpansionTokenRange());
+}
+ToID = ToSM.getFileID(MLoc);
} else {
-// FIXME: We want to re-use the existing MemoryBuffer!
-const llvm::MemoryBuffer *
-FromBuf = Cache->getBuffer(FromContext.getDiagnostics(), FromSM);
-std::unique_ptr ToBuf
- = llvm::MemoryBuffer::getMemBufferCopy(FromBuf->getBuffer(),
- FromBuf->getBufferIdentifier());
-ToID = ToSM.createFileID(std::move(ToBuf),
- FromSLoc.getFile().getFileCharacteristic());
+// Include location of this file.
+SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
+
+const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
+if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+ // FIXME: We probably want to use getVirtualFile(), so we don't hit the
+ // disk again
+ // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
+ // than mmap the files several times.
+ const FileEntry *Entry =
+ ToFileManager.getFile(Cache->OrigEntry->getName());
+ if (!Entry)
+return {};
+ ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+ FromSLoc.getFile().getFileCharacteristic());
+} else {
+ // FIXME: We want to re-use the existing MemoryBuffer!
+ const llvm::MemoryBuffer *FromBuf =
+ Cache->getBuffer(FromContext.getDiagnostics(), FromSM);
+ std::unique_ptr ToBuf =
+ llvm
[PATCH] D48938: [clangd] Track origins of symbols (various indexes, Sema).
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ilya-biryukov.
Surface it in the completion items C++ API, and when a flag is set.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48938
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Merge.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/tool/ClangdMain.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/IndexTests.cpp
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -282,6 +282,8 @@
DetR.Documentation = "--doc--";
L.Detail = &DetL;
R.Detail = &DetR;
+ L.Origin = SymbolOrigin::Dynamic;
+ R.Origin = SymbolOrigin::Static;
Symbol::Details Scratch;
Symbol M = mergeSymbol(L, R, &Scratch);
@@ -293,6 +295,8 @@
ASSERT_TRUE(M.Detail);
EXPECT_EQ(M.Detail->ReturnType, "DetL");
EXPECT_EQ(M.Detail->Documentation, "--doc--");
+ EXPECT_EQ(M.Origin,
+SymbolOrigin::Dynamic | SymbolOrigin::Static | SymbolOrigin::Merge);
}
TEST(MergeTest, PreferSymbolWithDefn) {
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -59,6 +59,7 @@
}
MATCHER(InsertInclude, "") { return bool(arg.HeaderInsertion); }
MATCHER_P(SnippetSuffix, Text, "") { return arg.SnippetSuffix == Text; }
+MATCHER_P(Origin, OriginSet, "") { return arg.Origin == OriginSet; }
// Shorthand for Contains(Named(Name)).
Matcher &> Has(std::string Name) {
@@ -137,6 +138,7 @@
Sym.ID = SymbolID(USR);
Sym.SymInfo.Kind = Kind;
Sym.IsIndexedForCodeCompletion = true;
+ Sym.Origin = SymbolOrigin::Static;
return Sym;
}
Symbol func(StringRef Name) { // Assumes the function has no args.
@@ -511,9 +513,12 @@
)cpp",
{func("ns::both"), cls("ns::Index")});
// We get results from both index and sema, with no duplicates.
- EXPECT_THAT(
- Results.Completions,
- UnorderedElementsAre(Named("local"), Named("Index"), Named("both")));
+ EXPECT_THAT(Results.Completions,
+ UnorderedElementsAre(
+ AllOf(Named("local"), Origin(SymbolOrigin::AST)),
+ AllOf(Named("Index"), Origin(SymbolOrigin::Static)),
+ AllOf(Named("both"),
+Origin(SymbolOrigin::AST | SymbolOrigin::Static;
}
TEST(CompletionTest, SemaIndexMergeWithLimit) {
@@ -1252,6 +1257,8 @@
C.Header = "\"foo.h\"";
C.Kind = CompletionItemKind::Method;
C.Score.Total = 1.0;
+ C.Origin =
+ static_cast(SymbolOrigin::AST | SymbolOrigin::Static);
CodeCompleteOptions Opts;
Opts.IncludeIndicator.Insert = "^";
@@ -1278,6 +1285,10 @@
EXPECT_EQ(R.label, "^Foo::x(bool) const");
EXPECT_THAT(R.additionalTextEdits, Not(IsEmpty()));
+ Opts.ShowOrigins = true;
+ R = C.render(Opts);
+ EXPECT_EQ(R.label, "^[AS]Foo::x(bool) const");
+
C.BundleSize = 2;
R = C.render(Opts);
EXPECT_EQ(R.detail, "[2 overloads]\n\"foo.h\"");
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -143,6 +143,11 @@
"Clang uses an index built from symbols in opened files"),
llvm::cl::init(true));
+static llvm::cl::opt ShowOrigins(
+"debug-origin", llvm::cl::desc("Show origins of completion items"),
+llvm::cl::init(clangd::CodeCompleteOptions().ShowOrigins),
+llvm::cl::Hidden);
+
static llvm::cl::opt YamlSymbolFile(
"yaml-symbol-file",
llvm::cl::desc(
@@ -261,6 +266,7 @@
CCOpts.IncludeIneligibleResults = IncludeIneligibleResults;
CCOpts.Limit = LimitResults;
CCOpts.BundleOverloads = CompletionStyle != Detailed;
+ CCOpts.ShowOrigins = ShowOrigins;
// Initialize and run ClangdLSPServer.
ClangdLSPServer LSPServer(Out, CCOpts, CompileCommandsDirPath, Opts);
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -52,6 +52,8 @@
const CanonicalIncludes *Includes = nullptr;
// Populate the Symbol.References field.
bool CountReferences = false;
+// Every symbol collected will be stamped with this origin.
+SymbolOrigin Origin = SymbolOrigin::Unknown;
};
SymbolCollector(Options Opts);
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -411,6 +411,7 @@
Detail.IncludeHeader = Include;
[PATCH] D48940: [clangd] Wait for first preamble before code completion
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: jkorous, MaskRay, ioeric, javed.absar.
To avoid doing extra work of processing headers in the preamble
mutilple times in parallel.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48940
Files:
clangd/TUScheduler.cpp
clangd/TUScheduler.h
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -101,6 +101,10 @@
/// - validate that the preamble is still valid, and only use it in this case
/// - accept that preamble contents may be outdated, and try to avoid reading
/// source code from headers.
+ /// However, Action will be scheduled to run after the first rebuild of the
+ /// preamble for the corresponding file finishes. Note that the rebuild can
+ /// still fail, in which case Action will get a null pointer instead of the
+ /// preamble.
/// If an error occurs during processing, it is forwarded to the \p Action
/// callback.
void runWithPreamble(llvm::StringRef Name, PathRef File,
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -176,6 +176,12 @@
bool blockUntilIdle(Deadline Timeout) const;
std::shared_ptr getPossiblyStalePreamble() const;
+ /// Wait for the first build of preamble to finish. Preamble itself can be
+ /// accessed via getPossibleStalePreamble(). Note that this function will
+ /// return after an unsuccessful build of the preamble too, i.e. result of
+ /// getPossiblyStalePreamble() can be null even after this function returns.
+ void waitForFirstPreamble() const;
+
std::size_t getUsedBytes() const;
bool isASTCached() const;
@@ -226,6 +232,8 @@
/// Guards members used by both TUScheduler and the worker thread.
mutable std::mutex Mutex;
std::shared_ptr LastBuiltPreamble; /* GUARDED_BY(Mutex)
*/
+ /// Becomes ready when the first preamble build finishes.
+ Notification PreambleWasBuilt;
/// Set to true to signal run() to finish processing.
bool Done;/* GUARDED_BY(Mutex) */
std::deque Requests; /* GUARDED_BY(Mutex) */
@@ -329,6 +337,9 @@
buildCompilerInvocation(Inputs);
if (!Invocation) {
log("Could not build CompilerInvocation for file " + FileName);
+ // Make sure anyone waiting for the preamble gets notified it could not
+ // be built.
+ PreambleWasBuilt.notify();
return;
}
@@ -340,6 +351,8 @@
if (NewPreamble)
LastBuiltPreamble = NewPreamble;
}
+PreambleWasBuilt.notify();
+
// Build the AST for diagnostics.
llvm::Optional AST =
buildAST(FileName, std::move(Invocation), Inputs, NewPreamble, PCHs);
@@ -392,6 +405,10 @@
return LastBuiltPreamble;
}
+void ASTWorker::waitForFirstPreamble() const {
+ PreambleWasBuilt.wait();
+}
+
std::size_t ASTWorker::getUsedBytes() const {
// Note that we don't report the size of ASTs currently used for processing
// the in-flight requests. We used this information for debugging purposes
@@ -655,6 +672,11 @@
std::string Contents,
tooling::CompileCommand Command, Context Ctx,
decltype(Action) Action) mutable {
+// We don't want to be running preamble actions before the preamble was
+// built for the first time. This avoids extra work of processing the
+// preamble headers in parallel multiple times.
+Worker->waitForFirstPreamble();
+
std::lock_guard BarrierLock(Barrier);
WithContext Guard(std::move(Ctx));
trace::Span Tracer(Name);
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -101,6 +101,10 @@
/// - validate that the preamble is still valid, and only use it in this case
/// - accept that preamble contents may be outdated, and try to avoid reading
/// source code from headers.
+ /// However, Action will be scheduled to run after the first rebuild of the
+ /// preamble for the corresponding file finishes. Note that the rebuild can
+ /// still fail, in which case Action will get a null pointer instead of the
+ /// preamble.
/// If an error occurs during processing, it is forwarded to the \p Action
/// callback.
void runWithPreamble(llvm::StringRef Name, PathRef File,
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -176,6 +176,12 @@
bool blockUntilIdle(Deadline Timeout) const;
std::shared_ptr getPossiblyStalePreamble() const;
+ /// Wait for the first build of preamble to finish. Preamble itself can be
+ /// accessed via getPossibleStalePreamble(). Note that this function will
+ /// return after
[PATCH] D48938: [clangd] Track origins of symbols (various indexes, Sema).
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm. neat! Comment at: clangd/index/Index.cpp:50 +return OS << "unknown"; + static char Sigils[] = "ADSM4567"; + for (unsigned I = 0; I < sizeof(Sigils); ++I) nit: `constexpr`? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48938 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark added a comment. In https://reviews.llvm.org/D48903#1151847, @hokein wrote: > I'm not familiar with this part of code, but the change looks fine to me. I > think @bkramer is the right person to review it. > > Please make sure the style align with LLVM code style. Woops indeed I forgot to run `git clang-format`. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark added a comment. In https://reviews.llvm.org/D48903#1151866, @ilya-biryukov wrote: > Mimicing RealFS seems like the right thing to do here, so I would vouch for > checking this change in. > I'm a little worried that there are tests/clients relying on the old > behavior, have you run all the tests? I didn't have time yesterday, I'm doing this now. Is `ninja/make clang-test` sufficient? Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment.
Thanks for your review, NoQ!
Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:68
: IILockGuard(nullptr), IIUniqueLock(nullptr),
- LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
- FgetsFn("fgets"), ReadFn("read"), RecvFn("recv"),
- PthreadLockFn("pthread_mutex_lock"),
- PthreadTryLockFn("pthread_mutex_trylock"),
- PthreadUnlockFn("pthread_mutex_unlock"),
- MtxLock("mtx_lock"),
- MtxTimedLock("mtx_timedlock"),
- MtxTryLock("mtx_trylock"),
- MtxUnlock("mtx_unlock"),
+ LockFn({"lock"}), UnlockFn({"unlock"}), SleepFn({"sleep"}),
GetcFn({"getc"}),
+ FgetsFn({"fgets"}), ReadFn({"read"}), RecvFn({"recv"}),
NoQ wrote:
> I wish the old syntax for simple C functions was preserved.
>
> This could be accidentally achieved by using `ArrayRef` instead of
> `std::vector` for your constructor's argument.
`ArrayRef` can't achieve this goal. The only way I can figure out is
changing the declaration to the following form.
`CallDescription(StringRef FuncName, unsigned RequiredArgs = NoArgRequirement,
ArrayRef QualifiedName = None) {}`
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:273-280
+std::string Regex = "^::(.*)?";
+Regex += llvm::join(CD.QualifiedName, "(.*)?::(.*)?") + "(.*)?$";
+
+auto Matches = match(namedDecl(matchesName(Regex)).bind("Regex"), *ND,
+ LCtx->getAnalysisDeclContext()->getASTContext());
+
+if (Matches.empty())
NoQ wrote:
> Uhm, i think we don't need to flatten the class name to a string and then
> regex to do this.
>
> We can just unwrap the declaration and see if the newly unwrapped layer
> matches the expected name on every step, until all expected names have been
> found.
Is the way you mentioned above is similar `NamedDecl::printQualifiedName()`?
Get the full name through the `DeclContext` chain. See
https://github.com/llvm-mirror/clang/blob/master/lib/AST/Decl.cpp#L1511.
Or ignore namespace ,like `std`, just only consider the `CXXRecordDecl`? If so,
`dyn_cast<>` might be enough.
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] D48474: [analyzer][ctu] fix unsortable diagnostics
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336275: [analyzer][ctu] fix unsortable diagnostics (authored
by r.stahl, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48474?vs=152433&id=154107#toc
Repository:
rC Clang
https://reviews.llvm.org/D48474
Files:
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
test/Analysis/Inputs/ctu-other.cpp
test/Analysis/Inputs/externalFnMap.txt
test/Analysis/ctu-hdr.h
test/Analysis/ctu-main.cpp
Index: test/Analysis/ctu-hdr.h
===
--- test/Analysis/ctu-hdr.h
+++ test/Analysis/ctu-hdr.h
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();
Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -4,6 +4,8 @@
// RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze
-analyzer-checker=core,debug.ExprInspection -analyzer-config
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir
-verify %s
+#include "ctu-hdr.h"
+
void clang_analyzer_eval(int);
int f(int);
@@ -41,6 +43,7 @@
}
int fun_using_anon_struct(int);
+int other_macro_diag(int);
int main() {
clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@
clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
clang_analyzer_eval(fun_using_anon_struct(8) == 8); //
expected-warning{{TRUE}}
+
+ clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
+ // expected-warning@Inputs/ctu-other.cpp:75{{REACHABLE}}
+ MACRODIAG(); // expected-warning{{REACHABLE}}
}
Index: test/Analysis/Inputs/externalFnMap.txt
===
--- test/Analysis/Inputs/externalFnMap.txt
+++ test/Analysis/Inputs/externalFnMap.txt
@@ -12,3 +12,4 @@
c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+c:@F@other_macro_diag#I# ctu-other.cpp.ast
Index: test/Analysis/Inputs/ctu-other.cpp
===
--- test/Analysis/Inputs/ctu-other.cpp
+++ test/Analysis/Inputs/ctu-other.cpp
@@ -1,3 +1,5 @@
+#include "../ctu-hdr.h"
+
int callback_to_main(int x);
int f(int x) {
return x - 1;
@@ -68,3 +70,8 @@
typedef struct { int n; } Anonymous;
int fun_using_anon_struct(int n) { Anonymous anon; anon.n = n; return anon.n; }
+
+int other_macro_diag(int x) {
+ MACRODIAG();
+ return x;
+}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -406,11 +406,15 @@
std::pair InSameTU = SM.isInTheSameTranslationUnit(XOffs, YOffs);
if (InSameTU.first)
return XL.isBeforeInTranslationUnitThan(YL);
- const FileEntry *XFE = SM.getFileEntryForID(XL.getFileID());
- const FileEntry *YFE = SM.getFileEntryForID(YL.getFileID());
+ const FileEntry *XFE = SM.getFileEntryForID(XL.getSpellingLoc().getFileID());
+ const FileEntry *YFE = SM.getFileEntryForID(YL.getSpellingLoc().getFileID());
if (!XFE || !YFE)
return XFE && !YFE;
- return XFE->getName() < YFE->getName();
+ int NameCmp = XFE->getName().compare(YFE->getName());
+ if (NameCmp != 0)
+return NameCmp == -1;
+ // Last resort: Compare raw file IDs that are possibly expansions.
+ return XL.getFileID() < YL.getFileID();
}
static bool compare(const PathDiagnostic &X, const PathDiagnostic &Y) {
Index: test/Analysis/ctu-hdr.h
===
--- test/Analysis/ctu-hdr.h
+++ test/Analysis/ctu-hdr.h
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();
Index: test/Analysis/ctu-main.cpp
===
--- test/Analysis/ctu-main.cpp
+++ test/Analysis/ctu-main.cpp
@@ -4,6 +4,8 @@
// RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir -verify %s
+#include "ctu-hdr.h"
+
void clang_analyzer_eval(int);
int f(int);
@@ -41,6 +43,7 @@
}
int fun_using_anon_struct(int);
+int other_macro_diag(int);
int main() {
clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@
clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
clang_analyzer_eval(fun_using_anon_struct(8) == 8); // expected-warning{{TRUE}}
+
+ clang_analyzer_eval(other_macro_diag(1) == 1); // expected-warning{{TRUE}}
+ // e
r336275 - [analyzer][ctu] fix unsortable diagnostics
Author: r.stahl
Date: Wed Jul 4 07:12:58 2018
New Revision: 336275
URL: http://llvm.org/viewvc/llvm-project?rev=336275&view=rev
Log:
[analyzer][ctu] fix unsortable diagnostics
Summary: In the provided test case the PathDiagnostic compare function was not
able to find a difference.
Reviewers: xazax.hun, NoQ, dcoughlin, george.karpenkov
Reviewed By: george.karpenkov
Subscribers: a_sidorin, szepet, rnkovacs, a.sidorin, mikhail.ramalho,
cfe-commits
Differential Revision: https://reviews.llvm.org/D48474
Added:
cfe/trunk/test/Analysis/ctu-hdr.h
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
cfe/trunk/test/Analysis/Inputs/externalFnMap.txt
cfe/trunk/test/Analysis/ctu-main.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=336275&r1=336274&r2=336275&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Wed Jul 4 07:12:58
2018
@@ -406,11 +406,15 @@ static bool compareCrossTUSourceLocs(Ful
std::pair InSameTU = SM.isInTheSameTranslationUnit(XOffs, YOffs);
if (InSameTU.first)
return XL.isBeforeInTranslationUnitThan(YL);
- const FileEntry *XFE = SM.getFileEntryForID(XL.getFileID());
- const FileEntry *YFE = SM.getFileEntryForID(YL.getFileID());
+ const FileEntry *XFE = SM.getFileEntryForID(XL.getSpellingLoc().getFileID());
+ const FileEntry *YFE = SM.getFileEntryForID(YL.getSpellingLoc().getFileID());
if (!XFE || !YFE)
return XFE && !YFE;
- return XFE->getName() < YFE->getName();
+ int NameCmp = XFE->getName().compare(YFE->getName());
+ if (NameCmp != 0)
+return NameCmp == -1;
+ // Last resort: Compare raw file IDs that are possibly expansions.
+ return XL.getFileID() < YL.getFileID();
}
static bool compare(const PathDiagnostic &X, const PathDiagnostic &Y) {
Modified: cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/ctu-other.cpp?rev=336275&r1=336274&r2=336275&view=diff
==
--- cfe/trunk/test/Analysis/Inputs/ctu-other.cpp (original)
+++ cfe/trunk/test/Analysis/Inputs/ctu-other.cpp Wed Jul 4 07:12:58 2018
@@ -1,3 +1,5 @@
+#include "../ctu-hdr.h"
+
int callback_to_main(int x);
int f(int x) {
return x - 1;
@@ -68,3 +70,8 @@ int chf1(int x) {
typedef struct { int n; } Anonymous;
int fun_using_anon_struct(int n) { Anonymous anon; anon.n = n; return anon.n; }
+
+int other_macro_diag(int x) {
+ MACRODIAG();
+ return x;
+}
Modified: cfe/trunk/test/Analysis/Inputs/externalFnMap.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/externalFnMap.txt?rev=336275&r1=336274&r2=336275&view=diff
==
--- cfe/trunk/test/Analysis/Inputs/externalFnMap.txt (original)
+++ cfe/trunk/test/Analysis/Inputs/externalFnMap.txt Wed Jul 4 07:12:58 2018
@@ -12,3 +12,4 @@ c:@F@h_chain#I# ctu-chain.cpp.ast
c:@N@chns@S@chcls@F@chf4#I# ctu-chain.cpp.ast
c:@N@chns@F@chf2#I# ctu-chain.cpp.ast
c:@F@fun_using_anon_struct#I# ctu-other.cpp.ast
+c:@F@other_macro_diag#I# ctu-other.cpp.ast
Added: cfe/trunk/test/Analysis/ctu-hdr.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctu-hdr.h?rev=336275&view=auto
==
--- cfe/trunk/test/Analysis/ctu-hdr.h (added)
+++ cfe/trunk/test/Analysis/ctu-hdr.h Wed Jul 4 07:12:58 2018
@@ -0,0 +1,3 @@
+#define MACRODIAG() clang_analyzer_warnIfReached()
+
+void clang_analyzer_warnIfReached();
Modified: cfe/trunk/test/Analysis/ctu-main.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ctu-main.cpp?rev=336275&r1=336274&r2=336275&view=diff
==
--- cfe/trunk/test/Analysis/ctu-main.cpp (original)
+++ cfe/trunk/test/Analysis/ctu-main.cpp Wed Jul 4 07:12:58 2018
@@ -4,6 +4,8 @@
// RUN: cp %S/Inputs/externalFnMap.txt %T/ctudir/
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze
-analyzer-checker=core,debug.ExprInspection -analyzer-config
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir
-verify %s
+#include "ctu-hdr.h"
+
void clang_analyzer_eval(int);
int f(int);
@@ -41,6 +43,7 @@ int chf1(int x);
}
int fun_using_anon_struct(int);
+int other_macro_diag(int);
int main() {
clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
@@ -58,4 +61,8 @@ int main() {
clang_analyzer_eval(chns::chf1(4) == 12); // expected-warning{{TRUE}}
clang_analyzer_eval(fun_using_anon_struct(8) == 8); //
expected-warning{{TRUE}}
+
+ clang_analy
Re: r335800 - [analyzer] Add support for pre-C++17 copy elision.
We've started seeing assertion failures after this commit. assert.h assertion failed at llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:485 in static clang::ento::ProgramStateRef clang::ento::ExprEngine::elideDestructor(clang::ento::ProgramStateRef, const clang::CXXBindTemporaryExpr *, const clang::LocationContext *): !State->contains(I) The stack trace is clang::ento::ExprEngine::elideDestructor clang::ento::ExprEngine::prepareForObjectConstruction clang::ento::ExprEngine::prepareForObjectConstruction clang::ento::ExprEngine::VisitCXXConstructExpr clang::ento::ExprEngine::Visit clang::ento::ExprEngine::ProcessStmt clang::ento::ExprEngine::processCFGElement clang::ento::CoreEngine::HandlePostStmt clang::ento::CoreEngine::dispatchWorkItem clang::ento::CoreEngine::ExecuteWorkList clang::ento::ExprEngine::ExecuteWorkList ::AnalysisConsumer::ActionExprEngine ::AnalysisConsumer::HandleCode ::AnalysisConsumer::HandleDeclsCallGraph ::AnalysisConsumer::runAnalysisOnTranslationUnit ::AnalysisConsumer::HandleTranslationUnit I haven't come up with a test case yet. On Thu, Jun 28, 2018 at 2:34 AM Artem Dergachev via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: dergachev > Date: Wed Jun 27 17:30:18 2018 > New Revision: 335800 > > URL: http://llvm.org/viewvc/llvm-project?rev=335800&view=rev > Log: > [analyzer] Add support for pre-C++17 copy elision. > > r335795 adds copy elision information to CFG. This commit allows static > analyzer > to elide elidable copy constructors by constructing the objects that were > previously subject to elidable copy directly in the target region of the > copy. > > The chain of elided constructors may potentially be indefinitely long. This > only happens when the object is being returned from a function which in > turn is > returned from another function, etc. > > NRVO is not supported yet. > > Differential Revision: https://reviews.llvm.org/D47671 > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp > cfe/trunk/test/Analysis/cxx17-mandatory-elision.cpp > cfe/trunk/test/Analysis/gtest.cpp > cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp > cfe/trunk/test/Analysis/lifetime-extension.cpp > cfe/trunk/test/Analysis/temporaries.cpp > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=335800&r1=335799&r2=335800&view=diff > > == > --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > (original) > +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > Wed Jun 27 17:30:18 2018 > @@ -780,9 +780,30 @@ private: >llvm::PointerUnion P, >const LocationContext *LC); > > + /// If the given expression corresponds to a temporary that was used for > + /// passing into an elidable copy/move constructor and that constructor > + /// was actually elided, track that we also need to elide the > destructor. > + static ProgramStateRef elideDestructor(ProgramStateRef State, > + const CXXBindTemporaryExpr *BTE, > + const LocationContext *LC); > + > + /// Stop tracking the destructor that corresponds to an elided > constructor. > + static ProgramStateRef > + cleanupElidedDestructor(ProgramStateRef State, > + const CXXBindTemporaryExpr *BTE, > + const LocationContext *LC); > + > + /// Returns true if the given expression corresponds to a temporary that > + /// was constructed for passing into an elidable copy/move constructor > + /// and that constructor was actually elided. > + static bool isDestructorElided(ProgramStateRef State, > + const CXXBindTemporaryExpr *BTE, > + const LocationContext *LC); > + >/// Check if all objects under construction have been fully constructed >/// for the given context range (including FromLC, not including ToLC). > - /// This is useful for assertions. > + /// This is useful for assertions. Also checks if elided destructors > + /// were cleaned up. >static bool areAllObjectsFullyConstructed(ProgramStateRef State, > const LocationContext *FromLC, > const LocationContext *ToLC); > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=335800&r1=335799&r2=335800&view=diff > > == > --- cfe/trunk/lib/StaticAnalyzer/Core/Exp
Re: r335795 - [CFG] [analyzer] Add construction contexts that explain pre-C++17 copy elision.
We've started seeing an assertion failure after this commit: assert.h assertion failed at llvm/tools/clang/lib/Analysis/CFG.cpp:1266 in void (anonymous namespace)::CFGBuilder::findConstructionContexts(const clang::ConstructionContextLayer *, clang::Stmt *): CE->getNumArgs() == 1 The stack trace is: ::CFGBuilder::findConstructionContexts ::CFGBuilder::VisitReturnStmt ::CFGBuilder::Visit ::CFGBuilder::addStmt ::CFGBuilder::VisitCompoundStmt ::CFGBuilder::Visit ::CFGBuilder::addStmt ::CFGBuilder::buildCFG clang::CFG::buildCFG clang::AnalysisDeclContext::getCFG clang::ento::AnalysisManager::getCFG ::AnalysisConsumer::HandleCode clang::RecursiveASTVisitor::TraverseDecl clang::RecursiveASTVisitor::TraverseDeclContextHelper clang::RecursiveASTVisitor::TraverseDecl clang::RecursiveASTVisitor::TraverseDeclContextHelper clang::RecursiveASTVisitor::TraverseDecl ::AnalysisConsumer::runAnalysisOnTranslationUnit ::AnalysisConsumer::HandleTranslationUnit No test case yet. On Thu, Jun 28, 2018 at 2:09 AM Artem Dergachev via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: dergachev > Date: Wed Jun 27 17:04:54 2018 > New Revision: 335795 > > URL: http://llvm.org/viewvc/llvm-project?rev=335795&view=rev > Log: > [CFG] [analyzer] Add construction contexts that explain pre-C++17 copy > elision. > > Before C++17 copy elision was optional, even if the elidable copy/move > constructor had arbitrary side effects. The elidable constructor is present > in the AST, but marked as elidable. > > In these cases CFG now contains additional information that allows its > clients > to figure out if a temporary object is only being constructed so that to > pass > it to an elidable constructor. If so, it includes a reference to the > elidable > constructor's construction context, so that the client could elide the > elidable constructor and construct the object directly at its final > destination. > > Differential Revision: https://reviews.llvm.org/D47616 > > Modified: > cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h > cfe/trunk/include/clang/Analysis/CFG.h > cfe/trunk/include/clang/Analysis/ConstructionContext.h > cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h > cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp > cfe/trunk/lib/Analysis/CFG.cpp > cfe/trunk/lib/Analysis/ConstructionContext.cpp > cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp > cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp > cfe/trunk/test/Analysis/analyzer-config.c > cfe/trunk/test/Analysis/analyzer-config.cpp > cfe/trunk/test/Analysis/cfg-rich-constructors.cpp > cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp > > Modified: cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h?rev=335795&r1=335794&r2=335795&view=diff > > == > --- cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h (original) > +++ cfe/trunk/include/clang/Analysis/AnalysisDeclContext.h Wed Jun 27 > 17:04:54 2018 > @@ -451,6 +451,7 @@ public: > bool addStaticInitBranches = false, > bool addCXXNewAllocator = true, > bool addRichCXXConstructors = true, > + bool markElidedCXXConstructors = true, > CodeInjector *injector = nullptr); > >AnalysisDeclContext *getContext(const Decl *D); > > Modified: cfe/trunk/include/clang/Analysis/CFG.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=335795&r1=335794&r2=335795&view=diff > > == > --- cfe/trunk/include/clang/Analysis/CFG.h (original) > +++ cfe/trunk/include/clang/Analysis/CFG.h Wed Jun 27 17:04:54 2018 > @@ -1025,6 +1025,7 @@ public: > bool AddCXXNewAllocator = false; > bool AddCXXDefaultInitExprInCtors = false; > bool AddRichCXXConstructors = false; > +bool MarkElidedCXXConstructors = false; > > BuildOptions() = default; > > > Modified: cfe/trunk/include/clang/Analysis/ConstructionContext.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ConstructionContext.h?rev=335795&r1=335794&r2=335795&view=diff > > == > --- cfe/trunk/include/clang/Analysis/ConstructionContext.h (original) > +++ cfe/trunk/include/clang/Analysis/ConstructionContext.h Wed Jun 27 > 17:04:54 2018 > @@ -107,7 +107,10 @@ public: > INITIALIZER_BEGIN = SimpleConstructorInitializerKind, > INITIALIZER_END = CXX17ElidedCopyConstructorInitializerKind, > NewAllocatedObjectKind, > -TemporaryObjectKind, > +SimpleTemporaryObjectKind, > +ElidedTemporaryObjectKi
[PATCH] D48044: [Power9] Update fp128 as a valid homogenous aggregate base type
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.
Other than a few style nits that can be fixed on the commit, this LGTM.
Comment at: include/clang/AST/Type.h:1802
bool isFloat16Type() const; // C11 extension ISO/IEC TS 18661
+ bool isFloat128Type() const;
bool isRealType() const; // C99 6.2.5p17 (real floating + integer)
// IEEE 754 binary128
Comment at: lib/CodeGen/TargetInfo.cpp:4609
// Homogeneous aggregates for ELFv2 must have base types of float,
// double, long double, or 128-bit vectors.
if (const BuiltinType *BT = Ty->getAs()) {
This comment should probably be updated.
Comment at: lib/CodeGen/TargetInfo.cpp:4633
uint32_t NumRegs =
- Base->isVectorType() ? 1 : (getContext().getTypeSize(Base) + 63) / 64;
+ ((getContext().getTargetInfo().hasFloat128Type() &&
+ Base->isFloat128Type()) ||
This expression looks very messy, I think it's probably better to rewrite it as
multiple expressions or an `if` statement.
Comment at: test/CodeGen/ppc64le-f128Aggregates.c:19
+
+struct fp2a2b { __float128 a[2]; __float128 b[2]; };
+
Is it still a homogeneous aggregate if it's nested?
i.e. `struct fp10 { struct fp2 a, struct fp3 b };`
And if so, should we add that to the test?
https://reviews.llvm.org/D48044
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48941: [ASTImporter] import FunctionDecl end locations
r.stahl created this revision.
r.stahl added reviewers: martong, a.sidorin, balazske, xazax.hun.
Herald added subscribers: cfe-commits, rnkovacs.
On constructors that do not take the end source location, it was not imported.
Fixes test from https://reviews.llvm.org/D47698 /
https://reviews.llvm.org/rC336269.
Repository:
rC Clang
https://reviews.llvm.org/D48941
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
FromSM);
}
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
Decl *FromTU = getTuDecl(
R"(
#define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2533,6 +2533,7 @@
D->isInlineSpecified(),
D->isImplicit(),
D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
SmallVector CtorInitializers;
for (auto *I : FromConstructor->inits()) {
@@ -2555,6 +2556,7 @@
NameInfo, T, TInfo,
D->isInlineSpecified(),
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
} else if (auto *FromConversion = dyn_cast(D)) {
ToFunction = CXXConversionDecl::Create(Importer.getToContext(),
cast(DC),
@@ -2580,6 +2582,7 @@
D->isInlineSpecified(),
D->hasWrittenPrototype(),
D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
}
// Import the qualifier, if any.
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1620,7 +1620,7 @@
FromSM);
}
-TEST_P(ASTImporterTestBase, DISABLED_ImportNestedMacro) {
+TEST_P(ASTImporterTestBase, ImportNestedMacro) {
Decl *FromTU = getTuDecl(
R"(
#define FUNC_INT void declToImport
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2533,6 +2533,7 @@
D->isInlineSpecified(),
D->isImplicit(),
D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
if (unsigned NumInitializers = FromConstructor->getNumCtorInitializers()) {
SmallVector CtorInitializers;
for (auto *I : FromConstructor->inits()) {
@@ -2555,6 +2556,7 @@
NameInfo, T, TInfo,
D->isInlineSpecified(),
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
} else if (auto *FromConversion = dyn_cast(D)) {
ToFunction = CXXConversionDecl::Create(Importer.getToContext(),
cast(DC),
@@ -2580,6 +2582,7 @@
D->isInlineSpecified(),
D->hasWrittenPrototype(),
D->isConstexpr());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
}
// Import the qualifier, if any.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48941: [ASTImporter] import FunctionDecl end locations
r.stahl added a comment. In https://reviews.llvm.org/D47698#1141871, @r.stahl wrote: > improved code quality; added nested macro test. it "works", but is disabled > because it revealed another bug: the function end location is not imported. > will send a patch Related to this. Repository: rC Clang https://reviews.llvm.org/D48941 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r336283 - [clang-tidy] Fix http://llvm.org/PR38055
Author: alexfh
Date: Wed Jul 4 08:19:49 2018
New Revision: 336283
URL: http://llvm.org/viewvc/llvm-project?rev=336283&view=rev
Log:
[clang-tidy] Fix http://llvm.org/PR38055
Modified:
clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp?rev=336283&r1=336282&r2=336283&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedParametersCheck.cpp Wed Jul
4 08:19:49 2018
@@ -159,8 +159,9 @@ void UnusedParametersCheck::warnOnUnused
MyDiag << removeParameter(Result, FD, ParamIndex);
// Fix all call sites.
- for (const auto *Call : Indexer->getFnCalls(Function))
-MyDiag << removeArgument(Result, Call, ParamIndex);
+ for (const CallExpr *Call : Indexer->getFnCalls(Function))
+if (ParamIndex < Call->getNumArgs()) // See PR38055 for example.
+ MyDiag << removeArgument(Result, Call, ParamIndex);
}
void UnusedParametersCheck::check(const MatchFinder::MatchResult &Result) {
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp?rev=336283&r1=336282&r2=336283&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-parameters.cpp Wed Jul
4 08:19:49 2018
@@ -222,6 +222,21 @@ static Function dontGe
return Function();
}
+namespace PR38055 {
+namespace {
+struct a {
+ void b(int c) {;}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: parameter 'c' is unused
+// CHECK-FIXES: {{^}} void b() {;}{{$}}
+};
+template
+class d {
+ a e;
+ void f() { e.b(); }
+};
+} // namespace
+} // namespace PR38055
+
namespace strict_mode_off {
// Do not warn on empty function bodies.
void f1(int foo1) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48942: [PCH] Add an option to not write comments into PCH
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added a subscriber: ioeric.
Will be used in clangd, see the follow-up change.
Clangd does not use comments read from PCH to avoid crashes due to
changed contents of the file. However, reading them considerably slows
down code completion on files with large preambles.
Repository:
rC Clang
https://reviews.llvm.org/D48942
Files:
include/clang/Lex/PreprocessorOptions.h
lib/Serialization/ASTWriter.cpp
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -78,6 +78,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
@@ -3252,6 +3253,9 @@
void ASTWriter::WriteComments() {
Stream.EnterSubblock(COMMENTS_BLOCK_ID, 3);
+ auto _ = llvm::make_scope_exit([this] { Stream.ExitBlock(); });
+ if (!PP->getPreprocessorOpts().WriteCommentListToPCH)
+return;
ArrayRef RawComments = Context->Comments.getComments();
RecordData Record;
for (const auto *I : RawComments) {
@@ -3262,7 +3266,6 @@
Record.push_back(I->isAlmostTrailingComment());
Stream.EmitRecord(COMMENTS_RAW_COMMENT, Record);
}
- Stream.ExitBlock();
}
//===--===//
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -88,6 +88,11 @@
/// processing the rest of the file.
bool GeneratePreamble = false;
+ /// Whether to write comment locations into the PCH when building it.
+ /// Reading the comments from the PCH can be a performance hit even if the
+ /// clients don't use them.
+ bool WriteCommentListToPCH = true;
+
/// The implicit PTH input included at the start of the translation unit, or
/// empty.
std::string ImplicitPTHInclude;
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -78,6 +78,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/PointerIntPair.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
@@ -3252,6 +3253,9 @@
void ASTWriter::WriteComments() {
Stream.EnterSubblock(COMMENTS_BLOCK_ID, 3);
+ auto _ = llvm::make_scope_exit([this] { Stream.ExitBlock(); });
+ if (!PP->getPreprocessorOpts().WriteCommentListToPCH)
+return;
ArrayRef RawComments = Context->Comments.getComments();
RecordData Record;
for (const auto *I : RawComments) {
@@ -3262,7 +3266,6 @@
Record.push_back(I->isAlmostTrailingComment());
Stream.EmitRecord(COMMENTS_RAW_COMMENT, Record);
}
- Stream.ExitBlock();
}
//===--===//
Index: include/clang/Lex/PreprocessorOptions.h
===
--- include/clang/Lex/PreprocessorOptions.h
+++ include/clang/Lex/PreprocessorOptions.h
@@ -88,6 +88,11 @@
/// processing the rest of the file.
bool GeneratePreamble = false;
+ /// Whether to write comment locations into the PCH when building it.
+ /// Reading the comments from the PCH can be a performance hit even if the
+ /// clients don't use them.
+ bool WriteCommentListToPCH = true;
+
/// The implicit PTH input included at the start of the translation unit, or
/// empty.
std::string ImplicitPTHInclude;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48943: [clangd] Do not write comments into Preamble PCH
ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: jkorous, MaskRay, ioeric.
To avoid wasting time deserializing them on code completion and
further reparses.
We do not use the comments anyway, because we cannot rely on the file
contents staying the same for reparses that reuse the prebuilt
preamble PCH.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48943
Files:
clangd/ClangdUnit.cpp
clangd/CodeCompletionStrings.cpp
Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -32,34 +32,6 @@
}
}
-bool canRequestComment(const ASTContext &Ctx, const NamedDecl &D,
- bool CommentsFromHeaders) {
- if (CommentsFromHeaders)
-return true;
- auto &SourceMgr = Ctx.getSourceManager();
- // Accessing comments for decls from invalid preamble can lead to crashes.
- // So we only return comments from the main file when doing code completion.
- // For indexing, we still read all the comments.
- // FIXME: find a better fix, e.g. store file contents in the preamble or get
- // doc comments from the index.
- auto canRequestForDecl = [&](const NamedDecl &D) -> bool {
-for (auto *Redecl : D.redecls()) {
- auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
- if (!SourceMgr.isWrittenInMainFile(Loc))
-return false;
-}
-return true;
- };
- // First, check the decl itself.
- if (!canRequestForDecl(D))
-return false;
- // Completion also returns comments for properties, corresponding to ObjC
- // methods.
- const ObjCMethodDecl *M = dyn_cast(&D);
- const ObjCPropertyDecl *PDecl = M ? M->findPropertyDecl() : nullptr;
- return !PDecl || canRequestForDecl(*PDecl);
-}
-
bool LooksLikeDocComment(llvm::StringRef CommentText) {
// We don't report comments that only contain "special" chars.
// This avoids reporting various delimiters, like:
@@ -87,12 +59,13 @@
// the comments for namespaces.
return "";
}
- if (!canRequestComment(Ctx, *Decl, CommentsFromHeaders))
-return "";
-
const RawComment *RC = getCompletionComment(Ctx, Decl);
if (!RC)
return "";
+
+ // Sanity check that the comment does not come from the PCH. We choose to not
+ // write them into PCH, because they are racy and slow to load.
+ assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getLocStart()));
std::string Doc = RC->getFormattedText(Ctx.getSourceManager(),
Ctx.getDiagnostics());
if (!LooksLikeDocComment(Doc))
return "";
@@ -104,11 +77,14 @@
const CodeCompleteConsumer::OverloadCandidate &Result,
unsigned ArgIndex, bool CommentsFromHeaders) {
auto *Func = Result.getFunction();
- if (!Func || !canRequestComment(Ctx, *Func, CommentsFromHeaders))
+ if (!Func)
return "";
const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
if (!RC)
return "";
+ // Sanity check that the comment does not come from the PCH. We choose to not
+ // write them into PCH, because they are racy and slow to load.
+ assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getLocStart()));
std::string Doc = RC->getFormattedText(Ctx.getSourceManager(),
Ctx.getDiagnostics());
if (!LooksLikeDocComment(Doc))
return "";
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -24,6 +24,7 @@
#include "clang/Lex/Lexer.h"
#include "clang/Lex/MacroInfo.h"
#include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
#include "clang/Sema/Sema.h"
#include "clang/Serialization/ASTWriter.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -323,6 +324,9 @@
// the preamble and make it smaller.
assert(!CI.getFrontendOpts().SkipFunctionBodies);
CI.getFrontendOpts().SkipFunctionBodies = true;
+ // We don't want to write comments into PCH. They are racy and slow to read
+ // back. We rely on dynamic index for the comments instead.
+ CI.getPreprocessorOpts().WriteCommentListToPCH = false;
CppFilePreambleCallbacks SerializedDeclsCollector(FileName,
PreambleCallback);
if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -32,34 +32,6 @@
}
}
-bool canRequestComment(const ASTContext &Ctx, const NamedDecl &D,
- bool CommentsFromHeaders) {
- if (CommentsFromHeaders)
-return true;
- auto &SourceMgr = Ctx.getSourceManager();
- // Accessing comments for decls from invalid preamble can lead to crashes.
- // So we only return comments from the main file when doing code completion.
- // For indexing, we sti
[PATCH] D48942: [PCH] Add an option to not write comments into PCH
ilya-biryukov added a comment. Not sure about two things: - Using PreprocessorOptions for plumbing this setting. I'd rather put it into FrontendOptions, but there seems to be no way to get those in the ASTWriter... Happy to search for the alternatives - Lack of tests. I'm not sure how to properly test it in clang. I've added asserts in clangd, see the follow-up https://reviews.llvm.org/D48943. But happy to look into writing actual tests if that looks important. Also not sure if someone else should look at this change, suggestions for reviewers are very welcome! Repository: rC Clang https://reviews.llvm.org/D48942 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark updated this revision to Diff 154118.
simark added a comment.
- Fixed formatting (ran git-clang-format)
- Fixed expectation in TEST_F(InMemoryFileSystemTest, WorkingDirectory)
- Added test TEST_F(InMemoryFileSystemTest, StatusName)
Repository:
rC Clang
https://reviews.llvm.org/D48903
Files:
lib/Basic/VirtualFileSystem.cpp
unittests/Basic/VirtualFileSystemTest.cpp
Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -794,7 +794,7 @@
auto Stat = FS.status("/b/c");
ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
- ASSERT_EQ("c", Stat->getName());
+ ASSERT_EQ("/b/c", Stat->getName());
ASSERT_EQ("/b", *FS.getCurrentWorkingDirectory());
Stat = FS.status("c");
@@ -919,6 +919,30 @@
ASSERT_TRUE(Stat->isRegularFile());
}
+// Test that the name returned by status() is in the same form as the path that
+// was requested.
+TEST_F(InMemoryFileSystemTest, StatusName) {
+ NormalizedFS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"),
+ /*User=*/None,
+ /*Group=*/None, sys::fs::file_type::regular_file);
+ NormalizedFS.setCurrentWorkingDirectory("/a/b");
+
+ auto Stat = NormalizedFS.status("../b/c");
+ ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"
+<< NormalizedFS.toString();
+ ASSERT_TRUE(Stat->isRegularFile());
+ ASSERT_EQ("../b/c", Stat->getName());
+
+ auto File = NormalizedFS.openFileForRead("../b/c");
+ ASSERT_FALSE(File.getError()) << File.getError() << "\n"
+<< NormalizedFS.toString();
+ Stat = (*File)->status();
+ ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"
+<< NormalizedFS.toString();
+ ASSERT_TRUE(Stat->isRegularFile());
+ ASSERT_EQ("../b/c", Stat->getName());
+}
+
// NOTE: in the tests below, we use '//root/' as our root directory, since it
is
// a legal *absolute* path on Windows as well as *nix.
class VFSFromYAMLTest : public ::testing::Test {
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -508,10 +508,17 @@
class InMemoryFileAdaptor : public File {
InMemoryFile &Node;
+ // The name to use when returning a Status for this file.
+ std::string RequestedName;
+
public:
- explicit InMemoryFileAdaptor(InMemoryFile &Node) : Node(Node) {}
+ explicit InMemoryFileAdaptor(InMemoryFile &Node, std::string RequestedName)
+ : Node(Node), RequestedName(std::move(RequestedName)) {}
- llvm::ErrorOr status() override { return Node.getStatus(); }
+ llvm::ErrorOr status() override {
+Status St = Node.getStatus();
+return Status::copyWithNewName(St, RequestedName);
+ }
llvm::ErrorOr>
getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
@@ -710,8 +717,10 @@
llvm::ErrorOr InMemoryFileSystem::status(const Twine &Path) {
auto Node = lookupInMemoryNode(*this, Root.get(), Path);
- if (Node)
-return (*Node)->getStatus();
+ if (Node) {
+Status St = (*Node)->getStatus();
+return Status::copyWithNewName(St, Path.str());
+ }
return Node.getError();
}
@@ -724,7 +733,8 @@
// When we have a file provide a heap-allocated wrapper for the memory buffer
// to match the ownership semantics for File.
if (auto *F = dyn_cast(*Node))
-return std::unique_ptr(new detail::InMemoryFileAdaptor(*F));
+return std::unique_ptr(
+new detail::InMemoryFileAdaptor(*F, Path.str()));
// FIXME: errc::not_a_file?
return make_error_code(llvm::errc::invalid_argument);
Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -794,7 +794,7 @@
auto Stat = FS.status("/b/c");
ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n" << FS.toString();
- ASSERT_EQ("c", Stat->getName());
+ ASSERT_EQ("/b/c", Stat->getName());
ASSERT_EQ("/b", *FS.getCurrentWorkingDirectory());
Stat = FS.status("c");
@@ -919,6 +919,30 @@
ASSERT_TRUE(Stat->isRegularFile());
}
+// Test that the name returned by status() is in the same form as the path that
+// was requested.
+TEST_F(InMemoryFileSystemTest, StatusName) {
+ NormalizedFS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"),
+ /*User=*/None,
+ /*Group=*/None, sys::fs::file_type::regular_file);
+ NormalizedFS.setCurrentWorkingDirectory("/a/b");
+
+ auto Stat = NormalizedFS.status("../b/c");
+ ASSERT_FALSE(Stat.getError()) << Stat.getError() << "\n"
+<< NormalizedFS.toString();
+ ASSERT_TRUE(Stat->isRegularFile());
+ ASSERT
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark added a comment. I ran `ninja clang-test`: Testing Time: 1720.20s Expected Passes: 12472 Expected Failures : 18 Unsupported Tests : 263 Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48941: [ASTImporter] import FunctionDecl end locations
martong added inline comments.
Comment at: lib/AST/ASTImporter.cpp:2559
D->isImplicit());
+ToFunction->setRangeEnd(Importer.Import(D->getLocEnd()));
} else if (auto *FromConversion = dyn_cast(D)) {
Why don't we need to call the `setRangeEnd()` when e.g. we create a
`CXXMethodDecl` or a `CXXConversionDecl`? Couldn't we just call this after all
these branches (at line 2587)?
Repository:
rC Clang
https://reviews.llvm.org/D48941
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48866: [clang-tidy] Add incorrect-pointer-cast checker
alexfh added a comment. Some patterns are covered by compiler diagnostics: https://godbolt.org/g/HvsjnP. Is there any benefit in re-implementing them? Comment at: clang-tidy/misc/IncorrectPointerCastCheck.cpp:60 +diag(CastExpr->getLocStart(), + "Do not use C-style cast from %0 to %1! " + "Different type layout. Struct members are incompatible") The style of the message differs from that of the other checks (and Clang warnings). Messages are not complete sentences and as such should not use capitalization, trailing periods etc. Also I would avoid imperatives and exclamation marks. The message should use neutral tone and state what exactly is wrong with the code and why. Comment at: test/clang-tidy/misc-incorrect-pointer-cast.cpp:109 +} \ No newline at end of file "No newline at end of file". Please fix. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48866 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
hfinkel added a comment. In https://reviews.llvm.org/D48721#1150677, @bjope wrote: > In https://reviews.llvm.org/D48721#1150361, @hfinkel wrote: > > > In https://reviews.llvm.org/D48721#1150333, @bjope wrote: > > > > > Is the fault that the metadata only should be put on the back edge, not > > > the branch in the preheader? > > > > > > Yea. Our past thinking has been that any backedge in the loop is valid. The > > metadata shouldn't end up other places, although it's benign unless those > > other places are (or may later become) a backedge for some different loop. > > > I'm no expert on clang. The patch seems to fix the problem if the goal is to > only add the loop-metadata on the backedge , but I'll leave it to someone > else to approve it. > > I'm a little bit concerned about opt not detecting this kind of problems > though. > Would it be possible for some verifier to detect if we have loop metadata on > some branch that aren't in the latch block? > And/or should the optimization that "merges" two branches with different > loop metadata), be smarter about which loop metadata to keep? Or maybe we > should be defensive and discard loop metadata on the merged branch > instruction? We could add this to the verifier. We have transformation passes that aren't explicitly loop aware, and so the question is would those passes now need to do something special to remain valid in the presence of this metadata. As a general rule, metadata in invalid locations is simply ignored. It clearly is a problem, if metadata is moving from one valid location to an unrelated valid location. Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48946: [Preamble] Check system dependencies in preamble too
ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
PrecompiledPreamble hasn't checked the system dependencies changed
before. This result in invalid preamble not being rebuilt if headers
that changed were found in -isystem include paths.
This pattern is sometimes used to avoid showing warnings in third
party code, so we want to correctly handle those cases.
Tested in clangd, see the follow-up patch.
Repository:
rC Clang
https://reviews.llvm.org/D48946
Files:
lib/Frontend/PrecompiledPreamble.cpp
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -63,6 +63,15 @@
return Overlay;
}
+class PreambleDependencyCollector : public DependencyCollector {
+public:
+ // We want to collect all dependencies for correctness. Avoiding the real
+ // system dependencies (e.g. stl from /usr/lib) would probably be a good idea,
+ // but there is no way to distinguish between those and the ones that can be
+ // spuriously added by '-isystem' (e.g. to avoid errors from those headers).
+ bool needSystemDependencies() override { return true; }
+};
+
/// Keeps a track of files to be deleted in destructor.
class TemporaryFiles {
public:
@@ -311,7 +320,7 @@
Clang->setSourceManager(
new SourceManager(Diagnostics, Clang->getFileManager()));
- auto PreambleDepCollector = std::make_shared();
+ auto PreambleDepCollector = std::make_shared();
Clang->addDependencyCollector(PreambleDepCollector);
// Remap the main source file to the preamble buffer.
Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -63,6 +63,15 @@
return Overlay;
}
+class PreambleDependencyCollector : public DependencyCollector {
+public:
+ // We want to collect all dependencies for correctness. Avoiding the real
+ // system dependencies (e.g. stl from /usr/lib) would probably be a good idea,
+ // but there is no way to distinguish between those and the ones that can be
+ // spuriously added by '-isystem' (e.g. to avoid errors from those headers).
+ bool needSystemDependencies() override { return true; }
+};
+
/// Keeps a track of files to be deleted in destructor.
class TemporaryFiles {
public:
@@ -311,7 +320,7 @@
Clang->setSourceManager(
new SourceManager(Diagnostics, Clang->getFileManager()));
- auto PreambleDepCollector = std::make_shared();
+ auto PreambleDepCollector = std::make_shared();
Clang->addDependencyCollector(PreambleDepCollector);
// Remap the main source file to the preamble buffer.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48947: [clangd] Added a test for preambles and -isystem
ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric.
Herald added subscribers: jkorous, MaskRay.
Checks that preambles are properly invalidated when headers from
-isystem paths change.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48947
Files:
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -34,13 +34,16 @@
namespace clangd {
using ::testing::ElementsAre;
+using ::testing::Contains;
using ::testing::Eq;
using ::testing::Gt;
using ::testing::IsEmpty;
using ::testing::Pair;
using ::testing::UnorderedElementsAre;
namespace {
+// FIXME: This is copied from CodeCompleteTests.cpp. Share the code instead.
+MATCHER_P(Named, Name, "") { return arg.Name == Name; }
bool diagsContainErrors(const std::vector &Diagnostics) {
for (auto D : Diagnostics) {
@@ -927,6 +930,40 @@
EXPECT_EQ(Expected, *Changed);
}
+TEST_F(ClangdVFSTest, ChangedHeaderFromISystem) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ auto SourcePath = testPath("source/foo.cpp");
+ auto HeaderPath = testPath("headers/foo.h");
+ FS.Files[HeaderPath] = "struct X { int bar; };";
+ Annotations Code(R"cpp(
+#include "foo.h"
+
+int main() {
+ X().ba^
+})cpp");
+ CDB.ExtraClangFlags.push_back("-xc++");
+ CDB.ExtraClangFlags.push_back("-isystem" + testPath("headers"));
+
+ runAddDocument(Server, SourcePath, Code.code());
+ auto Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+clangd::CodeCompleteOptions()))
+ .Completions;
+ EXPECT_THAT(Completions, ElementsAre(Named("bar")));
+ // Update the header and rerun addDocument to make sure we get the updated
+ // files.
+ FS.Files[HeaderPath] = "struct X { int bar; int baz; };";
+ runAddDocument(Server, SourcePath, Code.code());
+ Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+clangd::CodeCompleteOptions()))
+ .Completions;
+ // We want to make sure we see the updated version.
+ EXPECT_THAT(Completions, ElementsAre(Named("bar"), Named("baz")));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -34,13 +34,16 @@
namespace clangd {
using ::testing::ElementsAre;
+using ::testing::Contains;
using ::testing::Eq;
using ::testing::Gt;
using ::testing::IsEmpty;
using ::testing::Pair;
using ::testing::UnorderedElementsAre;
namespace {
+// FIXME: This is copied from CodeCompleteTests.cpp. Share the code instead.
+MATCHER_P(Named, Name, "") { return arg.Name == Name; }
bool diagsContainErrors(const std::vector &Diagnostics) {
for (auto D : Diagnostics) {
@@ -927,6 +930,40 @@
EXPECT_EQ(Expected, *Changed);
}
+TEST_F(ClangdVFSTest, ChangedHeaderFromISystem) {
+ MockFSProvider FS;
+ ErrorCheckingDiagConsumer DiagConsumer;
+ MockCompilationDatabase CDB;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ auto SourcePath = testPath("source/foo.cpp");
+ auto HeaderPath = testPath("headers/foo.h");
+ FS.Files[HeaderPath] = "struct X { int bar; };";
+ Annotations Code(R"cpp(
+#include "foo.h"
+
+int main() {
+ X().ba^
+})cpp");
+ CDB.ExtraClangFlags.push_back("-xc++");
+ CDB.ExtraClangFlags.push_back("-isystem" + testPath("headers"));
+
+ runAddDocument(Server, SourcePath, Code.code());
+ auto Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+clangd::CodeCompleteOptions()))
+ .Completions;
+ EXPECT_THAT(Completions, ElementsAre(Named("bar")));
+ // Update the header and rerun addDocument to make sure we get the updated
+ // files.
+ FS.Files[HeaderPath] = "struct X { int bar; int baz; };";
+ runAddDocument(Server, SourcePath, Code.code());
+ Completions = cantFail(runCodeComplete(Server, SourcePath, Code.point(),
+clangd::CodeCompleteOptions()))
+ .Completions;
+ // We want to make sure we see the updated version.
+ EXPECT_THAT(Completions, ElementsAre(Named("bar"), Named("baz")));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
hfinkel added a comment. In https://reviews.llvm.org/D48721#1152023, @deepak2427 wrote: > I encountered the issue while working with the unroller and found that it was > not following the pragma info, and traced it back to the issue with metadata. > As far as I understood, for for-loops and while-loops, we add the metadata > only to the loop back-edge. So it would make sense to keep them consistent. > I'm not an expert in clang, and do not know how we can detect such problems. The code change is likely okay. We need to have the tests updated. With rare exception, we don't have end-to-end tests in Clang. We test Clang's CodeGen independently, and so Clang's CodeGen tests shouldn't run the optimizer. Please write tests that directly check the expected output of Clang (without running the optimizer). If you look at other tests in the CodeGen directory, you should see what I mean. If you have any questions, please feel free to ask. Thanks! Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
ilya-biryukov added a comment. I usually run `ninja check-clang check-clang-tools` for clang changes. Have never used `clang-test`, not sure what it does. I ran it with this change, found a few failures from clang-move: Failing Tests (5): Extra Tools Unit Tests :: clang-move/./ClangMoveTests/ClangMove.AddDependentNewHeader Extra Tools Unit Tests :: clang-move/./ClangMoveTests/ClangMove.AddDependentOldHeader Extra Tools Unit Tests :: clang-move/./ClangMoveTests/ClangMove.DontMoveAll Extra Tools Unit Tests :: clang-move/./ClangMoveTests/ClangMove.MoveHeaderAndCC Extra Tools Unit Tests :: clang-move/./ClangMoveTests/ClangMove.MoveHeaderOnly Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark added a comment. In https://reviews.llvm.org/D48903#1152485, @ilya-biryukov wrote: > I usually run `ninja check-clang check-clang-tools` for clang changes. Have > never used `clang-test`, not sure what it does. I think `clang-test` is an alias for `check-clang`. > I ran it with this change, found a few failures from clang-move: > > Failing Tests (5): > Extra Tools Unit Tests :: > clang-move/./ClangMoveTests/ClangMove.AddDependentNewHeader > Extra Tools Unit Tests :: > clang-move/./ClangMoveTests/ClangMove.AddDependentOldHeader > Extra Tools Unit Tests :: > clang-move/./ClangMoveTests/ClangMove.DontMoveAll > Extra Tools Unit Tests :: > clang-move/./ClangMoveTests/ClangMove.MoveHeaderAndCC > Extra Tools Unit Tests :: > clang-move/./ClangMoveTests/ClangMove.MoveHeaderOnly Doh, I'll run `check-clang-tools` too. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44143: [clang-tidy] Create properly seeded random generator check
boga95 added a comment. How can I commit it? Comment at: docs/clang-tidy/checks/cert-msc51-cpp.rst:7 +This check flags all pseudo-random number engines, engine adaptor +instantiations and `srand()` when initialized or seeded with default argument, +constant expression or any user-configurable type. Pseudo-random number aaron.ballman wrote: > Please add double backticks around `srand()` instead of single backticks. Should I use double backticks in release notes too? https://reviews.llvm.org/D44143 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun added a comment. In https://reviews.llvm.org/D48027#1142324, @MTC wrote: > - There is possible match the wrong AST node, e.g. for the NamedDecl `foo()`, > if the function body has the `a::b::x`, when we match `a::b::X()`, we will > match `foo()` . Because I'm not familiar with ASTMatcher, my bad, I can't > think of a perfect way to match only the specified nodes. Hmm, instead of using the match function which will traverse the ast, we could use the `HasNameMatcher` class which can be invoked on only one node. That class is in an internal namespace though, so I think the best would be to ask the maintainers whether it is ok to use that class externally, or were whe should put the functionality we need. 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] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size
mstorsjo added a comment. Ping @joerg https://reviews.llvm.org/D38680 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45179: [libc++] Add _LIBCPP_FORCE_NODISCARD define to force-enable nodiscard in pre-C++17
lebedev.ri added a comment. Ping? Repository: rL LLVM https://reviews.llvm.org/D45179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r336297 - Remove old workaround that is no longer needed
Author: ericwf
Date: Wed Jul 4 13:16:05 2018
New Revision: 336297
URL: http://llvm.org/viewvc/llvm-project?rev=336297&view=rev
Log:
Remove old workaround that is no longer needed
Modified:
libcxx/trunk/utils/libcxx/test/config.py
Modified: libcxx/trunk/utils/libcxx/test/config.py
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/utils/libcxx/test/config.py?rev=336297&r1=336296&r2=336297&view=diff
==
--- libcxx/trunk/utils/libcxx/test/config.py (original)
+++ libcxx/trunk/utils/libcxx/test/config.py Wed Jul 4 13:16:05 2018
@@ -933,9 +933,6 @@ class Configuration(object):
# FIXME: Enable the two warnings below.
self.cxx.addWarningFlagIfSupported('-Wno-conversion')
self.cxx.addWarningFlagIfSupported('-Wno-unused-local-typedef')
-# FIXME: Remove this warning once the min/max handling patch lands
-# See https://reviews.llvm.org/D33080
-self.cxx.addWarningFlagIfSupported('-Wno-#warnings')
std = self.get_lit_conf('std', None)
if std in ['c++98', 'c++03']:
# The '#define static_assert' provided by libc++ in C++03 mode
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48951: [clang-move] ClangMoveTests: Remove dots in output paths
simark created this revision.
Herald added subscribers: cfe-commits, ioeric.
Following https://reviews.llvm.org/D48903 ([VirtualFileSystem]
InMemoryFileSystem::status: Return
a Status with the requested name), the paths output by clang-move in the
FileToReplacements map may contain leading "./". For example, where we
would get "foo.h", we'll now get "./foo.h". This breaks the tests,
because we are doing exact string lookups in the FileToFileID and
Results maps (they contain "foo.h", but we search for "./foo.h").
To mitigate this, try to normalize a little bit the paths output by
clang-move to remove that leading "./".
This patch should be safe to merge before https://reviews.llvm.org/D48903,
remove_dots will just
be a no-op.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48951
Files:
unittests/clang-move/ClangMoveTests.cpp
Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -239,8 +239,10 @@
// The Key is file name, value is the new code after moving the class.
std::map Results;
for (const auto &It : FileToReplacements) {
-StringRef FilePath = It.first;
-Results[FilePath] = Context.getRewrittenText(FileToFileID[FilePath]);
+// The path may come out as "./foo.h", normalize to "foo.h".
+SmallString<32> FilePath (It.first);
+llvm::sys::path::remove_dots(FilePath);
+Results[FilePath.str().str()] =
Context.getRewrittenText(FileToFileID[FilePath]);
}
return Results;
}
Index: unittests/clang-move/ClangMoveTests.cpp
===
--- unittests/clang-move/ClangMoveTests.cpp
+++ unittests/clang-move/ClangMoveTests.cpp
@@ -239,8 +239,10 @@
// The Key is file name, value is the new code after moving the class.
std::map Results;
for (const auto &It : FileToReplacements) {
-StringRef FilePath = It.first;
-Results[FilePath] = Context.getRewrittenText(FileToFileID[FilePath]);
+// The path may come out as "./foo.h", normalize to "foo.h".
+SmallString<32> FilePath (It.first);
+llvm::sys::path::remove_dots(FilePath);
+Results[FilePath.str().str()] = Context.getRewrittenText(FileToFileID[FilePath]);
}
return Results;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48903: [VirtualFileSystem] InMemoryFileSystem::status: Return a Status with the requested name
simark added a comment. I opened https://reviews.llvm.org/D48951 to fix the failures in clang-move. Repository: rC Clang https://reviews.llvm.org/D48903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types
a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Nice! Repository: rC Clang https://reviews.llvm.org/D48773 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
