[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.
ebevhan added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:89
SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
-StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
+StateMgr(stateMgr), ArrayIndexTy(context.getSignedSizeType()),
ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
a.sidorin wrote:
> As you correctly pointed, ssize_t is 32-bit on 32-bit systems. Therefore, it
> is too short. So, we can leave this line as-is.
But if it's hardcoded to LongLongTy, you have the same problem on 64-bit
systems.
https://reviews.llvm.org/D46944
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333160 - [X86] NFC Include immintrin.h in CodeGen tests
Author: gbuella
Date: Thu May 24 00:09:08 2018
New Revision: 333160
URL: http://llvm.org/viewvc/llvm-project?rev=333160&view=rev
Log:
[X86] NFC Include immintrin.h in CodeGen tests
Following r333110:
"Move all Intel defined intrinsic includes into immintrin.h"
Modified:
cfe/trunk/test/CodeGen/adx-builtins.c
cfe/trunk/test/CodeGen/avx-builtins.c
cfe/trunk/test/CodeGen/avx2-builtins.c
cfe/trunk/test/CodeGen/bmi-builtins.c
cfe/trunk/test/CodeGen/bmi2-builtins.c
cfe/trunk/test/CodeGen/builtin-clflushopt.c
cfe/trunk/test/CodeGen/builtin-clwb.c
cfe/trunk/test/CodeGen/builtin-movdir.c
cfe/trunk/test/CodeGen/builtin-wbnoinvd.c
cfe/trunk/test/CodeGen/cldemote.c
cfe/trunk/test/CodeGen/f16c-builtins.c
cfe/trunk/test/CodeGen/fsgsbase-builtins.c
cfe/trunk/test/CodeGen/lzcnt-builtins.c
cfe/trunk/test/CodeGen/mmx-builtins.c
cfe/trunk/test/CodeGen/popcnt-builtins.c
cfe/trunk/test/CodeGen/ptwrite.c
cfe/trunk/test/CodeGen/rdpid-builtins.c
cfe/trunk/test/CodeGen/rdrand-builtins.c
cfe/trunk/test/CodeGen/sse-builtins.c
cfe/trunk/test/CodeGen/sse2-builtins.c
cfe/trunk/test/CodeGen/sse3-builtins.c
cfe/trunk/test/CodeGen/sse41-builtins.c
cfe/trunk/test/CodeGen/sse42-builtins.c
cfe/trunk/test/CodeGen/ssse3-builtins.c
cfe/trunk/test/CodeGen/waitpkg.c
cfe/trunk/test/CodeGen/x86-nontemporal.c
cfe/trunk/test/Headers/pconfigintin.c
cfe/trunk/test/Headers/sgxintrin.c
Modified: cfe/trunk/test/CodeGen/adx-builtins.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/adx-builtins.c?rev=333160&r1=333159&r2=333160&view=diff
==
--- cfe/trunk/test/CodeGen/adx-builtins.c (original)
+++ cfe/trunk/test/CodeGen/adx-builtins.c Thu May 24 00:09:08 2018
@@ -1,6 +1,6 @@
// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ffreestanding
-target-feature +adx -emit-llvm -o - %s | FileCheck %s
-#include
+#include
unsigned char test_addcarryx_u32(unsigned char __cf, unsigned int __x,
unsigned int __y, unsigned int *__p) {
Modified: cfe/trunk/test/CodeGen/avx-builtins.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx-builtins.c?rev=333160&r1=333159&r2=333160&view=diff
==
--- cfe/trunk/test/CodeGen/avx-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx-builtins.c Thu May 24 00:09:08 2018
@@ -2,7 +2,7 @@
// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin
-target-feature +avx -fno-signed-char -emit-llvm -o - -Wall -Werror | FileCheck
%s
-#include
+#include
// NOTE: This should match the tests in
llvm/test/CodeGen/X86/sse-intrinsics-fast-isel.ll
Modified: cfe/trunk/test/CodeGen/avx2-builtins.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx2-builtins.c?rev=333160&r1=333159&r2=333160&view=diff
==
--- cfe/trunk/test/CodeGen/avx2-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx2-builtins.c Thu May 24 00:09:08 2018
@@ -2,7 +2,7 @@
// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin
-target-feature +avx2 -fno-signed-char -emit-llvm -o - -Wall -Werror |
FileCheck %s
-#include
+#include
// NOTE: This should match the tests in
llvm/test/CodeGen/X86/avx2-intrinsics-fast-isel.ll
Modified: cfe/trunk/test/CodeGen/bmi-builtins.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/bmi-builtins.c?rev=333160&r1=333159&r2=333160&view=diff
==
--- cfe/trunk/test/CodeGen/bmi-builtins.c (original)
+++ cfe/trunk/test/CodeGen/bmi-builtins.c Thu May 24 00:09:08 2018
@@ -1,7 +1,7 @@
// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin
-target-feature +bmi -emit-llvm -o - -Wall -Werror | FileCheck %s
-#include
+#include
// NOTE: This should match the tests in
llvm/test/CodeGen/X86/bmi-intrinsics-fast-isel.ll
Modified: cfe/trunk/test/CodeGen/bmi2-builtins.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/bmi2-builtins.c?rev=333160&r1=333159&r2=333160&view=diff
==
--- cfe/trunk/test/CodeGen/bmi2-builtins.c (original)
+++ cfe/trunk/test/CodeGen/bmi2-builtins.c Thu May 24 00:09:08 2018
@@ -2,7 +2,7 @@
// RUN: %clang_cc1 -ffreestanding %s -triple=i386-apple-darwin -target-feature
+bmi2 -emit-llvm -o - | FileCheck %s --check-prefix=B32
-#include
+#include
unsigned int test_bzhi_u32(unsigned int __X, unsigned int __Y) {
// CHECK: @llvm.x86.bmi.bzhi.32
Modified: cfe/trunk/test/CodeGen/builtin-clflushopt.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtin-clflushopt.c?rev=333160&r1=333159&r2=333160&view=diff
[PATCH] D47067: Update NRVO logic to support early return
Quuxplusone added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:12760
// to deduce an implicit return type.
- if (FD->getReturnType()->isRecordType() &&
- (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+ if (!FD->getReturnType()->isScalarType())
computeNRVO(Body, getCurFunction());
rsmith wrote:
> tzik wrote:
> > Quuxplusone wrote:
> > > What is the purpose of this change?
> > > If it's "no functional change" it should be done separately IMHO; if it
> > > is supposed to change codegen, then it needs some codegen tests. (From
> > > looking at the code: maybe this affects codegen on functions that return
> > > member function pointers by value?)
> > I think the previous implementation was incorrect.
> > Though computeNRVO clears ReturnStmt::NRVOCandidate when the corresponding
> > variable is not NRVO variable, CGStmt checks both of
> > ReturnStmt::NRVOCandidate and VarDecl::NRVOVariable anyway.
> > So, computeNRVO took no effect in the previous code, and the absence of
> > computeNRVO here on function templates did not matter.
> > Note that there was no chance to call computeNRVO on function template
> > elsewhere too.
> >
> > OTOH in the new implementation, computeNRVO is necessary, and its absence
> > on function templates matters.
> >
> > We can remove computeNRVO here separately as a NFC patch and readd the new
> > impl to the same place, but it's probably less readable, IMO.
> What happens if we can't tell whether we have an NRVO candidate or not until
> instantiation:
>
> ```
> template X f() {
> T v;
> return v; // is an NRVO candidate if T is X, otherwise is not
> }
> ```
>
> (I think this is not hard to handle: the dependent construct here can only
> affect whether you get NRVO at all, not which variable you perform NRVO on,
> but I want to check that it is handled properly.)
Ah, so if I understand correctly — which I probably don't —...
the parts of this diff related to `isRecordType()` make NRVO more reliable on
template functions? Because the existing code has lots of checks for
`isRecordType()` which means that they don't run for dependent (as-yet-unknown)
types, even if those types are going to turn out to be record types
occasionally?
I see that line 12838 runs `computeNRVO` unconditionally for *all*
ObjCMethodDecls, //even// ones with scalar return types. So maybe running
`computeNRVO` unconditionally right here would also be correct?
Datapoint: I made a patch to do nothing but replace this condition with `if
(Body)` and to remove the similar condition guarding `computeNRVO` in
`Sema::ActOnBlockStmtExpr`. It passed the Clang test suite with flying colors.
I don't know if this means that the test suite is missing some tests, or if it
means that the conditions are unnecessary.
IMHO if you're changing this anyway, it would be nice to put the remaining
condition/optimization `if (getReturnType().isScalarType()) return;` inside
`computeNRVO` itself, instead of repeating that condition at every call site.
Repository:
rC Clang
https://reviews.llvm.org/D47067
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name
jolesiak added a comment. LGTM, would be nice though if somebody else took a look (@klimek ?). Repository: rC Clang https://reviews.llvm.org/D47095 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
jolesiak added a comment. Does it look fine now @krasimir ? Repository: rC Clang https://reviews.llvm.org/D47195 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
sammccall added inline comments.
Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+ llvm_unreachable("No USR name mangling for fixed point types.");
case BuiltinType::Float16:
leonardchan wrote:
> rsmith wrote:
> > leonardchan wrote:
> > > phosek wrote:
> > > > We need some solution for fixed point types.
> > > Added character ~ to indicate fixed point type followed by string
> > > detailing the type. I have not added a test to it because logically, I do
> > > not think we will ever reach that point. This logic is implemented in the
> > > VisitType method, which mostly gets called by visitors to c++ nodes like
> > > VisitTemplateParameterList, but we have disabled the use of fixed point
> > > types in c++. VisitType does get called in VisitFunctionDecl but the
> > > function exits early since we are not reading c++ (line
> > > lib/Index/USRGeneration.cpp:238).
> > @rjmccall Is this an acceptable USR encoding? (Is our USR encoding scheme
> > documented anywhere?)
> I chatted with @sammccall about this who said it was ok to add these types if
> no one opposed this. I posted this on cfe-dev also and it seemed that no one
> spoke up about it, so I thought this was ok.
>
> I also couldn't find any standard or documentation about reserving characters
> for USR. It doesn't seem that USR is also parsed in any way, so I don't think
> I'm breaking anything (running ninja check-all passes).
>
> And unless we also do enable this for C++, this code actually may not be run
> since this method will not be visited if limited to C.
If the conclusion is to only allow these types for C, then you could punt on
this by setting IgnoreResult so USR generation fails. As you say USRs only
include type information in C++.
I think nothing parses them except humans, and using a prefix character is
probably better than eating 24 single characters. But one prefix character +
one trailing character might be less surprising.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47137: [Sparc] Add floating-point register names
dcederman updated this revision to Diff 148351.
dcederman added a comment.
Removed the non-existing registers f33, f35, and so on. I also removed the dX
and qX aliases. After trying them they felt more confusing than helpful.
https://reviews.llvm.org/D47137
Files:
lib/Basic/Targets/Sparc.cpp
test/CodeGen/sparcv8-inline-asm.c
test/CodeGen/sparcv9-inline-asm.c
Index: test/CodeGen/sparcv9-inline-asm.c
===
--- /dev/null
+++ test/CodeGen/sparcv9-inline-asm.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -triple sparcv9-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+void test_gcc_registers(void) {
+register unsigned int regO6 asm("o6") = 0;
+register unsigned int regSP asm("sp") = 1;
+register unsigned int reg14 asm("r14") = 2;
+register unsigned int regI6 asm("i6") = 3;
+register unsigned int regFP asm("fp") = 4;
+register unsigned int reg30 asm("r30") = 5;
+
+register float fF20 asm("f20") = 8.0;
+register double dF40 asm("f40") = 11.0;
+register long double qF40 asm("f40") = 14.0;
+
+// Test remapping register names in register ... asm("rN") statments.
+
+// CHECK: call void asm sideeffect "add $0,$1,$2", "{r14},{r14},{r14}"
+asm volatile("add %0,%1,%2" : : "r" (regO6), "r" (regSP), "r" (reg14));
+
+// CHECK: call void asm sideeffect "add $0,$1,$2", "{r30},{r30},{r30}"
+asm volatile("add %0,%1,%2" : : "r" (regI6), "r" (regFP), "r" (reg30));
+
+// CHECK: call void asm sideeffect "fadds $0,$1,$2", "{f20},{f20},{f20}"
+asm volatile("fadds %0,%1,%2" : : "f" (fF20), "f" (fF20), "f"(fF20));
+
+// CHECK: call void asm sideeffect "faddd $0,$1,$2", "{f40},{f40},{f40}"
+asm volatile("faddd %0,%1,%2" : : "f" (dF40), "f" (dF40), "f"(dF40));
+
+// CHECK: call void asm sideeffect "faddq $0,$1,$2", "{f40},{f40},{f40}"
+asm volatile("faddq %0,%1,%2" : : "f" (qF40), "f" (qF40), "f"(qF40));
+
+}
Index: test/CodeGen/sparcv8-inline-asm.c
===
--- test/CodeGen/sparcv8-inline-asm.c
+++ test/CodeGen/sparcv8-inline-asm.c
@@ -1,7 +1,7 @@
// RUN: %clang_cc1 -triple sparc-unknown-unknown -emit-llvm %s -o - | FileCheck %s
// CHECK: define float @fabsf(float %a)
-// CHECK: %{{.*}} = call float asm sideeffect "fabss $1, $0;", "=e,f"(float %{{.*}}) #1
+// CHECK: %{{.*}} = call float asm sideeffect "fabss $1, $0;", "=e,f"(float %{{.*}})
float fabsf(float a) {
float res;
__asm __volatile__("fabss %1, %0;"
@@ -9,3 +9,34 @@
: /* reg in */ "f"(a));
return res;
}
+
+void test_gcc_registers(void) {
+register unsigned int regO6 asm("o6") = 0;
+register unsigned int regSP asm("sp") = 1;
+register unsigned int reg14 asm("r14") = 2;
+register unsigned int regI6 asm("i6") = 3;
+register unsigned int regFP asm("fp") = 4;
+register unsigned int reg30 asm("r30") = 5;
+
+register float fF20 asm("f20") = 8.0;
+register double dF20 asm("f20") = 11.0;
+register long double qF20 asm("f20") = 14.0;
+
+// Test remapping register names in register ... asm("rN") statments.
+
+// CHECK: call void asm sideeffect "add $0,$1,$2", "{r14},{r14},{r14}"
+asm volatile("add %0,%1,%2" : : "r" (regO6), "r" (regSP), "r" (reg14));
+
+// CHECK: call void asm sideeffect "add $0,$1,$2", "{r30},{r30},{r30}"
+asm volatile("add %0,%1,%2" : : "r" (regI6), "r" (regFP), "r" (reg30));
+
+ // CHECK: call void asm sideeffect "fadds $0,$1,$2", "{f20},{f20},{f20}"
+asm volatile("fadds %0,%1,%2" : : "f" (fF20), "f" (fF20), "f"(fF20));
+
+// CHECK: call void asm sideeffect "faddd $0,$1,$2", "{f20},{f20},{f20}"
+asm volatile("faddd %0,%1,%2" : : "f" (dF20), "f" (dF20), "f"(dF20));
+
+// CHECK: call void asm sideeffect "faddq $0,$1,$2", "{f20},{f20},{f20}"
+asm volatile("faddq %0,%1,%2" : : "f" (qF20), "f" (qF20), "f"(qF20));
+
+}
Index: lib/Basic/Targets/Sparc.cpp
===
--- lib/Basic/Targets/Sparc.cpp
+++ lib/Basic/Targets/Sparc.cpp
@@ -20,9 +20,17 @@
using namespace clang::targets;
const char *const SparcTargetInfo::GCCRegNames[] = {
+// Integer registers
"r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", "r8", "r9", "r10",
"r11", "r12", "r13", "r14", "r15", "r16", "r17", "r18", "r19", "r20", "r21",
-"r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31"
+"r22", "r23", "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",
+
+// Floating-point registers
+"f0", "f1", "f2", "f3", "f4", "f5", "f6", "f7", "f8", "f9", "f10",
+"f11", "f12", "f13", "f14", "f15", "f16", "f17", "f18", "f19", "f20", "f21",
+"f22", "f23", "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", "f32",
+"f34", "f36", "f38", "f40", "f42", "f44", "f46", "f48", "f50", "f52", "f54",
+"f56", "f58", "f60", "f62",
};
ArrayRef SparcTargetInfo::getGCCRegName
[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence
martong updated this revision to Diff 148352.
martong added a comment.
Moved `using std::get` up, before `testStructuralMatch`.
Repository:
rC Clang
https://reviews.llvm.org/D46867
Files:
unittests/AST/ASTImporterTest.cpp
unittests/AST/CMakeLists.txt
unittests/AST/Language.cpp
unittests/AST/Language.h
unittests/AST/MatchVerifier.h
unittests/AST/StructuralEquivalenceTest.cpp
Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- /dev/null
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -0,0 +1,207 @@
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "Language.h"
+#include "DeclMatcher.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ast_matchers {
+
+using std::get;
+
+struct StructuralEquivalenceTest : ::testing::Test {
+ 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") {
+
+this->Code0 = SrcCode0;
+this->Code1 = SrcCode1;
+ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+const char *const InputFileName = "input.cc";
+
+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);
+
+ // We should find one Decl but one only.
+ assert(FoundDecls.size() == 1);
+
+ return FoundDecls[0];
+};
+
+NamedDecl *D0 = getDecl(Ctx0, Identifier);
+NamedDecl *D1 = getDecl(Ctx1, Identifier);
+assert(D0);
+assert(D1);
+return std::make_tuple(D0, D1);
+ }
+
+ bool testStructuralMatch(NamedDecl *D0, NamedDecl *D1) {
+llvm::DenseSet> NonEquivalentDecls;
+StructuralEquivalenceContext Ctx(D0->getASTContext(), D1->getASTContext(),
+ NonEquivalentDecls, false, false);
+return Ctx.IsStructurallyEquivalent(D0, D1);
+ }
+
+ bool testStructuralMatch(std::tuple t) {
+return testStructuralMatch(get<0>(t), get<1>(t));
+ }
+};
+
+TEST_F(StructuralEquivalenceTest, Int) {
+ auto Decls = makeNamedDecls("int foo;", "int foo;", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, IntVsSignedInt) {
+ auto Decls = makeNamedDecls("int foo;", "signed int foo;", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, Char) {
+ auto Decls = makeNamedDecls("char foo;", "char foo;", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+// This test is disabled for now.
+// FIXME Whether this is equivalent is dependendant on the target.
+TEST_F(StructuralEquivalenceTest, DISABLED_CharVsSignedChar) {
+ auto Decls = makeNamedDecls("char foo;", "signed char foo;", Lang_CXX);
+ EXPECT_FALSE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, ForwardRecordDecl) {
+ auto Decls = makeNamedDecls("struct foo;", "struct foo;", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, IntVsSignedIntInStruct) {
+ auto Decls = makeNamedDecls("struct foo { int x; };",
+ "struct foo { signed int x; };", Lang_CXX);
+ EXPECT_TRUE(testStructuralMatch(Decls));
+}
+
+TEST_F(StructuralEquivalenceTest, CharVsSignedCharInStruct) {
+ auto Decls = makeNamedDecls("struct foo { char x; };",
+ "struct foo { signed char x; };", Lang_CXX);
+ EXPECT_FALSE(testStructuralMatch(Decls));
+}
+
+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);
+ EXPECT_TRUE(testStru
[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333166: [ASTImporter] Add unit tests for structural
equivalence (authored by martong, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46867?vs=148352&id=148353#toc
Repository:
rC Clang
https://reviews.llvm.org/D46867
Files:
unittests/AST/ASTImporterTest.cpp
unittests/AST/CMakeLists.txt
unittests/AST/Language.cpp
unittests/AST/Language.h
unittests/AST/MatchVerifier.h
unittests/AST/StructuralEquivalenceTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -19,6 +19,7 @@
#include "clang/Tooling/Tooling.h"
#include "DeclMatcher.h"
+#include "Language.h"
#include "gtest/gtest.h"
#include "llvm/ADT/StringMap.h"
@@ -29,50 +30,6 @@
using internal::BindableMatcher;
using llvm::StringMap;
-typedef std::vector ArgVector;
-typedef std::vector RunOptions;
-
-static bool isCXX(Language Lang) {
- return Lang == Lang_CXX || Lang == Lang_CXX11;
-}
-
-static ArgVector getBasicRunOptionsForLanguage(Language Lang) {
- ArgVector BasicArgs;
- // Test with basic arguments.
- switch (Lang) {
- case Lang_C:
-BasicArgs = {"-x", "c", "-std=c99"};
-break;
- case Lang_C89:
-BasicArgs = {"-x", "c", "-std=c89"};
-break;
- case Lang_CXX:
-BasicArgs = {"-std=c++98", "-frtti"};
-break;
- case Lang_CXX11:
-BasicArgs = {"-std=c++11", "-frtti"};
-break;
- case Lang_OpenCL:
- case Lang_OBJCXX:
-llvm_unreachable("Not implemented yet!");
- }
- return BasicArgs;
-}
-
-static RunOptions getRunOptionsForLanguage(Language Lang) {
- ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang);
-
- // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
- // default behaviour.
- if (isCXX(Lang)) {
-ArgVector ArgsForDelayedTemplateParse = BasicArgs;
-ArgsForDelayedTemplateParse.emplace_back("-fdelayed-template-parsing");
-return {BasicArgs, ArgsForDelayedTemplateParse};
- }
-
- return {BasicArgs};
-}
-
// Creates a virtual file and assigns that to the context of given AST. If the
// file already exists then the file will not be created again as a duplicate.
static void
Index: unittests/AST/MatchVerifier.h
===
--- unittests/AST/MatchVerifier.h
+++ unittests/AST/MatchVerifier.h
@@ -23,20 +23,12 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Tooling/Tooling.h"
+#include "Language.h"
#include "gtest/gtest.h"
namespace clang {
namespace ast_matchers {
-enum Language {
-Lang_C,
-Lang_C89,
-Lang_CXX,
-Lang_CXX11,
-Lang_OpenCL,
-Lang_OBJCXX
-};
-
/// \brief Base class for verifying some property of nodes found by a matcher.
template
class MatchVerifier : public MatchFinder::MatchCallback {
@@ -113,6 +105,10 @@
Args.push_back("-std=c++11");
FileName = "input.cc";
break;
+ case Lang_CXX14:
+Args.push_back("-std=c++14");
+FileName = "input.cc";
+break;
case Lang_OpenCL:
FileName = "input.cl";
break;
Index: unittests/AST/CMakeLists.txt
===
--- unittests/AST/CMakeLists.txt
+++ unittests/AST/CMakeLists.txt
@@ -15,9 +15,11 @@
DeclTest.cpp
EvaluateAsRValueTest.cpp
ExternalASTSourceTest.cpp
+ Language.cpp
NamedDeclPrinterTest.cpp
SourceLocationTest.cpp
StmtPrinterTest.cpp
+ StructuralEquivalenceTest.cpp
)
target_link_libraries(ASTTests
Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -0,0 +1,207 @@
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/AST/ASTStructuralEquivalence.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "Language.h"
+#include "DeclMatcher.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ast_matchers {
+
+using std::get;
+
+struct StructuralEquivalenceTest : ::testing::Test {
+ 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") {
+
+this->Code0 = SrcCode0;
+this->Code1 = SrcCode1;
+ArgVector Args = getBasicRunOptionsForLanguage(Lang);
+
+const char *const InputFileName = "input.cc";
+
+AST0 = tooling::buildASTFromCodeWithArgs(Code0, Args,
r333166 - [ASTImporter] Add unit tests for structural equivalence
Author: martong
Date: Thu May 24 01:41:07 2018
New Revision: 333166
URL: http://llvm.org/viewvc/llvm-project?rev=333166&view=rev
Log:
[ASTImporter] Add unit tests for structural equivalence
Summary:
This patch add new tests for structural equivalence. For that a new
common header is created which holds the test related language specific
types and functions.
Reviewers: a.sidorin, xazax.hun, szepet
Subscribers: rnkovacs, dkrupp, cfe-commits
Differential Revision: https://reviews.llvm.org/D46867
Added:
cfe/trunk/unittests/AST/Language.cpp
cfe/trunk/unittests/AST/Language.h
cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
Modified:
cfe/trunk/unittests/AST/ASTImporterTest.cpp
cfe/trunk/unittests/AST/CMakeLists.txt
cfe/trunk/unittests/AST/MatchVerifier.h
Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=333166&r1=333165&r2=333166&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Thu May 24 01:41:07 2018
@@ -19,6 +19,7 @@
#include "clang/Tooling/Tooling.h"
#include "DeclMatcher.h"
+#include "Language.h"
#include "gtest/gtest.h"
#include "llvm/ADT/StringMap.h"
@@ -29,50 +30,6 @@ using internal::Matcher;
using internal::BindableMatcher;
using llvm::StringMap;
-typedef std::vector ArgVector;
-typedef std::vector RunOptions;
-
-static bool isCXX(Language Lang) {
- return Lang == Lang_CXX || Lang == Lang_CXX11;
-}
-
-static ArgVector getBasicRunOptionsForLanguage(Language Lang) {
- ArgVector BasicArgs;
- // Test with basic arguments.
- switch (Lang) {
- case Lang_C:
-BasicArgs = {"-x", "c", "-std=c99"};
-break;
- case Lang_C89:
-BasicArgs = {"-x", "c", "-std=c89"};
-break;
- case Lang_CXX:
-BasicArgs = {"-std=c++98", "-frtti"};
-break;
- case Lang_CXX11:
-BasicArgs = {"-std=c++11", "-frtti"};
-break;
- case Lang_OpenCL:
- case Lang_OBJCXX:
-llvm_unreachable("Not implemented yet!");
- }
- return BasicArgs;
-}
-
-static RunOptions getRunOptionsForLanguage(Language Lang) {
- ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang);
-
- // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
- // default behaviour.
- if (isCXX(Lang)) {
-ArgVector ArgsForDelayedTemplateParse = BasicArgs;
-ArgsForDelayedTemplateParse.emplace_back("-fdelayed-template-parsing");
-return {BasicArgs, ArgsForDelayedTemplateParse};
- }
-
- return {BasicArgs};
-}
-
// Creates a virtual file and assigns that to the context of given AST. If the
// file already exists then the file will not be created again as a duplicate.
static void
Modified: cfe/trunk/unittests/AST/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/CMakeLists.txt?rev=333166&r1=333165&r2=333166&view=diff
==
--- cfe/trunk/unittests/AST/CMakeLists.txt (original)
+++ cfe/trunk/unittests/AST/CMakeLists.txt Thu May 24 01:41:07 2018
@@ -15,9 +15,11 @@ add_clang_unittest(ASTTests
DeclTest.cpp
EvaluateAsRValueTest.cpp
ExternalASTSourceTest.cpp
+ Language.cpp
NamedDeclPrinterTest.cpp
SourceLocationTest.cpp
StmtPrinterTest.cpp
+ StructuralEquivalenceTest.cpp
)
target_link_libraries(ASTTests
Added: cfe/trunk/unittests/AST/Language.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/Language.cpp?rev=333166&view=auto
==
--- cfe/trunk/unittests/AST/Language.cpp (added)
+++ cfe/trunk/unittests/AST/Language.cpp Thu May 24 01:41:07 2018
@@ -0,0 +1,60 @@
+//===-- unittest/AST/Language.cpp - AST unit test support
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This file defines language options for AST unittests.
+//
+//===--===//
+
+#include "Language.h"
+
+namespace clang {
+namespace ast_matchers {
+
+ArgVector getBasicRunOptionsForLanguage(Language Lang) {
+ ArgVector BasicArgs;
+ // Test with basic arguments.
+ switch (Lang) {
+ case Lang_C:
+BasicArgs = {"-x", "c", "-std=c99"};
+break;
+ case Lang_C89:
+BasicArgs = {"-x", "c", "-std=c89"};
+break;
+ case Lang_CXX:
+BasicArgs = {"-std=c++98", "-frtti"};
+break;
+ case Lang_CXX11:
+BasicArgs = {"-std=c++11", "-frtti"};
+break;
+ case Lang_CXX14:
+BasicArgs = {"-std=c++14", "-frtti"};
+break;
+ case Lang_OpenCL:
+ case Lang_OBJCXX:
+llvm_unreachable("Not implemented yet!");
+ }
+ return
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov updated this revision to Diff 148355.
ilya-biryukov added a comment.
- Reimplemented LRU cache with shared_ptr and weak_ptr.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
test/clangd/trace.test
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Preamble",
+# CHECK: "name": "BuildPreamble",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: {
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Build",
+# CHECK: "name": "BuildAST",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: },
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
/// within a bounded amount of time.
};
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionParams {
+ /// Maximum number of ASTs to be retained in memory when there are no pending
+ /// requests for them.
+ unsigned MaxRetainedASTs = 3;
+};
+
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
/// preambles and ASTs) for opened files.
/// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
ASTParsedCallback ASTCallback,
- std::chrono::steady_clock::duration UpdateDebounce);
+ std::chrono::steady_clock::duration UpdateDebounce,
+ ASTRetentionParams RetentionConfig = {});
~TUScheduler();
/// Returns estimated memory usage for each of the currently open files.
@@ -98,12 +108,17 @@
private:
/// This class stores per-file data in the Files map.
struct FileData;
-
+public:
+ /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+ /// an LRU cache.
+ class IdleASTs;
+private:
const bool StorePreamblesInMemory;
const std::shared_ptr PCHOps;
const ASTParsedCallback ASTCallback;
Semaphore Barrier;
llvm::StringMap> Files;
+ std::unique_ptr ASTCache;
// None when running tasks synchronously and non-None when running tasks
// asynchronously.
llvm::Optional PreambleTasks;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,16 +45,155 @@
#include "TUScheduler.h"
#include "Logger.h"
#include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
+#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Path.h"
+#include
#include
#include
#include
namespace clang {
namespace clangd {
using std::chrono::steady_clock;
+namespace {
+/// The value cached in the TUScheduler::IdleASTs. If the AST was evicted from
+/// the cache, it will be recomputed on calls to get(). This class is **not**
+/// thread-safe, except getASTBytes() method. Users are responsible for
+/// synchronizing calls to update() and get().
+class CachedAST {
+public:
+ CachedAST(TUScheduler::IdleASTs &Owner) : Owner(Owner) {}
+ ~CachedAST();
+
+ /// Update the function used to compute the value.
+ void update(std::function()> ComputeF);
+
+ /// Get the value stored in the cache (or recompute it) and mark it as used.
+ std::shared_ptr> get();
+
+ /// Get the size of the stored AST. Returns 0 if no AST is cached.
+ size_t getASTBytes() const;
+
+private:
+ TUScheduler::IdleASTs &Owner;
+ std::function()> ComputeF;
+ // We only need synchronization for getting the AST size.
+ mutable std::mutex Mut;
+ std::weak_ptr> AST; /* GUARDED_BY(Mut) */
+ size_t LastASTSize = 0; /* GUARDED_BY(Mut) */
+};
+} // namespace
+
+/// Provides an LRU cache of ASTs.
+class TUScheduler::IdleASTs {
+ friend class CachedAST;
+
+public:
+ IdleASTs(unsigned MaxRetainedASTs) : MaxRetainedASTs(MaxRetainedASTs) {}
+
+ std::unique_ptr create() {
+return llvm::make_unique(*this);
+ }
+
+private:
+ void remove(CachedAST &Key) {
+std::unique_lock Lock(Mut);
+auto Existing = std::find_if(
+LRU.begin(), LRU.end(),
+[&Key](const std::pair>> &P) {
+ return P.first == &Key;
+});
+if (Existing == LRU.end())
+ return;
+std::shared_ptr> ForCleanup =
+std::move(Existing->second);
+LRU.erase(Existing);
+// If destructor of AST needs to run, we want it to happen outside the lock.
+Lock.unlock();
+ForCleanup.reset();
+ }
+
+ void markAc
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov added inline comments.
Comment at: clangd/ClangdUnit.h:132
-/// Manages resources, required by clangd. Allows to rebuild file with new
-/// contents, and provides AST and Preamble for it.
-class CppFile {
+/// A helper class that handles building preambles and ASTs for a file. Also
+/// adds some logging.
sammccall wrote:
> This may be change aversion, but I'm not sure this class does enough after
> this change - it doesn't store the inputs or the outputs/cache, so it kind of
> seems like it wants to be a function.
>
> I guess the motivation here is that storing the outputs means dealing with
> the cache, and the cache is local to TUScheduler.
> But `CppFile` is only used in TUScheduler, so we could move this there too?
> It feels like expanding the scope more than I'd like.
>
> The upside is that I think it's a more natural division of responsibility:
> `CppFile` could continue to be the "main" holder of the
> `shared_ptr` (which we don't limit, but share), and instead of
> `Optional` it'd have a `weak_ptr` which is controlled
> and can be refreshed through the cache.
>
> As discussed offline, the cache could look something like:
> ```
> class Cache {
>shared_ptr put(ParsedAST);
>void hintUsed(ParsedAST*); // optional, bumps LRU when client reads
>void hintUnused(ParsedAST*); // optional, releases when client abandons
> }
>
> shared_ptr CppFile::getAST() {
> shared_ptr AST = WeakAST.lock();
> if (AST)
> Cache.hintUsed(AST.get());
> else
> WeakAST = AST = Cache.put(build...);
> return AST;
> }
> ```
I've reimplemented the cache with weak_ptr/shared_ptr.
With a slightly different interface to hide more stuff in the cache API. Please
take a look and let me know what you think.
Nevertheless, I still find `CppFile` refactoring useful. It unties preambles
from the ASTs and I believe this is a good thing, given that their lifetimes
are different.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47256: [clangd] Fix code completion in MACROs with stringification.
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47256 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47313: [ASTImporter] Corrected lookup at import of templated record decl
balazske created this revision.
Herald added subscribers: cfe-commits, martong.
Herald added a reviewer: a.sidorin.
When a CXXRecordDecl under ClassTemplateDecl is imported, check
the templated record decl for similarity instead of the template.
Repository:
rC Clang
https://reviews.llvm.org/D47313
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1139,6 +1139,50 @@
has(fieldDecl(hasType(dependentSizedArrayType(;
}
+TEST_P(ASTImporterTestBase, ImportOfTemplatedDeclOfClassTemplateDecl) {
+ Decl *FromTU = getTuDecl("template struct S{};", Lang_CXX);
+ auto From =
+ FirstDeclMatcher().match(FromTU, classTemplateDecl());
+ ASSERT_TRUE(From);
+ auto To = cast(Import(From, Lang_CXX));
+ ASSERT_TRUE(To);
+ Decl *ToTemplated = To->getTemplatedDecl();
+ Decl *ToTemplated1 = Import(From->getTemplatedDecl(), Lang_CXX);
+ EXPECT_TRUE(ToTemplated1);
+ EXPECT_EQ(ToTemplated1, ToTemplated);
+}
+
+TEST_P(ASTImporterTestBase, ImportCorrectTemplatedDecl) {
+ auto Code =
+R"(
+namespace x {
+ template struct S1{};
+ template struct S2{};
+ template struct S3{};
+}
+)";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX);
+ auto FromNs =
+ FirstDeclMatcher().match(FromTU, namespaceDecl());
+ auto ToNs = cast(Import(FromNs, Lang_CXX));
+ ASSERT_TRUE(ToNs);
+ auto From =
+ FirstDeclMatcher().match(FromTU,
+ classTemplateDecl(
+ hasName("S2")));
+ auto To =
+ FirstDeclMatcher().match(ToNs,
+ classTemplateDecl(
+ hasName("S2")));
+ ASSERT_TRUE(From);
+ ASSERT_TRUE(To);
+ auto ToTemplated = To->getTemplatedDecl();
+ auto ToTemplated1 =
+ cast(Import(From->getTemplatedDecl(), Lang_CXX));
+ EXPECT_TRUE(ToTemplated1);
+ ASSERT_EQ(ToTemplated1, ToTemplated);
+}
+
TEST_P(ASTImporterTestBase, DISABLED_ImportFunctionWithBackReferringParameter)
{
Decl *From, *To;
std::tie(From, To) = getImportedDecl(
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -2009,7 +2009,14 @@
if (const auto *Tag = Typedef->getUnderlyingType()->getAs())
Found = Tag->getDecl();
}
-
+
+ if (D->getDescribedTemplate()) {
+if (auto *Template = dyn_cast(Found))
+ Found = Template->getTemplatedDecl();
+else
+ continue;
+ }
+
if (auto *FoundRecord = dyn_cast(Found)) {
if (!SearchName) {
// If both unnamed structs/unions are in a record context, make sure
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1139,6 +1139,50 @@
has(fieldDecl(hasType(dependentSizedArrayType(;
}
+TEST_P(ASTImporterTestBase, ImportOfTemplatedDeclOfClassTemplateDecl) {
+ Decl *FromTU = getTuDecl("template struct S{};", Lang_CXX);
+ auto From =
+ FirstDeclMatcher().match(FromTU, classTemplateDecl());
+ ASSERT_TRUE(From);
+ auto To = cast(Import(From, Lang_CXX));
+ ASSERT_TRUE(To);
+ Decl *ToTemplated = To->getTemplatedDecl();
+ Decl *ToTemplated1 = Import(From->getTemplatedDecl(), Lang_CXX);
+ EXPECT_TRUE(ToTemplated1);
+ EXPECT_EQ(ToTemplated1, ToTemplated);
+}
+
+TEST_P(ASTImporterTestBase, ImportCorrectTemplatedDecl) {
+ auto Code =
+R"(
+namespace x {
+ template struct S1{};
+ template struct S2{};
+ template struct S3{};
+}
+)";
+ Decl *FromTU = getTuDecl(Code, Lang_CXX);
+ auto FromNs =
+ FirstDeclMatcher().match(FromTU, namespaceDecl());
+ auto ToNs = cast(Import(FromNs, Lang_CXX));
+ ASSERT_TRUE(ToNs);
+ auto From =
+ FirstDeclMatcher().match(FromTU,
+ classTemplateDecl(
+ hasName("S2")));
+ auto To =
+ FirstDeclMatcher().match(ToNs,
+ classTemplateDecl(
+ hasName("S2")));
+ ASSERT_TRUE(From);
+ ASSERT_TRUE(To);
+ auto ToTemplated = To->getTemplatedDecl();
+ auto ToTemplated1 =
+ cast(Import(From->getTemplatedDecl(), Lang_CXX));
+ EXPECT_TRUE(ToTemplated1);
+ ASSERT_EQ(ToTemplated1, ToTemplated);
+}
+
TEST_P(ASTImporterTestBase, DISABLED_ImportFunctionWithBackReferringParameter) {
Decl *From, *To;
std::tie(From, To) = getImportedDecl(
Index: lib/AST/ASTImporter.
[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
SjoerdMeijer added a comment. Just out of curiousity: - How do you plan to implement this? Are you going to generate from the pragma some sort of "script" that dictates the transformation order which is going to be fed to the pass manager? - About the stacking of pragmas, in your example you apply the bottom one first, but would a user perhaps expect the first to be applied? In other words, is the expected behaviour described somewhere (in a spec)? https://reviews.llvm.org/D47267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46943: [clangd] Boost scores for decls from current file in completion
ilya-biryukov added a comment. After an offline chat: we decided to boost sema results that have **any** decls in the main file. Even though it introduces some unwanted inconsistencies in some cases (e.g. see the comment), most of us agree that's a very simple implementation that does boost useful things, including stuff from association header. To get better results, we need to design and build the real proximity scoring that handles associated headers and other things. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46943 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47262: [VFS] Implement getRealPath in InMemoryFileSystem.
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D47262 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333170 - Fix ASTImporterTest on Windows after r333082
Author: hans Date: Thu May 24 03:49:38 2018 New Revision: 333170 URL: http://llvm.org/viewvc/llvm-project?rev=333170&view=rev Log: Fix ASTImporterTest on Windows after r333082 With MS compatibility, Sema adds an implicit definition of type_info, which was causing the matchers to return 3 instead of 2. Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=333170&r1=333169&r2=333170&view=diff == --- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original) +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Thu May 24 03:49:38 2018 @@ -1500,7 +1500,7 @@ TEST_P(ASTImporterTestBase, )", Lang_CXX); ASSERT_EQ(2u, DeclCounter().match( -ToTU, cxxRecordDecl(hasParent(translationUnitDecl(); +ToTU, cxxRecordDecl(unless(isImplicit(); Decl *FromTU = getTuDecl( R"( @@ -1515,7 +1515,7 @@ TEST_P(ASTImporterTestBase, Import(FromD, Lang_CXX); EXPECT_EQ(2u, DeclCounter().match( -ToTU, cxxRecordDecl(hasParent(translationUnitDecl(); +ToTU, cxxRecordDecl(unless(isImplicit(); } TEST_P( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47313: [ASTImporter] Corrected lookup at import of templated record decl
martong added a comment. Looks good to me, but let's wait for Aleksei's approval and comments. Repository: rC Clang https://reviews.llvm.org/D47313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47195: [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333171: [clang-format] Fix putting ObjC message arguments in
one line for multiline… (authored by jolesiak, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47195?vs=148169&id=148370#toc
Repository:
rC Clang
https://reviews.llvm.org/D47195
Files:
lib/Format/ContinuationIndenter.cpp
unittests/Format/FormatTestObjC.cpp
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1387,6 +1387,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+ }
+
if (Current.is(tok::r_square)) {
// If this ends the array subscript expr, reset the corresponding value.
const FormatToken *NextNonComment = Current.getNextNonComment();
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -796,6 +796,41 @@
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ // Message receiver taking multiple lines.
+ Style.ColumnLimit = 20;
+ // Non-corner case.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] a:42 b:42];");
+ // Arguments just fit into one line.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42 b:42];");
+ // Arguments just over a column limit.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42\n"
+ "bb:42];");
+ // Arguments just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -1387,6 +1387,29 @@
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+ }
+
if (Current.is(tok::r_square)) {
// If this ends
Re: r333082 - Fix duplicate class template definitions problem
On Wed, May 23, 2018 at 3:53 PM, Gabor Marton via cfe-commits
wrote:
> Author: martong
> Date: Wed May 23 06:53:36 2018
> New Revision: 333082
>
> URL: http://llvm.org/viewvc/llvm-project?rev=333082&view=rev
> Log:
> Fix duplicate class template definitions problem
>
> Summary:
> We fail to import a `ClassTemplateDecl` if the "To" context already
> contains a definition and then a forward decl. This is because
> `localUncachedLookup` does not find the definition. This is not a
> lookup error, the parser behaves differently than assumed in the
> importer code. A `DeclContext` contains one DenseMap (`LookupPtr`)
> which maps names to lists. The list is a special list `StoredDeclsList`
> which is optimized to have one element. During building the initial
> AST, the parser first adds the definition to the `DeclContext`. Then
> during parsing the second declaration (the forward decl) the parser
> again calls `DeclContext::addDecl` but that will not add a new element
> to the `StoredDeclsList` rarther it simply overwrites the old element
> with the most recent one. This patch fixes the error by finding the
> definition in the redecl chain. Added tests for the same issue with
> `CXXRecordDecl` and with `ClassTemplateSpecializationDecl`. These tests
> pass and they pass because in `VisitRecordDecl` and in
> `VisitClassTemplateSpecializationDecl` we already use
> `D->getDefinition()` after the lookup.
>
> Reviewers: a.sidorin, xazax.hun, szepet
>
> Subscribers: rnkovacs, dkrupp, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D46950
[..]
> +TEST_P(ASTImporterTestBase,
> + ImportDefinitionOfClassIfThereIsAnExistingFwdDeclAndDefinition) {
> + Decl *ToTU = getToTuDecl(
> + R"(
> + struct B {
> +void f();
> + };
> +
> + struct B;
> + )",
> + Lang_CXX);
> + ASSERT_EQ(2u, DeclCounter().match(
> +ToTU, cxxRecordDecl(hasParent(translationUnitDecl();
This doesn't hold when targeting Windows, as Sema will add an implicit
declaration of type_info, causing the matcher to return 3 instead of
2.
I've committed r333170 to fix.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333171 - [clang-format] Fix putting ObjC message arguments in one line for multiline receiver
Author: jolesiak
Date: Thu May 24 03:50:36 2018
New Revision: 333171
URL: http://llvm.org/viewvc/llvm-project?rev=333171&view=rev
Log:
[clang-format] Fix putting ObjC message arguments in one line for multiline
receiver
Summary:
Reapply reverted changes from D46879.
Currently BreakBeforeParameter is set to true everytime message receiver spans
multiple lines, e.g.:
```
[[object block:^{
return 42;
}] aa:42 bb:42];
```
will be formatted:
```
[[object block:^{
return 42;
}] aa:42
bb:42];
```
even though arguments could fit into one line. This change fixes this behavior.
Test Plan:
make -j12 FormatTests && tools/clang/unittests/Format/FormatTests
Reviewers: benhamilton, krasimir
Reviewed By: benhamilton, krasimir
Subscribers: djasper, klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D47195
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestObjC.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=333171&r1=333170&r2=333171&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu May 24 03:50:36 2018
@@ -1387,6 +1387,29 @@ void ContinuationIndenter::moveStatePast
(Current.is(tok::greater) && Current.is(TT_DictLiteral
State.Stack.pop_back();
+ // Reevaluate whether ObjC message arguments fit into one line.
+ // If a receiver spans multiple lines, e.g.:
+ // [[object block:^{
+ // return 42;
+ // }] a:42 b:42];
+ // BreakBeforeParameter is calculated based on an incorrect assumption
+ // (it is checked whether the whole expression fits into one line without
+ // considering a line break inside a message receiver).
+ // We check whether arguements fit after receiver scope closer (into the same
+ // line).
+ if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous;
+if (CurrentScopeOpener.is(TT_ObjCMethodExpr) &&
+CurrentScopeOpener.MatchingParen) {
+ int NecessarySpaceInLine =
+ getLengthToMatchingParen(CurrentScopeOpener, State.Stack) +
+ CurrentScopeOpener.TotalLength - Current.TotalLength - 1;
+ if (State.Column + Current.ColumnWidth + NecessarySpaceInLine <=
+ Style.ColumnLimit)
+State.Stack.back().BreakBeforeParameter = false;
+}
+ }
+
if (Current.is(tok::r_square)) {
// If this ends the array subscript expr, reset the corresponding value.
const FormatToken *NextNonComment = Current.getNextNonComment();
Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=333171&r1=333170&r2=333171&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Thu May 24 03:50:36 2018
@@ -796,6 +796,41 @@ TEST_F(FormatTestObjC, FormatObjCMethodE
verifyFormat("[((Foo *)foo) bar];");
verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+ // Message receiver taking multiple lines.
+ Style.ColumnLimit = 20;
+ // Non-corner case.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] a:42 b:42];");
+ // Arguments just fit into one line.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42 b:42];");
+ // Arguments just over a column limit.
+ verifyFormat("[[object block:^{\n"
+ " return 42;\n"
+ "}] aaa:42\n"
+ "bb:42];");
+ // Arguments just fit into one line.
+ Style.ColumnLimit = 23;
+ verifyFormat("[[obj a:42\n"
+ " b:42\n"
+ " c:42\n"
+ " d:42] e:42 f:42];");
+
+ // Arguments do not fit into one line with a receiver.
+ Style.ColumnLimit = 20;
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42];");
+ verifyFormat("[[obj a:42] a:42\n"
+ "b:42\n"
+ "c:42];");
+ verifyFormat("[[obj aa:42\n"
+ " b:42]\n"
+ "cc:42\n"
+ " d:42];");
+
+
Style.ColumnLimit = 70;
verifyFormat(
"void f() {\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
martong added a comment. > Could you add a test for TSK_Undeclared as well? TLDR: No, I cannot. `TSK_Undeclared` is used to indicate that a template specialization was formed from a template-id but has not yet been declared, defined, or instantiated. Consequently, `ClassTemplateSpecializationDecl::ClassTemplateSpecializationDecl` and `VarTemplateSpecializationDecl::VarTemplateSpecializationDecl` initializes its member `SpecializationKind` to the value `TSK_Undeclared`. `SpecializationKind` is getting set during parsing to a meaningful kind, it is never left as `TSK_Undeclared`. So, the only way I could write a test is to manually change the `SpecializationKind` in the "From" context, but I consider that as not a meaningful test, since we cannot create such `Decl` by the parser. Repository: rC Clang https://reviews.llvm.org/D47058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47262: [VFS] Implement getRealPath in InMemoryFileSystem.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333172: [VFS] Implement getRealPath in InMemoryFileSystem.
(authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47262
Files:
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
Index: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
===
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp
@@ -788,6 +788,19 @@
return {};
}
+std::error_code
+InMemoryFileSystem::getRealPath(const Twine &Path,
+SmallVectorImpl &Output) const {
+ auto CWD = getCurrentWorkingDirectory();
+ if (!CWD || CWD->empty())
+return errc::operation_not_permitted;
+ Path.toVector(Output);
+ if (auto EC = makeAbsolute(Output))
+return EC;
+ llvm::sys::path::remove_dots(Output, /*remove_dot_dot=*/true);
+ return {};
+}
+
} // namespace vfs
} // namespace clang
Index: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
===
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
@@ -813,6 +813,28 @@
NormalizedFS.getCurrentWorkingDirectory().get()));
}
+TEST_F(InMemoryFileSystemTest, GetRealPath) {
+ SmallString<16> Path;
+ EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
+
+ auto GetRealPath = [this](StringRef P) {
+SmallString<16> Output;
+auto EC = FS.getRealPath(P, Output);
+EXPECT_FALSE(EC);
+return Output.str().str();
+ };
+
+ FS.setCurrentWorkingDirectory("a");
+ EXPECT_EQ(GetRealPath("b"), "a/b");
+ EXPECT_EQ(GetRealPath("../b"), "b");
+ EXPECT_EQ(GetRealPath("b/./c"), "a/b/c");
+
+ FS.setCurrentWorkingDirectory("/a");
+ EXPECT_EQ(GetRealPath("b"), "/a/b");
+ EXPECT_EQ(GetRealPath("../b"), "/b");
+ EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
+}
+
TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
FS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), 0xFEEDFACE);
auto Stat = FS.status("/a");
Index: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
===
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h
@@ -374,6 +374,14 @@
llvm::ErrorOr getCurrentWorkingDirectory() const override {
return WorkingDirectory;
}
+ /// Canonicalizes \p Path by combining with the current working
+ /// directory and normalizing the path (e.g. remove dots). If the current
+ /// working directory is not set, this returns errc::operation_not_permitted.
+ ///
+ /// This doesn't resolve symlinks as they are not supported in in-memory file
+ /// system.
+ std::error_code getRealPath(const Twine &Path,
+ SmallVectorImpl &Output) const override;
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
};
Index: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
===
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp
@@ -788,6 +788,19 @@
return {};
}
+std::error_code
+InMemoryFileSystem::getRealPath(const Twine &Path,
+SmallVectorImpl &Output) const {
+ auto CWD = getCurrentWorkingDirectory();
+ if (!CWD || CWD->empty())
+return errc::operation_not_permitted;
+ Path.toVector(Output);
+ if (auto EC = makeAbsolute(Output))
+return EC;
+ llvm::sys::path::remove_dots(Output, /*remove_dot_dot=*/true);
+ return {};
+}
+
} // namespace vfs
} // namespace clang
Index: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
===
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
@@ -813,6 +813,28 @@
NormalizedFS.getCurrentWorkingDirectory().get()));
}
+TEST_F(InMemoryFileSystemTest, GetRealPath) {
+ SmallString<16> Path;
+ EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
+
+ auto GetRealPath = [this](StringRef P) {
+SmallString<16> Output;
+auto EC = FS.getRealPath(P, Output);
+EXPECT_FALSE(EC);
+return Output.str().str();
+ };
+
+ FS.setCurrentWorkingDirectory("a");
+ EXPECT_EQ(GetRealPath("b"), "a/b");
+ EXPECT_EQ(GetRealPath("../b"), "b");
+ EXPECT_EQ(GetRealPath("b/./c"), "a/b/c");
+
+ FS.setCurrentWorkingDirectory("/a");
+ EXPECT_EQ(GetRealPath("b"), "/a/b");
+ EXPECT_EQ(GetRealPath("../b"), "/b");
+ EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
+}
+
TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
FS.addFile("/a/b/c", 0, MemoryBuffe
r333172 - [VFS] Implement getRealPath in InMemoryFileSystem.
Author: ioeric
Date: Thu May 24 04:17:00 2018
New Revision: 333172
URL: http://llvm.org/viewvc/llvm-project?rev=333172&view=rev
Log:
[VFS] Implement getRealPath in InMemoryFileSystem.
Reviewers: bkramer
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D47262
Modified:
cfe/trunk/include/clang/Basic/VirtualFileSystem.h
cfe/trunk/lib/Basic/VirtualFileSystem.cpp
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
Modified: cfe/trunk/include/clang/Basic/VirtualFileSystem.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/VirtualFileSystem.h?rev=333172&r1=333171&r2=333172&view=diff
==
--- cfe/trunk/include/clang/Basic/VirtualFileSystem.h (original)
+++ cfe/trunk/include/clang/Basic/VirtualFileSystem.h Thu May 24 04:17:00 2018
@@ -374,6 +374,14 @@ public:
llvm::ErrorOr getCurrentWorkingDirectory() const override {
return WorkingDirectory;
}
+ /// Canonicalizes \p Path by combining with the current working
+ /// directory and normalizing the path (e.g. remove dots). If the current
+ /// working directory is not set, this returns errc::operation_not_permitted.
+ ///
+ /// This doesn't resolve symlinks as they are not supported in in-memory file
+ /// system.
+ std::error_code getRealPath(const Twine &Path,
+ SmallVectorImpl &Output) const override;
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
};
Modified: cfe/trunk/lib/Basic/VirtualFileSystem.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/VirtualFileSystem.cpp?rev=333172&r1=333171&r2=333172&view=diff
==
--- cfe/trunk/lib/Basic/VirtualFileSystem.cpp (original)
+++ cfe/trunk/lib/Basic/VirtualFileSystem.cpp Thu May 24 04:17:00 2018
@@ -788,6 +788,19 @@ std::error_code InMemoryFileSystem::setC
return {};
}
+std::error_code
+InMemoryFileSystem::getRealPath(const Twine &Path,
+SmallVectorImpl &Output) const {
+ auto CWD = getCurrentWorkingDirectory();
+ if (!CWD || CWD->empty())
+return errc::operation_not_permitted;
+ Path.toVector(Output);
+ if (auto EC = makeAbsolute(Output))
+return EC;
+ llvm::sys::path::remove_dots(Output, /*remove_dot_dot=*/true);
+ return {};
+}
+
} // namespace vfs
} // namespace clang
Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=333172&r1=333171&r2=333172&view=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Thu May 24 04:17:00 2018
@@ -813,6 +813,28 @@ TEST_F(InMemoryFileSystemTest, WorkingDi
NormalizedFS.getCurrentWorkingDirectory().get()));
}
+TEST_F(InMemoryFileSystemTest, GetRealPath) {
+ SmallString<16> Path;
+ EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
+
+ auto GetRealPath = [this](StringRef P) {
+SmallString<16> Output;
+auto EC = FS.getRealPath(P, Output);
+EXPECT_FALSE(EC);
+return Output.str().str();
+ };
+
+ FS.setCurrentWorkingDirectory("a");
+ EXPECT_EQ(GetRealPath("b"), "a/b");
+ EXPECT_EQ(GetRealPath("../b"), "b");
+ EXPECT_EQ(GetRealPath("b/./c"), "a/b/c");
+
+ FS.setCurrentWorkingDirectory("/a");
+ EXPECT_EQ(GetRealPath("b"), "/a/b");
+ EXPECT_EQ(GetRealPath("../b"), "/b");
+ EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
+}
+
TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
FS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), 0xFEEDFACE);
auto Stat = FS.status("/a");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47262: [VFS] Implement getRealPath in InMemoryFileSystem.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333172: [VFS] Implement getRealPath in InMemoryFileSystem.
(authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47262?vs=148220&id=148376#toc
Repository:
rC Clang
https://reviews.llvm.org/D47262
Files:
include/clang/Basic/VirtualFileSystem.h
lib/Basic/VirtualFileSystem.cpp
unittests/Basic/VirtualFileSystemTest.cpp
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -788,6 +788,19 @@
return {};
}
+std::error_code
+InMemoryFileSystem::getRealPath(const Twine &Path,
+SmallVectorImpl &Output) const {
+ auto CWD = getCurrentWorkingDirectory();
+ if (!CWD || CWD->empty())
+return errc::operation_not_permitted;
+ Path.toVector(Output);
+ if (auto EC = makeAbsolute(Output))
+return EC;
+ llvm::sys::path::remove_dots(Output, /*remove_dot_dot=*/true);
+ return {};
+}
+
} // namespace vfs
} // namespace clang
Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -813,6 +813,28 @@
NormalizedFS.getCurrentWorkingDirectory().get()));
}
+TEST_F(InMemoryFileSystemTest, GetRealPath) {
+ SmallString<16> Path;
+ EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
+
+ auto GetRealPath = [this](StringRef P) {
+SmallString<16> Output;
+auto EC = FS.getRealPath(P, Output);
+EXPECT_FALSE(EC);
+return Output.str().str();
+ };
+
+ FS.setCurrentWorkingDirectory("a");
+ EXPECT_EQ(GetRealPath("b"), "a/b");
+ EXPECT_EQ(GetRealPath("../b"), "b");
+ EXPECT_EQ(GetRealPath("b/./c"), "a/b/c");
+
+ FS.setCurrentWorkingDirectory("/a");
+ EXPECT_EQ(GetRealPath("b"), "/a/b");
+ EXPECT_EQ(GetRealPath("../b"), "/b");
+ EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
+}
+
TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
FS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), 0xFEEDFACE);
auto Stat = FS.status("/a");
Index: include/clang/Basic/VirtualFileSystem.h
===
--- include/clang/Basic/VirtualFileSystem.h
+++ include/clang/Basic/VirtualFileSystem.h
@@ -374,6 +374,14 @@
llvm::ErrorOr getCurrentWorkingDirectory() const override {
return WorkingDirectory;
}
+ /// Canonicalizes \p Path by combining with the current working
+ /// directory and normalizing the path (e.g. remove dots). If the current
+ /// working directory is not set, this returns errc::operation_not_permitted.
+ ///
+ /// This doesn't resolve symlinks as they are not supported in in-memory file
+ /// system.
+ std::error_code getRealPath(const Twine &Path,
+ SmallVectorImpl &Output) const override;
std::error_code setCurrentWorkingDirectory(const Twine &Path) override;
};
Index: lib/Basic/VirtualFileSystem.cpp
===
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -788,6 +788,19 @@
return {};
}
+std::error_code
+InMemoryFileSystem::getRealPath(const Twine &Path,
+SmallVectorImpl &Output) const {
+ auto CWD = getCurrentWorkingDirectory();
+ if (!CWD || CWD->empty())
+return errc::operation_not_permitted;
+ Path.toVector(Output);
+ if (auto EC = makeAbsolute(Output))
+return EC;
+ llvm::sys::path::remove_dots(Output, /*remove_dot_dot=*/true);
+ return {};
+}
+
} // namespace vfs
} // namespace clang
Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -813,6 +813,28 @@
NormalizedFS.getCurrentWorkingDirectory().get()));
}
+TEST_F(InMemoryFileSystemTest, GetRealPath) {
+ SmallString<16> Path;
+ EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
+
+ auto GetRealPath = [this](StringRef P) {
+SmallString<16> Output;
+auto EC = FS.getRealPath(P, Output);
+EXPECT_FALSE(EC);
+return Output.str().str();
+ };
+
+ FS.setCurrentWorkingDirectory("a");
+ EXPECT_EQ(GetRealPath("b"), "a/b");
+ EXPECT_EQ(GetRealPath("../b"), "b");
+ EXPECT_EQ(GetRealPath("b/./c"), "a/b/c");
+
+ FS.setCurrentWorkingDirectory("/a");
+ EXPECT_EQ(GetRealPath("b"), "/a/b");
+ EXPECT_EQ(GetRealPath("../b"), "/b");
+ EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
+}
+
TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
FS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), 0xFEEDFACE);
auto Stat = FS.status("/a");
Index: include/clang/Basic/VirtualFileSystem.h
[PATCH] D47256: [clangd] Fix code completion in MACROs with stringification.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333174: [clangd] Fix code completion in MACROs with
stringification. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47256
Files:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -693,6 +693,42 @@
)cpp");
}
+TEST(CompletionTest, CompleteInMacroWithStringification) {
+ auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+int Y;
+} // namespace ns
+
+int f(int input_num) {
+ F(ns::^)
+}
+)cpp");
+
+ EXPECT_THAT(Results.items,
+ UnorderedElementsAre(Named("X"), Named("Y")));
+}
+
+TEST(CompletionTest, CompleteInMacroAndNamespaceWithStringification) {
+ auto Results = completions(R"cpp(
+void f(const char *, int x);
+#define F(x) f(#x, x)
+
+namespace ns {
+int X;
+
+int f(int input_num) {
+ F(^)
+}
+} // namespace ns
+)cpp");
+
+ EXPECT_THAT(Results.items, Contains(Named("X")));
+}
+
TEST(CompletionTest, CompleteInExcludedPPBranch) {
auto Results = completions(R"cpp(
int bar(int param_in_bar) {
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -406,6 +406,50 @@
return Info.scopesForIndexQuery();
}
+// Should we perform index-based completion in a context of the specified kind?
+// FIXME: consider allowing completion, but restricting the result types.
+bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
+ switch (K) {
+ case CodeCompletionContext::CCC_TopLevel:
+ case CodeCompletionContext::CCC_ObjCInterface:
+ case CodeCompletionContext::CCC_ObjCImplementation:
+ case CodeCompletionContext::CCC_ObjCIvarList:
+ case CodeCompletionContext::CCC_ClassStructUnion:
+ case CodeCompletionContext::CCC_Statement:
+ case CodeCompletionContext::CCC_Expression:
+ case CodeCompletionContext::CCC_ObjCMessageReceiver:
+ case CodeCompletionContext::CCC_EnumTag:
+ case CodeCompletionContext::CCC_UnionTag:
+ case CodeCompletionContext::CCC_ClassOrStructTag:
+ case CodeCompletionContext::CCC_ObjCProtocolName:
+ case CodeCompletionContext::CCC_Namespace:
+ case CodeCompletionContext::CCC_Type:
+ case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
+ case CodeCompletionContext::CCC_PotentiallyQualifiedName:
+ case CodeCompletionContext::CCC_ParenthesizedExpression:
+ case CodeCompletionContext::CCC_ObjCInterfaceName:
+ case CodeCompletionContext::CCC_ObjCCategoryName:
+return true;
+ case CodeCompletionContext::CCC_Other: // Be conservative.
+ case CodeCompletionContext::CCC_OtherWithMacros:
+ case CodeCompletionContext::CCC_DotMemberAccess:
+ case CodeCompletionContext::CCC_ArrowMemberAccess:
+ case CodeCompletionContext::CCC_ObjCPropertyAccess:
+ case CodeCompletionContext::CCC_MacroName:
+ case CodeCompletionContext::CCC_MacroNameUse:
+ case CodeCompletionContext::CCC_PreprocessorExpression:
+ case CodeCompletionContext::CCC_PreprocessorDirective:
+ case CodeCompletionContext::CCC_NaturalLanguage:
+ case CodeCompletionContext::CCC_SelectorName:
+ case CodeCompletionContext::CCC_TypeQualifiers:
+ case CodeCompletionContext::CCC_ObjCInstanceMessage:
+ case CodeCompletionContext::CCC_ObjCClassMessage:
+ case CodeCompletionContext::CCC_Recovery:
+return false;
+ }
+ llvm_unreachable("unknown code completion context");
+}
+
// The CompletionRecorder captures Sema code-complete output, including context.
// It filters out ignored results (but doesn't apply fuzzy-filtering yet).
// It doesn't do scoring or conversion to CompletionItem yet, as we want to
@@ -431,12 +475,17 @@
void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
CodeCompletionResult *InResults,
unsigned NumResults) override final {
+// If a callback is called without any sema result and the context does not
+// support index-based completion, we simply skip it to give way to
+// potential future callbacks with results.
+if (NumResults == 0 && !contextAllowsIndex(Context.getKind()))
+ return;
if (CCSema) {
log(llvm::formatv(
"Multiple code complete callbacks (parser backtracked?). "
"Dropping results from context {0}, keeping results from {1}.",
- getCompletionKindString(this->CCContext.getKind()),
- getCompletionKindString(Context.getKind()))
[clang-tools-extra] r333174 - [clangd] Fix code completion in MACROs with stringification.
Author: ioeric
Date: Thu May 24 04:20:19 2018
New Revision: 333174
URL: http://llvm.org/viewvc/llvm-project?rev=333174&view=rev
Log:
[clangd] Fix code completion in MACROs with stringification.
Summary:
Currently, we only handle the first callback from sema code completion
and ignore results from potential following callbacks. This causes
causes loss of completion results when multiple contexts are tried by Sema.
For example, we wouldn't get any completion result in the following completion
as the first attemped context is natural language which has no
candidate. The parser would backtrack and tried a completion with AST
semantic, which would find candidate "::x".
```
void f(const char*, int);
#define F(x) f(#x, x)
int x;
void main() {
F(::^);
}
```
To fix this, we only process a sema callback when it gives completion results or
the context supports index-based completion.
Reviewers: ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: klimek, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47256
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=333174&r1=333173&r2=333174&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu May 24 04:20:19 2018
@@ -406,6 +406,50 @@ std::vector getQueryScopes(
return Info.scopesForIndexQuery();
}
+// Should we perform index-based completion in a context of the specified kind?
+// FIXME: consider allowing completion, but restricting the result types.
+bool contextAllowsIndex(enum CodeCompletionContext::Kind K) {
+ switch (K) {
+ case CodeCompletionContext::CCC_TopLevel:
+ case CodeCompletionContext::CCC_ObjCInterface:
+ case CodeCompletionContext::CCC_ObjCImplementation:
+ case CodeCompletionContext::CCC_ObjCIvarList:
+ case CodeCompletionContext::CCC_ClassStructUnion:
+ case CodeCompletionContext::CCC_Statement:
+ case CodeCompletionContext::CCC_Expression:
+ case CodeCompletionContext::CCC_ObjCMessageReceiver:
+ case CodeCompletionContext::CCC_EnumTag:
+ case CodeCompletionContext::CCC_UnionTag:
+ case CodeCompletionContext::CCC_ClassOrStructTag:
+ case CodeCompletionContext::CCC_ObjCProtocolName:
+ case CodeCompletionContext::CCC_Namespace:
+ case CodeCompletionContext::CCC_Type:
+ case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
+ case CodeCompletionContext::CCC_PotentiallyQualifiedName:
+ case CodeCompletionContext::CCC_ParenthesizedExpression:
+ case CodeCompletionContext::CCC_ObjCInterfaceName:
+ case CodeCompletionContext::CCC_ObjCCategoryName:
+return true;
+ case CodeCompletionContext::CCC_Other: // Be conservative.
+ case CodeCompletionContext::CCC_OtherWithMacros:
+ case CodeCompletionContext::CCC_DotMemberAccess:
+ case CodeCompletionContext::CCC_ArrowMemberAccess:
+ case CodeCompletionContext::CCC_ObjCPropertyAccess:
+ case CodeCompletionContext::CCC_MacroName:
+ case CodeCompletionContext::CCC_MacroNameUse:
+ case CodeCompletionContext::CCC_PreprocessorExpression:
+ case CodeCompletionContext::CCC_PreprocessorDirective:
+ case CodeCompletionContext::CCC_NaturalLanguage:
+ case CodeCompletionContext::CCC_SelectorName:
+ case CodeCompletionContext::CCC_TypeQualifiers:
+ case CodeCompletionContext::CCC_ObjCInstanceMessage:
+ case CodeCompletionContext::CCC_ObjCClassMessage:
+ case CodeCompletionContext::CCC_Recovery:
+return false;
+ }
+ llvm_unreachable("unknown code completion context");
+}
+
// The CompletionRecorder captures Sema code-complete output, including
context.
// It filters out ignored results (but doesn't apply fuzzy-filtering yet).
// It doesn't do scoring or conversion to CompletionItem yet, as we want to
@@ -431,12 +475,17 @@ struct CompletionRecorder : public CodeC
void ProcessCodeCompleteResults(class Sema &S, CodeCompletionContext Context,
CodeCompletionResult *InResults,
unsigned NumResults) override final {
+// If a callback is called without any sema result and the context does not
+// support index-based completion, we simply skip it to give way to
+// potential future callbacks with results.
+if (NumResults == 0 && !contextAllowsIndex(Context.getKind()))
+ return;
if (CCSema) {
log(llvm::formatv(
"Multiple code complete callbacks (parser backtracked?). "
"Dropping results from context {0}, keeping results from {1}.",
- getCompletionKindString(this->CCContext.getKind()),
- getCompletionKindString(Context.getKind(;
+ getCompletionKindString(Context.getKind()),
+ getCo
Re: r333082 - Fix duplicate class template definitions problem
Thanks a lot!
Gabor
On Thu, May 24, 2018 at 12:54 PM Hans Wennborg wrote:
> On Wed, May 23, 2018 at 3:53 PM, Gabor Marton via cfe-commits
> wrote:
> > Author: martong
> > Date: Wed May 23 06:53:36 2018
> > New Revision: 333082
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=333082&view=rev
> > Log:
> > Fix duplicate class template definitions problem
> >
> > Summary:
> > We fail to import a `ClassTemplateDecl` if the "To" context already
> > contains a definition and then a forward decl. This is because
> > `localUncachedLookup` does not find the definition. This is not a
> > lookup error, the parser behaves differently than assumed in the
> > importer code. A `DeclContext` contains one DenseMap (`LookupPtr`)
> > which maps names to lists. The list is a special list `StoredDeclsList`
> > which is optimized to have one element. During building the initial
> > AST, the parser first adds the definition to the `DeclContext`. Then
> > during parsing the second declaration (the forward decl) the parser
> > again calls `DeclContext::addDecl` but that will not add a new element
> > to the `StoredDeclsList` rarther it simply overwrites the old element
> > with the most recent one. This patch fixes the error by finding the
> > definition in the redecl chain. Added tests for the same issue with
> > `CXXRecordDecl` and with `ClassTemplateSpecializationDecl`. These tests
> > pass and they pass because in `VisitRecordDecl` and in
> > `VisitClassTemplateSpecializationDecl` we already use
> > `D->getDefinition()` after the lookup.
> >
> > Reviewers: a.sidorin, xazax.hun, szepet
> >
> > Subscribers: rnkovacs, dkrupp, cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D46950
>
> [..]
>
> > +TEST_P(ASTImporterTestBase,
> > + ImportDefinitionOfClassIfThereIsAnExistingFwdDeclAndDefinition) {
> > + Decl *ToTU = getToTuDecl(
> > + R"(
> > + struct B {
> > +void f();
> > + };
> > +
> > + struct B;
> > + )",
> > + Lang_CXX);
> > + ASSERT_EQ(2u, DeclCounter().match(
> > +ToTU,
> cxxRecordDecl(hasParent(translationUnitDecl();
>
> This doesn't hold when targeting Windows, as Sema will add an implicit
> declaration of type_info, causing the matcher to return 3 instead of
> 2.
>
> I've committed r333170 to fix.
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46943: [clangd] Boost scores for decls from current file in completion
ilya-biryukov updated this revision to Diff 148381.
ilya-biryukov added a comment.
- Simplify the change by boosting any Decls that have a redecl in the current
file
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46943
Files:
clangd/Quality.cpp
clangd/Quality.h
unittests/clangd/QualityTests.cpp
unittests/clangd/TestTU.cpp
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -1,5 +1,4 @@
-//===--- TestTU.cpp - Scratch source files for testing *-
-//C++-*-===//
+//===--- TestTU.cpp - Scratch source files for testing ---===//
//
// The LLVM Compiler Infrastructure
//
@@ -78,11 +77,11 @@
auto *ND = dyn_cast(D);
if (!ND || ND->getNameAsString() != QName)
continue;
-if (Result) {
+if (Result && ND->getCanonicalDecl() != Result) {
ADD_FAILURE() << "Multiple Decls named " << QName;
assert(false && "QName is not unique");
}
-Result = ND;
+Result = cast(ND->getCanonicalDecl());
}
if (!Result) {
ADD_FAILURE() << "No Decl named " << QName << " in AST";
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -118,6 +118,45 @@
EXPECT_LT(sortText(0, "a"), sortText(0, "z"));
}
+TEST(QualityTests, BoostCurrentFileDecls) {
+ TestTU Test;
+ Test.HeaderFilename = "foo.h";
+ Test.HeaderCode = R"cpp(
+int test_func_in_header();
+int test_func_in_header_and_cpp();
+)cpp";
+ Test.Code = R"cpp(
+#include "foo.h"
+int ::test_func_in_header_and_cpp() {
+}
+int test_func_in_cpp();
+ )cpp";
+
+ ParsedAST AST = Test.build();
+
+ SymbolQualitySignals FuncInCpp;
+ FuncInCpp.merge(CodeCompletionResult(&findDecl(AST, "test_func_in_cpp"),
+ CCP_Declaration));
+ /// Decls in the current file should get a proximity score of 1.0.
+ EXPECT_FLOAT_EQ(FuncInCpp.ProximityScore, 1.0);
+
+ SymbolQualitySignals FuncInHeader;
+ FuncInHeader.merge(CodeCompletionResult(&findDecl(AST, "test_func_in_header"),
+ CCP_Declaration));
+ /// Decls outside current file currently don't get a proximity score boost.
+ EXPECT_FLOAT_EQ(FuncInHeader.ProximityScore, 0.0);
+
+ SymbolQualitySignals FuncInHeaderAndCpp;
+ FuncInHeaderAndCpp.merge(CodeCompletionResult(
+ &findDecl(AST, "test_func_in_header_and_cpp"), CCP_Declaration));
+ /// Decls in both header **and** the main file get the same boost.
+ EXPECT_FLOAT_EQ(FuncInHeaderAndCpp.ProximityScore, 1.0);
+
+ /// Check the boost in proximity translates into a better score.
+ EXPECT_LE(FuncInHeader.evaluate(), FuncInCpp.evaluate());
+ EXPECT_FLOAT_EQ(FuncInHeaderAndCpp.evaluate(), FuncInCpp.evaluate());
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -49,6 +49,7 @@
//quality and relevance. Untangle this.
bool Deprecated = false;
unsigned References = 0;
+ float ProximityScore = 0.0; /// Proximity score, in a [0,1] interval.
void merge(const CodeCompletionResult &SemaCCResult);
void merge(const Symbol &IndexResult);
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -8,6 +8,8 @@
//===-===//
#include "Quality.h"
#include "index/Index.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/SourceManager.h"
#include "clang/Sema/CodeCompleteConsumer.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MathExtras.h"
@@ -17,9 +19,27 @@
namespace clangd {
using namespace llvm;
+namespace {
+bool hasDeclInMainFile(const Decl &D) {
+ auto &SourceMgr = D.getASTContext().getSourceManager();
+ for (auto *Redecl : D.redecls()) {
+auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
+if (SourceMgr.isWrittenInMainFile(Loc))
+ return true;
+ }
+ return false;
+}
+} // namespace
+
void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) {
SemaCCPriority = SemaCCResult.Priority;
-
+ if (SemaCCResult.Declaration) {
+// We boost things that have decls in the main file.
+// The real proximity scores would be more general when we have them.
+float DeclProximity =
+hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.0;
+ProximityScore = std::max(DeclProximity, ProximityScore);
+ }
if (SemaCCResult.Availability == CXAvailability_Deprecated)
Deprecated = true;
}
@@ -41,6 +61,10 @@
// Priority 80 is a really bad score.
Score *= 2 - std::min
r333157 - [Sparc] Use the leon arch for Leon3's when using an external assembler
Author: dcederman
Date: Wed May 23 23:16:02 2018
New Revision: 333157
URL: http://llvm.org/viewvc/llvm-project?rev=333157&view=rev
Log:
[Sparc] Use the leon arch for Leon3's when using an external assembler
Summary: This allows the use of the casa instruction available in most Leon3's.
Reviewers: jyknight
Reviewed By: jyknight
Subscribers: joerg, fedor.sergeev, jrtc27, cfe-commits
Differential Revision: https://reviews.llvm.org/D47138
Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp
cfe/trunk/test/Driver/sparc-as.c
Modified: cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp?rev=333157&r1=333156&r2=333157&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/Sparc.cpp Wed May 23 23:16:02 2018
@@ -45,14 +45,29 @@ const char *sparc::getSparcAsmModeForCPU
.Case("niagara2", "-Av8plusb")
.Case("niagara3", "-Av8plusd")
.Case("niagara4", "-Av8plusd")
+.Case("ma2100", "-Aleon")
+.Case("ma2150", "-Aleon")
+.Case("ma2155", "-Aleon")
+.Case("ma2450", "-Aleon")
+.Case("ma2455", "-Aleon")
+.Case("ma2x5x", "-Aleon")
+.Case("ma2080", "-Aleon")
+.Case("ma2085", "-Aleon")
+.Case("ma2480", "-Aleon")
+.Case("ma2485", "-Aleon")
+.Case("ma2x8x", "-Aleon")
+.Case("myriad2", "-Aleon")
+.Case("myriad2.1", "-Aleon")
+.Case("myriad2.2", "-Aleon")
+.Case("myriad2.3", "-Aleon")
.Case("leon2", "-Av8")
.Case("at697e", "-Av8")
.Case("at697f", "-Av8")
-.Case("leon3", "-Av8")
+.Case("leon3", "-Aleon")
.Case("ut699", "-Av8")
-.Case("gr712rc", "-Av8")
-.Case("leon4", "-Av8")
-.Case("gr740", "-Av8")
+.Case("gr712rc", "-Aleon")
+.Case("leon4", "-Aleon")
+.Case("gr740", "-Aleon")
.Default("-Av8");
}
}
Modified: cfe/trunk/test/Driver/sparc-as.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/sparc-as.c?rev=333157&r1=333156&r2=333157&view=diff
==
--- cfe/trunk/test/Driver/sparc-as.c (original)
+++ cfe/trunk/test/Driver/sparc-as.c Wed May 23 23:16:02 2018
@@ -76,6 +76,66 @@
// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
// RUN: | FileCheck -check-prefix=SPARC-V8PLUSD %s
+// RUN: %clang -mcpu=ma2100 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2150 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2155 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2450 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2455 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2x5x -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2080 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2085 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2480 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2485 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=ma2x8x -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN: %clang -mcpu=myriad2 -no-canonical-prefixes -target sparc \
+// RUN: -no-integrated-as --sysroot=%S/Inputs/basic_netbsd_tree %s -### 2>&1 \
+// RUN: | FileCheck -check-prefix=SPARC-LEON %s
+
+// RUN:
[PATCH] D45920: [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333179: [analyzer] Move RangeSet related declarations into
the RangedConstraintManager… (authored by mramalho, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D45920
Files:
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
lib/StaticAnalyzer/Core/RangedConstraintManager.h
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -23,263 +23,171 @@
using namespace clang;
using namespace ento;
-/// A Range represents the closed range [from, to]. The caller must
-/// guarantee that from <= to. Note that Range is immutable, so as not
-/// to subvert RangeSet's immutability.
-namespace {
-class Range : public std::pair {
-public:
- Range(const llvm::APSInt &from, const llvm::APSInt &to)
- : std::pair(&from, &to) {
-assert(from <= to);
- }
- bool Includes(const llvm::APSInt &v) const {
-return *first <= v && v <= *second;
- }
- const llvm::APSInt &From() const { return *first; }
- const llvm::APSInt &To() const { return *second; }
- const llvm::APSInt *getConcreteValue() const {
-return &From() == &To() ? &From() : nullptr;
- }
-
- void Profile(llvm::FoldingSetNodeID &ID) const {
-ID.AddPointer(&From());
-ID.AddPointer(&To());
- }
-};
-
-class RangeTrait : public llvm::ImutContainerInfo {
-public:
- // When comparing if one Range is less than another, we should compare
- // the actual APSInt values instead of their pointers. This keeps the order
- // consistent (instead of comparing by pointer values) and can potentially
- // be used to speed up some of the operations in RangeSet.
- static inline bool isLess(key_type_ref lhs, key_type_ref rhs) {
-return *lhs.first < *rhs.first ||
- (!(*rhs.first < *lhs.first) && *lhs.second < *rhs.second);
- }
-};
-
-/// RangeSet contains a set of ranges. If the set is empty, then
-/// there the value of a symbol is overly constrained and there are no
-/// possible values for that symbol.
-class RangeSet {
- typedef llvm::ImmutableSet PrimRangeSet;
- PrimRangeSet ranges; // no need to make const, since it is an
- // ImmutableSet - this allows default operator=
- // to work.
-public:
- typedef PrimRangeSet::Factory Factory;
- typedef PrimRangeSet::iterator iterator;
-
- RangeSet(PrimRangeSet RS) : ranges(RS) {}
-
- /// Create a new set with all ranges of this set and RS.
- /// Possible intersections are not checked here.
- RangeSet addRange(Factory &F, const RangeSet &RS) {
-PrimRangeSet Ranges(RS.ranges);
-for (const auto &range : ranges)
- Ranges = F.add(Ranges, range);
-return RangeSet(Ranges);
- }
-
- iterator begin() const { return ranges.begin(); }
- iterator end() const { return ranges.end(); }
-
- bool isEmpty() const { return ranges.isEmpty(); }
-
- /// Construct a new RangeSet representing '{ [from, to] }'.
- RangeSet(Factory &F, const llvm::APSInt &from, const llvm::APSInt &to)
- : ranges(F.add(F.getEmptySet(), Range(from, to))) {}
-
- /// Profile - Generates a hash profile of this RangeSet for use
- /// by FoldingSet.
- void Profile(llvm::FoldingSetNodeID &ID) const { ranges.Profile(ID); }
-
- /// getConcreteValue - If a symbol is contrained to equal a specific integer
- /// constant then this method returns that value. Otherwise, it returns
- /// NULL.
- const llvm::APSInt *getConcreteValue() const {
-return ranges.isSingleton() ? ranges.begin()->getConcreteValue() : nullptr;
- }
+void RangeSet::IntersectInRange(BasicValueFactory &BV, Factory &F,
+ const llvm::APSInt &Lower, const llvm::APSInt &Upper,
+ PrimRangeSet &newRanges, PrimRangeSet::iterator &i,
+ PrimRangeSet::iterator &e) const {
+ // There are six cases for each range R in the set:
+ // 1. R is entirely before the intersection range.
+ // 2. R is entirely after the intersection range.
+ // 3. R contains the entire intersection range.
+ // 4. R starts before the intersection range and ends in the middle.
+ // 5. R starts in the middle of the intersection range and ends after it.
+ // 6. R is entirely contained in the intersection range.
+ // These correspond to each of the conditions below.
+ for (/* i = begin(), e = end() */; i != e; ++i) {
+if (i->To() < Lower) {
+ continue;
+}
+if (i->From() > Upper) {
+ break;
+}
-private:
- void IntersectInRange(BasicValueFactory &BV, Factory &F,
-const llvm::APSInt &Lower, const llvm::APSInt &Upper,
-PrimRangeSet &newRanges, PrimRangeSet::iterator &i,
-PrimRangeSet::iterator &e) const {
-// There are six cases for each range R in
[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums
ilya-biryukov added a comment. > But I feel it's a bit odd that completion and workspace symbols would be > inconsistent. It does not seem that odd to me. Completion is something that should follow the language rules more closely, i.e. we don't want to deviate from sema completions too much. While workspaceSymbol is a user-facing feature and we probably want users to type as little as possible there. E.g. not showing `namespace_name::A` in completion seems bad to me, since people are used to referring to the enumerators this way and sema completion gives you those results. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47320: [UnrollAndJam] Add pragma clang loop unroll_and_jam handling.
dmgreen created this revision.
Herald added a subscriber: zzheng.
Split out from https://reviews.llvm.org/D47267 when we need it.
https://reviews.llvm.org/D47320
Files:
include/clang/Basic/DiagnosticParseKinds.td
lib/Parse/ParsePragma.cpp
lib/Sema/SemaStmtAttr.cpp
test/CodeGenCXX/pragma-unroll-and-jam.cpp
test/Parser/pragma-loop.cpp
test/Parser/pragma-unroll-and-jam.cpp
Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- test/Parser/pragma-unroll-and-jam.cpp
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -50,24 +50,24 @@
#pragma nounroll_and_jam
/* expected-error {{expected a for, while, or do-while loop to follow '#pragma nounroll_and_jam'}} */ int l = Length;
-/* expected-error {{incompatible directives '#pragma nounroll_and_jam' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
-#pragma nounroll_and_jam
+/* expected-error {{incompatible directives 'unroll_and_jam(disable)' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
+#pragma clang loop unroll_and_jam(disable)
for (int i = 0; i < Length; i++) {
for (int j = 0; j < Length; j++) {
List[i * Length + j] = Value;
}
}
-#pragma nounroll_and_jam
-#pragma unroll(4)
+/* expected-error {{incompatible directives 'unroll_and_jam(full)' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam(4)
+#pragma clang loop unroll_and_jam(full)
for (int i = 0; i < Length; i++) {
for (int j = 0; j < Length; j++) {
List[i * Length + j] = Value;
}
}
-// pragma clang unroll_and_jam is disabled for the moment
-/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+#pragma nounroll_and_jam
+#pragma unroll(4)
for (int i = 0; i < Length; i++) {
for (int j = 0; j < Length; j++) {
List[i * Length + j] = Value;
Index: test/Parser/pragma-loop.cpp
===
--- test/Parser/pragma-loop.cpp
+++ test/Parser/pragma-loop.cpp
@@ -147,7 +147,7 @@
/* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
/* expected-error {{missing argument; expected 'enable' or 'disable'}} */ #pragma clang loop distribute()
-/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop
+/* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, unroll_and_jam, unroll_and_jam_count, or distribute}} */ #pragma clang loop
/* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
/* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
/* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
Index: test/CodeGenCXX/pragma-unroll-and-jam.cpp
===
--- test/CodeGenCXX/pragma-unroll-and-jam.cpp
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -33,6 +33,39 @@
}
}
+void clang_unroll_and_jam(int *List, int Length, int Value) {
+ // CHECK: define {{.*}} @_Z20clang_unroll_and_jam
+#pragma clang loop unroll_and_jam(enable)
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_4:.*]]
+ List[i * Length + j] = Value;
+}
+ }
+}
+
+void clang_unroll_and_jam_count(int *List, int Length, int Value) {
+ // CHECK: define {{.*}} @_Z26clang_unroll_and_jam_count
+#pragma clang loop unroll_and_jam_count(4)
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_5:.*]]
+ List[i * Length + j] = Value;
+}
+ }
+}
+
+void clang_nounroll_and_jam(int *List, int Length, int Value) {
+ // CHECK: define {{.*}} @_Z22clang_nounroll_and_jam
+#pragma clang loop unroll_and_jam(disable)
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_6:.*]]
+ List[i * Length + j] = Value;
+}
+ }
+}
+
void clang_unroll_plus_nounroll_and_jam(int *List, int Length, int Value) {
// CHECK: define {{.*}} @_Z34clang_unroll_plus_nounroll_and_jam
#pragma nounroll_and_jam
@@ -51,5 +84,8 @@
// CHECK: ![[UNJ_4]] = !{!"llvm.loop.unroll_and_jam.count", i32 4}
// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[UNJ_DISABLE:.*]]}
// CHECK: ![[UNJ_DISABLE]] = !{!"llvm.loop.unroll_and_jam.disable"}
+// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[UNJ_ENABLE:.*]]}
+// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNJ_4:.*]]}
+// CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[UNJ_DISABLE:.*]]}
// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[UN
[PATCH] D47196: [Time-report ](2): Recursive timers in Clang
avt77 updated this revision to Diff 148395.
avt77 added a comment.
The sources were re-based to fit in LLVM_DEBUG rename.
One more counter was added.
Debug logging was improved again.
https://reviews.llvm.org/D47196
Files:
include/clang/Frontend/Utils.h
lib/CodeGen/BackendUtil.cpp
lib/CodeGen/CodeGenAction.cpp
lib/CodeGen/CodeGenFunction.cpp
lib/CodeGen/CodeGenModule.cpp
lib/Frontend/FrontendTiming.cpp
lib/Parse/CMakeLists.txt
lib/Parse/ParseTemplate.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaLambda.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
lib/Sema/TreeTransform.h
test/Frontend/ftime-report-template-decl.cpp
test/Headers/opencl-c-header.cl
Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -71,4 +71,5 @@
}
#endif //__OPENCL_C_VERSION__
-// CHECK-MOD: Reading modules
+// CHECK-DAG-MOD: Clang Timers: CodeGen Functions
+// CHECK-DAG-MOD: Reading modules
Index: test/Frontend/ftime-report-template-decl.cpp
===
--- test/Frontend/ftime-report-template-decl.cpp
+++ test/Frontend/ftime-report-template-decl.cpp
@@ -3,9 +3,15 @@
// Template function declarations
template
-void foo();
+T foo(T bar) {
+ T Result = bar * bar + bar / 1.2 + bar;
+ return Result;
+};
template
-void foo();
+T foo(T bar, U bar2) {
+ T Result = bar2 * bar + bar / 1.2 + bar2;
+ return Result;
+};
// Template function definitions.
template
@@ -130,9 +136,15 @@
template
oneT L<0>::O::Fun(U) { return one; }
-void Instantiate() {
+double Instantiate() {
sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
sassert(sizeof(L<0>::O::Fun(0)) == sizeof(one));
+ int R1 = foo(123);
+ char R2 = foo('d', 1234);
+ int R3 = foo(1.2);
+ double R4 = foo(34.56, 1234);
+ double R5 = R1 + R2 * R3 - R4 + one[0] - two[1];
+ return R5 * R1 + R4 / R3 + R2;
}
}
@@ -150,7 +162,8 @@
};
_Wrap_alloc::rebind w;
-// CHECK: Miscellaneous Ungrouped Timers
+// CHECK: Clang Timers: CodeGen Functions
+// CHECK-DAG: Miscellaneous Ungrouped Timers
// CHECK-DAG: LLVM IR Generation Time
// CHECK-DAG: Code Generation Time
// CHECK: Total
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -27,6 +27,7 @@
#include "clang/AST/StmtCXX.h"
#include "clang/AST/StmtObjC.h"
#include "clang/AST/StmtOpenMP.h"
+#include "clang/Frontend/Utils.h"
#include "clang/Sema/Designator.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Ownership.h"
@@ -10945,6 +10946,11 @@
LSI->CallOperator = NewCallOperator;
+ if (FrontendTimesIsEnabled) {
+getFrontendFunctionTimeCtx()->startFrontendTimer(
+{NewCallOperator, 0.0});
+ }
+
for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
auto *P = NewCallOperator->getParamDecl(I);
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1,4 +1,5 @@
-//===--- SemaTemplateInstantiateDecl.cpp - C++ Template Decl Instantiation ===/
+//===--- SemaTemplateInstantiateDecl.cpp - C++ T--emplate Decl Instantiation
+//===/
//
// The LLVM Compiler Infrastructure
//
@@ -9,7 +10,6 @@
// This file implements C++ template instantiation for declarations.
//
//===--===/
-#include "clang/Sema/SemaInternal.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTMutationListener.h"
@@ -20,11 +20,14 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/PrettyDeclStackTrace.h"
#include "clang/AST/TypeLoc.h"
+#include "clang/Frontend/Utils.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
+#include "clang/Sema/SemaInternal.h"
#include "clang/Sema/Template.h"
#include "clang/Sema/TemplateInstCallback.h"
+#define DEBUG_TYPE "templateinst"
using namespace clang;
static bool isDeclWithinFunction(const Decl *D) {
Index: lib/Sema/SemaLambda.cpp
===
--- lib/Sema/SemaLambda.cpp
+++ lib/Sema/SemaLambda.cpp
@@ -10,20 +10,23 @@
// This file implements semantic analysis for C++ lambda expressions.
//
//===--===//
-#include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/SemaLambda.h"
#include "TypeLocBuilder.h"
#include "clang/AST/ASTLambda.h"
#include "clang/AST/ExprCXX.h"
#include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/Utils.h"
+#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/Initialization.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/Scope.h"
#include "clang/
[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
dmgreen updated this revision to Diff 148393.
dmgreen added a comment.
This splits out the pragma clang loop unroll_and_jam handling into
https://reviews.llvm.org/D47320, for if/when we need it. Which I believe is
what you wanted, correct me if I'm wrong.
https://reviews.llvm.org/D47267
Files:
include/clang/Basic/Attr.td
include/clang/Parse/Parser.h
lib/CodeGen/CGLoopInfo.cpp
lib/CodeGen/CGLoopInfo.h
lib/Parse/ParsePragma.cpp
lib/Sema/SemaStmtAttr.cpp
test/CodeGenCXX/pragma-unroll-and-jam.cpp
test/Parser/pragma-unroll-and-jam.cpp
Index: test/Parser/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/Parser/pragma-unroll-and-jam.cpp
@@ -0,0 +1,78 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+// Note that this puts the expected lines before the directives to work around
+// limitations in the -verify mode.
+
+void test(int *List, int Length, int Value) {
+ int i = 0;
+
+#pragma unroll_and_jam
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ List[i * Length + j] = Value;
+}
+ }
+
+#pragma nounroll_and_jam
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ List[i * Length + j] = Value;
+}
+ }
+
+#pragma unroll_and_jam 4
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ List[i * Length + j] = Value;
+}
+ }
+
+/* expected-error {{expected ')'}} */ #pragma unroll_and_jam(4
+/* expected-error {{missing argument; expected an integer value}} */ #pragma unroll_and_jam()
+/* expected-warning {{extra tokens at end of '#pragma unroll_and_jam'}} */ #pragma unroll_and_jam 1 2
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ List[i * Length + j] = Value;
+}
+ }
+
+/* expected-warning {{extra tokens at end of '#pragma nounroll_and_jam'}} */ #pragma nounroll_and_jam 1
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ List[i * Length + j] = Value;
+}
+ }
+
+#pragma unroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int j = Length;
+#pragma unroll_and_jam 4
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll_and_jam'}} */ int k = Length;
+#pragma nounroll_and_jam
+/* expected-error {{expected a for, while, or do-while loop to follow '#pragma nounroll_and_jam'}} */ int l = Length;
+
+/* expected-error {{incompatible directives '#pragma nounroll_and_jam' and '#pragma unroll_and_jam(4)'}} */ #pragma unroll_and_jam 4
+#pragma nounroll_and_jam
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ List[i * Length + j] = Value;
+}
+ }
+
+#pragma nounroll_and_jam
+#pragma unroll(4)
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ List[i * Length + j] = Value;
+}
+ }
+
+// pragma clang unroll_and_jam is disabled for the moment
+/* expected-error {{invalid option 'unroll_and_jam'; expected vectorize, vectorize_width, interleave, interleave_count, unroll, unroll_count, or distribute}} */ #pragma clang loop unroll_and_jam(4)
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ List[i * Length + j] = Value;
+}
+ }
+
+#pragma unroll_and_jam
+/* expected-error {{expected statement}} */ }
Index: test/CodeGenCXX/pragma-unroll-and-jam.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pragma-unroll-and-jam.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -triple arm-none-eabi -std=c++11 -emit-llvm -o - %s | FileCheck %s
+
+void unroll_and_jam(int *List, int Length, int Value) {
+ // CHECK: define {{.*}} @_Z14unroll_and_jam
+#pragma unroll_and_jam
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
+ List[i * Length + j] = Value;
+}
+ }
+}
+
+void unroll_and_jam_count(int *List, int Length, int Value) {
+ // CHECK: define {{.*}} @_Z20unroll_and_jam_count
+#pragma unroll_and_jam(4)
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_2:.*]]
+ List[i * Length + j] = Value;
+}
+ }
+}
+
+void nounroll_and_jam(int *List, int Length, int Value) {
+ // CHECK: define {{.*}} @_Z16nounroll_and_jam
+#pragma nounroll_and_jam
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_3:.*]]
+ List[i * Length + j] = Value;
+}
+ }
+}
+
+void clang_unroll_plus_nounroll_and_jam(int *List, int Length, int Value) {
+ // CHECK: define {{.*}} @_Z34clang_unroll_plus_nounroll_and_jam
+#pragma nounroll_and_jam
+#pragma unroll(4)
+ for (int i = 0; i < Length; i++) {
+for (int j = 0; j < Length; j++) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_7:.*
[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clangd/CodeCompletionStrings.h:46 const CodeCompleteConsumer::OverloadCandidate &Result, - unsigned ArgIndex); + unsigned ArgIndex, bool NoCommentsFromHeaders = true); this invites double negation. Also it doesn't seem like a great default, violates principle of least surprise. prefer bool CommentsFromHeaders = true, or with no default. (or even consider an enum) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes
ioeric added a comment. Would it be possible to add some integration tests for file index plus preamble? Comment at: clangd/index/FileIndex.cpp:37 + std::vector TopLevelDecls( + AST.getTranslationUnitDecl()->decls().begin(), + AST.getTranslationUnitDecl()->decls().end()); Would this give us decls in namespaces? It seems that `AST.getTranslationUnitDecl()->decls()` are only decls that are immediately inside the TU context. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41537: Optionally add code completion results for arrow instead of dot
ilya-biryukov added a comment. I have only a few nits left, otherwise looks good. Thanks for making this change! Really excited for it to get in finally! Comment at: include/clang/Driver/CC1Options.td:449 +def code_completion_with_fixits : Flag<["-"], "code-completion-with-fixits">, + HelpText<"Include code completion results which require small fix-its .">; def disable_free : Flag<["-"], "disable-free">, NIT: there's a redundant space before the closing `.` Comment at: include/clang/Sema/CodeCompleteConsumer.h:786 + /// \brief FixIts that *must* be applied before inserting the text for the + /// corresponding completion item. NIT: remove \brief from the start of the comment. Those recently got removed from all of llvm Comment at: include/clang/Sema/CodeCompleteConsumer.h:1059 + /// \brief Whether to include completion items with small fix-its, e.g. change + /// '.' to '->' on member access, etc. NIT: another redundant `\brief` Comment at: include/clang/Sema/CodeCompleteOptions.h:42 + /// Show also results after corrections (small fix-its), e.g. change '.' to + /// '->' on member access, etc. NIT: Maybe `s/Show also results/Include results`? Comment at: lib/Sema/SemaCodeComplete.cpp:4148 + + CompletionSucceded |= DoCompletion(Base, IsArrow, None); + NIT: maybe swap the two cases to do the non-fixit ones first? It is the most common case, so it should probably go first. https://reviews.llvm.org/D41537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333186 - Disable an in-memory vfs file path test on windows.
Author: ioeric
Date: Thu May 24 06:52:48 2018
New Revision: 333186
URL: http://llvm.org/viewvc/llvm-project?rev=333186&view=rev
Log:
Disable an in-memory vfs file path test on windows.
The test uses unix paths and doesn't make sense to run on windows.
Fix bot failure caused by r333172:
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/10799
Modified:
cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
Modified: cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp?rev=333186&r1=333185&r2=333186&view=diff
==
--- cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp (original)
+++ cfe/trunk/unittests/Basic/VirtualFileSystemTest.cpp Thu May 24 06:52:48 2018
@@ -813,6 +813,7 @@ TEST_F(InMemoryFileSystemTest, WorkingDi
NormalizedFS.getCurrentWorkingDirectory().get()));
}
+#if !defined(_WIN32)
TEST_F(InMemoryFileSystemTest, GetRealPath) {
SmallString<16> Path;
EXPECT_EQ(FS.getRealPath("b", Path), errc::operation_not_permitted);
@@ -834,6 +835,7 @@ TEST_F(InMemoryFileSystemTest, GetRealPa
EXPECT_EQ(GetRealPath("../b"), "/b");
EXPECT_EQ(GetRealPath("b/./c"), "/a/b/c");
}
+#endif // _WIN32
TEST_F(InMemoryFileSystemTest, AddFileWithUser) {
FS.addFile("/a/b/c", 0, MemoryBuffer::getMemBuffer("abc"), 0xFEEDFACE);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes
ilya-biryukov added a comment. In https://reviews.llvm.org/D47272#1110958, @ioeric wrote: > Would it be possible to add some integration tests for file index plus > preamble? Sure, will do Comment at: clangd/index/FileIndex.cpp:37 + std::vector TopLevelDecls( + AST.getTranslationUnitDecl()->decls().begin(), + AST.getTranslationUnitDecl()->decls().end()); ioeric wrote: > Would this give us decls in namespaces? It seems that > `AST.getTranslationUnitDecl()->decls()` are only decls that are immediately > inside the TU context. I'll double check and add a test for that, but I think the indexer visits the namespaces that we provide, I'm not sure if we have tests though. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D46964: Implement class deduction guides for `std::array`
Hi Marshall, ARM and AArch64 libcxx buildbots are broken since that commits, logs are available at: http://lab.llvm.org:8011/builders/libcxx-libcxxabi-libunwind-armv7-linux/builds/107/steps/test.libcxx/logs/FAIL%3A%20libc%2B%2B%3A%3Adeduct.pass.cpp Thanks Yvan On 18 May 2018 at 23:05, Marshall Clow via Phabricator via cfe-commits wrote: > mclow.lists closed this revision. > mclow.lists added a comment. > > Committed as revision 332768 > > > https://reviews.llvm.org/D46964 > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes
ioeric added inline comments. Comment at: clangd/index/FileIndex.cpp:37 + std::vector TopLevelDecls( + AST.getTranslationUnitDecl()->decls().begin(), + AST.getTranslationUnitDecl()->decls().end()); ilya-biryukov wrote: > ioeric wrote: > > Would this give us decls in namespaces? It seems that > > `AST.getTranslationUnitDecl()->decls()` are only decls that are immediately > > inside the TU context. > I'll double check and add a test for that, but I think the indexer visits the > namespaces that we provide, I'm not sure if we have tests though. Oh, right! In that case, it seems that a single TU decl which contains all interesting decls would be sufficient? Is it possible that preamble doesn't have all available top level decls in TU decl? This seems to be possible for ParsedAST, which is why we used `getTopLevelDecls`, I think? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333188 - [clangd] Skip .inc headers when canonicalizing header #include.
Author: ioeric
Date: Thu May 24 07:40:24 2018
New Revision: 333188
URL: http://llvm.org/viewvc/llvm-project?rev=333188&view=rev
Log:
[clangd] Skip .inc headers when canonicalizing header #include.
Summary:
This assumes that .inc files are supposed to be included via headers
that include them.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ilya-biryukov, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47187
Modified:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=333188&r1=333187&r2=333188&view=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Thu May 24
07:40:24 2018
@@ -8,6 +8,8 @@
//===--===//
#include "CanonicalIncludes.h"
+#include "../Headers.h"
+#include "clang/Driver/Types.h"
#include "llvm/Support/Regex.h"
namespace clang {
@@ -33,12 +35,33 @@ void CanonicalIncludes::addSymbolMapping
}
llvm::StringRef
-CanonicalIncludes::mapHeader(llvm::StringRef Header,
+CanonicalIncludes::mapHeader(llvm::ArrayRef Headers,
llvm::StringRef QualifiedName) const {
+ assert(!Headers.empty());
auto SE = SymbolMapping.find(QualifiedName);
if (SE != SymbolMapping.end())
return SE->second;
std::lock_guard Lock(RegexMutex);
+ // Find the first header such that the extension is not '.inc', and isn't a
+ // recognized non-header file
+ auto I =
+ std::find_if(Headers.begin(), Headers.end(), [](llvm::StringRef Include)
{
+// Skip .inc file whose including header file should
+// be #included instead.
+return !Include.endswith(".inc");
+ });
+ if (I == Headers.end())
+return Headers[0]; // Fallback to the declaring header.
+ StringRef Header = *I;
+ // If Header is not expected be included (e.g. .cc file), we fall back to
+ // the declaring header.
+ StringRef Ext = llvm::sys::path::extension(Header).trim('.');
+ // Include-able headers must have precompile type. Treat files with
+ // non-recognized extenstions (TY_INVALID) as headers.
+ auto ExtType = driver::types::lookupTypeForExtension(Ext);
+ if ((ExtType != driver::types::TY_INVALID) &&
+ !driver::types::onlyPrecompileType(ExtType))
+return Headers[0];
for (auto &Entry : RegexHeaderMappingTable) {
#ifndef NDEBUG
std::string Dummy;
@@ -65,9 +88,8 @@ collectIWYUHeaderMaps(CanonicalIncludes
// FIXME(ioeric): resolve the header and store actual file path. For now,
// we simply assume the written header is suitable to be #included.
Includes->addMapping(PP.getSourceManager().getFilename(Range.getBegin()),
- (Text.startswith("<") || Text.startswith("\""))
- ? Text.str()
- : ("\"" + Text + "\"").str());
+ isLiteralInclude(Text) ? Text.str()
+ : ("\"" + Text +
"\"").str());
return false;
}
Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h?rev=333188&r1=333187&r2=333188&view=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h Thu May 24
07:40:24 2018
@@ -48,9 +48,10 @@ public:
void addSymbolMapping(llvm::StringRef QualifiedName,
llvm::StringRef CanonicalPath);
- /// Returns the canonical include for symbol with \p QualifiedName, which is
- /// declared in \p Header
- llvm::StringRef mapHeader(llvm::StringRef Header,
+ /// Returns the canonical include for symbol with \p QualifiedName.
+ /// \p Headers is the include stack: Headers.front() is the file declaring
the
+ /// symbol, and Headers.back() is the main file.
+ llvm::StringRef mapHeader(llvm::ArrayRef Headers,
llvm::StringRef QualifiedName) const;
private:
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=333188&r1=333187&r2=333188&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333188: [clangd] Skip .inc headers when canonicalizing
header #include. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47187
Files:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -131,9 +131,9 @@
auto Factory = llvm::make_unique(
CollectorOpts, PragmaHandler.get());
-std::vector Args = {"symbol_collector", "-fsyntax-only",
- "-std=c++11", "-include",
- TestHeaderName, TestFileName};
+std::vector Args = {
+"symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11",
+"-include", TestHeaderName, TestFileName};
Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
tooling::ToolInvocation Invocation(
Args,
@@ -666,6 +666,70 @@
IncludeHeader("\"the/good/header.h\"";
}
+TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) {
+ CollectorOpts.CollectIncludePath = true;
+ CanonicalIncludes Includes;
+ Includes.addMapping(TestHeaderName, "");
+ CollectorOpts.Includes = &Includes;
+ auto IncFile = testPath("test.inc");
+ auto IncURI = URI::createFile(IncFile).toString();
+ InMemoryFileSystem->addFile(IncFile, 0,
+ llvm::MemoryBuffer::getMemBuffer("class X {};"));
+ runSymbolCollector("#include \"test.inc\"\nclass Y {};", /*Main=*/"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader("")),
+ AllOf(QName("Y"), DeclURI(TestHeaderURI),
+ IncludeHeader("";
+}
+
+TEST_F(SymbolCollectorTest, MainFileIsHeaderWhenSkipIncFile) {
+ CollectorOpts.CollectIncludePath = true;
+ CanonicalIncludes Includes;
+ CollectorOpts.Includes = &Includes;
+ TestFileName = testPath("main.h");
+ TestFileURI = URI::createFile(TestFileName).toString();
+ auto IncFile = testPath("test.inc");
+ auto IncURI = URI::createFile(IncFile).toString();
+ InMemoryFileSystem->addFile(IncFile, 0,
+ llvm::MemoryBuffer::getMemBuffer("class X {};"));
+ runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader(TestFileURI;
+}
+
+TEST_F(SymbolCollectorTest, MainFileIsHeaderWithoutExtensionWhenSkipIncFile) {
+ CollectorOpts.CollectIncludePath = true;
+ CanonicalIncludes Includes;
+ CollectorOpts.Includes = &Includes;
+ TestFileName = testPath("no_ext_main");
+ TestFileURI = URI::createFile(TestFileName).toString();
+ auto IncFile = testPath("test.inc");
+ auto IncURI = URI::createFile(IncFile).toString();
+ InMemoryFileSystem->addFile(IncFile, 0,
+ llvm::MemoryBuffer::getMemBuffer("class X {};"));
+ runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader(TestFileURI;
+}
+
+TEST_F(SymbolCollectorTest, FallbackToIncFileWhenIncludingFileIsCC) {
+ CollectorOpts.CollectIncludePath = true;
+ CanonicalIncludes Includes;
+ CollectorOpts.Includes = &Includes;
+ auto IncFile = testPath("test.inc");
+ auto IncURI = URI::createFile(IncFile).toString();
+ InMemoryFileSystem->addFile(IncFile, 0,
+ llvm::MemoryBuffer::getMemBuffer("class X {};"));
+ runSymbolCollector("", /*Main=*/"#include \"test.inc\"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), DeclURI(IncURI),
+ IncludeHeader(IncURI;
+}
+
TEST_F(SymbolCollectorTest, AvoidUsingFwdDeclsAsCanonicalDecls) {
CollectorOpts.CollectIncludePath = true;
Annotations Header(R"(
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+
[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only
ilya-biryukov updated this revision to Diff 148413.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Invert flag, remove the default.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47274
Files:
clangd/CodeComplete.cpp
clangd/CodeCompletionStrings.cpp
clangd/CodeCompletionStrings.h
clangd/index/SymbolCollector.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -17,6 +17,7 @@
#include "SyncAPI.h"
#include "TestFS.h"
#include "index/MemIndex.h"
+#include "llvm/Support/Error.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -998,6 +999,46 @@
}
}
+TEST(CompletionTest, DocumentationFromChangedFileCrash) {
+ MockFSProvider FS;
+ auto FooH = testPath("foo.h");
+ auto FooCpp = testPath("foo.cpp");
+ FS.Files[FooH] = R"cpp(
+// this is my documentation comment.
+int func();
+ )cpp";
+ FS.Files[FooCpp] = "";
+
+ MockCompilationDatabase CDB;
+ IgnoreDiagnostics DiagConsumer;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ Annotations Source(R"cpp(
+#include "foo.h"
+int func() {
+ // This makes sure we have func from header in the AST.
+}
+int a = fun^
+ )cpp");
+ Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
+ // We need to wait for preamble to build.
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
+
+ // Change the header file. Completion will reuse the old preamble!
+ FS.Files[FooH] = R"cpp(
+int func();
+ )cpp";
+
+ clangd::CodeCompleteOptions Opts;
+ Opts.IncludeComments = true;
+ CompletionList Completions =
+ cantFail(runCodeComplete(Server, FooCpp, Source.point(), Opts));
+ // We shouldn't crash. Unfortunately, current workaround is to not produce
+ // comments for symbols from headers.
+ EXPECT_THAT(Completions.items,
+ Contains(AllOf(Not(IsDocumented()), Named("func";
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -381,7 +381,8 @@
/*EnableSnippets=*/false);
std::string FilterText = getFilterText(*CCS);
std::string Documentation =
- formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion));
+ formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
+ /*CommentsFromHeaders=*/true));
std::string CompletionDetail = getDetail(*CCS);
std::string Include;
Index: clangd/CodeCompletionStrings.h
===
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -25,18 +25,25 @@
/// markers stripped. See clang::RawComment::getFormattedText() for the detailed
/// explanation of how the comment text is transformed.
/// Returns empty string when no comment is available.
+/// If \p CommentsFromHeaders parameter is set, only comments from the main
+/// file will be returned. It is used to workaround crashes when parsing
+/// comments in the stale headers, coming from completion preamble.
std::string getDocComment(const ASTContext &Ctx,
- const CodeCompletionResult &Result);
+ const CodeCompletionResult &Result,
+ bool CommentsFromHeaders);
/// Gets a minimally formatted documentation for parameter of \p Result,
/// corresponding to argument number \p ArgIndex.
/// This currently looks for comments attached to the parameter itself, and
/// doesn't extract them from function documentation.
/// Returns empty string when no comment is available.
+/// If \p CommentsFromHeaders parameter is set, only comments from the main
+/// file will be returned. It is used to workaround crashes when parsing
+/// comments in the stale headers, coming from completion preamble.
std::string
getParameterDocComment(const ASTContext &Ctx,
const CodeCompleteConsumer::OverloadCandidate &Result,
- unsigned ArgIndex);
+ unsigned ArgIndex, bool CommentsFromHeaders);
/// Gets label and insert text for a completion item. For example, for function
/// `Foo`, this returns <"Foo(int x, int y)", "Foo"> without snippts enabled.
Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -10,6 +10,7 @@
#include "CodeCompletionStrings.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/RawCommentList.h"
+#include "clang/Basic/SourceManager.h"
#include
namespace clang {
@@ -122,10 +123,23 @@
}
[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only
ilya-biryukov added inline comments. Comment at: clangd/CodeCompletionStrings.h:46 const CodeCompleteConsumer::OverloadCandidate &Result, - unsigned ArgIndex); + unsigned ArgIndex, bool NoCommentsFromHeaders = true); sammccall wrote: > this invites double negation. > Also it doesn't seem like a great default, violates principle of least > surprise. > > prefer bool CommentsFromHeaders = true, or with no default. > (or even consider an enum) Thanks for spotting this! I opted for a positive bool flag with no defaults. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47274 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst
gramanas updated this revision to Diff 148414.
gramanas added a comment.
Adress review comments.
Limit changes to the storeInst
Repository:
rC Clang
https://reviews.llvm.org/D47097
Files:
lib/CodeGen/CGDecl.cpp
test/CodeGen/debug-info-preserve-scope.c
Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+ a = b;
+}
+
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+
+// CHECK: ![[artificialDbgLoc]] = !DILocation(line: 0
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
}
}
+ // Set artificial debug location in order to preserve the scope
+ auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
Address DeclPtr = Address::invalid();
bool DoStore = false;
bool IsScalar = hasScalarEvaluationKind(Ty);
Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+static int a;
+
+// CHECK-LABEL: define void @f
+void f(int b) {
+ a = b;
+}
+
+// CHECK: store i32 {{.*}} !dbg ![[artificialDbgLoc:[0-9]+]]
+
+// CHECK: ![[artificialDbgLoc]] = !DILocation(line: 0
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
}
}
+ // Set artificial debug location in order to preserve the scope
+ auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
Address DeclPtr = Address::invalid();
bool DoStore = false;
bool IsScalar = hasScalarEvaluationKind(Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333189 - [clangd] Serve comments for headers decls from dynamic index only
Author: ibiryukov
Date: Thu May 24 07:49:23 2018
New Revision: 333189
URL: http://llvm.org/viewvc/llvm-project?rev=333189&view=rev
Log:
[clangd] Serve comments for headers decls from dynamic index only
Summary:
To fix a crash in code completion that occurrs when reading doc
comments from files that were updated after the preamble was
computed. In that case, the files on disk could've been changed and we
can't rely on finding the comment text with the same range anymore.
The current workaround is to not provide comments from the headers at
all and rely on the dynamic index instead.
A more principled solution would be to store contents of the files
read inside the preamble, but it is way harder to implement properly,
given that it would definitely increase the sizes of the preamble.
Together with D47272, this should fix all preamble-related crashes
we're aware of.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: klimek, ioeric, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47274
Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=333189&r1=333188&r2=333189&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu May 24 07:49:23 2018
@@ -586,9 +586,11 @@ public:
const auto *CCS = Candidate.CreateSignatureString(
CurrentArg, S, *Allocator, CCTUInfo, true);
assert(CCS && "Expected the CodeCompletionString to be non-null");
+ // FIXME: for headers, we need to get a comment from the index.
SigHelp.signatures.push_back(ProcessOverloadCandidate(
Candidate, *CCS,
- getParameterDocComment(S.getASTContext(), Candidate, CurrentArg)));
+ getParameterDocComment(S.getASTContext(), Candidate, CurrentArg,
+ /*CommentsFromHeader=*/false)));
}
}
@@ -1030,7 +1032,8 @@ private:
SemaCCS = Recorder->codeCompletionString(*SR);
if (Opts.IncludeComments) {
assert(Recorder->CCSema);
-DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR);
+DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR,
+ /*CommentsFromHeader=*/false);
}
}
return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get(),
DocComment);
Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=333189&r1=333188&r2=333189&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Thu May 24
07:49:23 2018
@@ -10,6 +10,7 @@
#include "CodeCompletionStrings.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/RawCommentList.h"
+#include "clang/Basic/SourceManager.h"
#include
namespace clang {
@@ -122,10 +123,23 @@ void processSnippetChunks(const CodeComp
}
}
+std::string getFormattedComment(const ASTContext &Ctx, const RawComment &RC,
+bool CommentsFromHeaders) {
+ auto &SourceMgr = Ctx.getSourceManager();
+ // Parsing comments 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.
+ if (!CommentsFromHeaders && !SourceMgr.isWrittenInMainFile(RC.getLocStart()))
+return "";
+ return RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+}
} // namespace
std::string getDocComment(const ASTContext &Ctx,
- const CodeCompletionResult &Result) {
+ const CodeCompletionResult &Result,
+ bool CommentsFromHeaders) {
// FIXME: clang's completion also returns documentation for RK_Pattern if
they
// contain a pattern for ObjC properties. Unfortunately, there is no API to
// get this declaration, so we don't show documentation in that case.
@@ -137,20 +151,20 @@ std::string getDocComment(const ASTConte
const RawComment *RC = getCompletionComment(Ctx, Decl);
if (!RC)
return "";
- return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+ return getFormattedComment(Ctx,
[PATCH] D47274: [clangd] Serve comments for headers decls from dynamic index only
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333189: [clangd] Serve comments for headers decls from
dynamic index only (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47274
Files:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -586,9 +586,11 @@
const auto *CCS = Candidate.CreateSignatureString(
CurrentArg, S, *Allocator, CCTUInfo, true);
assert(CCS && "Expected the CodeCompletionString to be non-null");
+ // FIXME: for headers, we need to get a comment from the index.
SigHelp.signatures.push_back(ProcessOverloadCandidate(
Candidate, *CCS,
- getParameterDocComment(S.getASTContext(), Candidate, CurrentArg)));
+ getParameterDocComment(S.getASTContext(), Candidate, CurrentArg,
+ /*CommentsFromHeader=*/false)));
}
}
@@ -1030,7 +1032,8 @@
SemaCCS = Recorder->codeCompletionString(*SR);
if (Opts.IncludeComments) {
assert(Recorder->CCSema);
-DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR);
+DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR,
+ /*CommentsFromHeader=*/false);
}
}
return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get(), DocComment);
Index: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
===
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
@@ -10,6 +10,7 @@
#include "CodeCompletionStrings.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/RawCommentList.h"
+#include "clang/Basic/SourceManager.h"
#include
namespace clang {
@@ -122,10 +123,23 @@
}
}
+std::string getFormattedComment(const ASTContext &Ctx, const RawComment &RC,
+bool CommentsFromHeaders) {
+ auto &SourceMgr = Ctx.getSourceManager();
+ // Parsing comments 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.
+ if (!CommentsFromHeaders && !SourceMgr.isWrittenInMainFile(RC.getLocStart()))
+return "";
+ return RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+}
} // namespace
std::string getDocComment(const ASTContext &Ctx,
- const CodeCompletionResult &Result) {
+ const CodeCompletionResult &Result,
+ bool CommentsFromHeaders) {
// FIXME: clang's completion also returns documentation for RK_Pattern if they
// contain a pattern for ObjC properties. Unfortunately, there is no API to
// get this declaration, so we don't show documentation in that case.
@@ -137,20 +151,20 @@
const RawComment *RC = getCompletionComment(Ctx, Decl);
if (!RC)
return "";
- return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+ return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
}
std::string
getParameterDocComment(const ASTContext &Ctx,
const CodeCompleteConsumer::OverloadCandidate &Result,
- unsigned ArgIndex) {
+ unsigned ArgIndex, bool CommentsFromHeaders) {
auto Func = Result.getFunction();
if (!Func)
return "";
const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
if (!RC)
return "";
- return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+ return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
}
void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -389,7 +389,8 @@
/*EnableSnippets=*/false);
std::string FilterText = getFilterText(*CCS);
std::string Documentation =
- formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion));
+ formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
sammccall added a comment.
Thanks, this looks a lot better!
There do seem to be a lot of little classes that exist exactly one-per-TU
(ASTWorker, ASTBuilder, CachedAST, to a lesser extent ParsedAST) and I'm not
sure the balance of responsibilities is quite right. Some comments below.
Comment at: clangd/ClangdUnit.h:132
-/// Manages resources, required by clangd. Allows to rebuild file with new
-/// contents, and provides AST and Preamble for it.
-class CppFile {
+/// A helper class that handles building preambles and ASTs for a file. Also
+/// adds some logging.
ilya-biryukov wrote:
> sammccall wrote:
> > This may be change aversion, but I'm not sure this class does enough after
> > this change - it doesn't store the inputs or the outputs/cache, so it kind
> > of seems like it wants to be a function.
> >
> > I guess the motivation here is that storing the outputs means dealing with
> > the cache, and the cache is local to TUScheduler.
> > But `CppFile` is only used in TUScheduler, so we could move this there too?
> > It feels like expanding the scope more than I'd like.
> >
> > The upside is that I think it's a more natural division of responsibility:
> > `CppFile` could continue to be the "main" holder of the
> > `shared_ptr` (which we don't limit, but share), and instead of
> > `Optional` it'd have a `weak_ptr` which is controlled
> > and can be refreshed through the cache.
> >
> > As discussed offline, the cache could look something like:
> > ```
> > class Cache {
> >shared_ptr put(ParsedAST);
> >void hintUsed(ParsedAST*); // optional, bumps LRU when client reads
> >void hintUnused(ParsedAST*); // optional, releases when client abandons
> > }
> >
> > shared_ptr CppFile::getAST() {
> > shared_ptr AST = WeakAST.lock();
> > if (AST)
> > Cache.hintUsed(AST.get());
> > else
> > WeakAST = AST = Cache.put(build...);
> > return AST;
> > }
> > ```
> I've reimplemented the cache with weak_ptr/shared_ptr.
> With a slightly different interface to hide more stuff in the cache API.
> Please take a look and let me know what you think.
>
> Nevertheless, I still find `CppFile` refactoring useful. It unties preambles
> from the ASTs and I believe this is a good thing, given that their lifetimes
> are different.
I'm not sure how much they were tangled before, they were computed in the same
place, but accessed separately, and it's not sure you ever *want* to compute
them separately? (possibly in unit tests?)
I do think making ASTWorker maintain the old preamble and pass it in is
confusing. The remaining members are trivial and unrelated enough that I do
think if the references to the preamble/ast are removed, then moving the
remaining members to ASTWorker or to a dumb struct, and making these free
functions would make it easier to navigate the class structure.
Comment at: clangd/TUScheduler.cpp:71
+
+ /// Update the function used to compute the value.
+ void update(std::function()> ComputeF);
I think I understand this more as "updates the value" but the value is lazy...
I find this API somewhat hard to follow, maybe just because it's unfamiliar.
I've mostly seen cache APIs look like one of:
1. `Cache(function Compute)`, `Value Cache::get(Input)`
2. `void Cache::put(Key, Value)`, `Optional Cache::get(Key)`
3. `Handle Cache::put(Value)`, `Optional Handle::get()`
This one is `Slot Cache::create()`, `void Slot::update(function
LazyV)`, `Value Slot::get()`.
It's similar-ish to 3, but has 3 nontrivial operations instead of 2, and the
slot object is externally mutable instead of immutable, so it seems more
complex. What does it buy us in exchange?
(1 & 2 work well with a natural key or inputs that are easy to compare, which
we don't particularly have)
Comment at: clangd/TUScheduler.cpp:91
+/// Provides an LRU cache of ASTs.
+class TUScheduler::IdleASTs {
+ friend class CachedAST;
naming: `IdleASTs` doesn't mention the function of this class, which is a cache.
I'd suggest swapping the names: call the class `ASTCache` and the *instance*
`IdleASTs` as that reflects its role within the TUScheduler.
For `CachedAST`, I think the relationship would be well-exposed by nesting it
as `ASTCache::Entry`. This also gives you the `friend` for free, which seems
like a hint that it's an appropriate structure.
(Though I'm not sure CachedAST is that useful)
Comment at: clangd/TUScheduler.cpp:153
+ /// one.
+ std::vector>>>
+ LRU; /* GUARDED_BY(Mut) */
as discussed offline, using a CachedAST* as a key shouldn't be necessary, the
ParsedAST* should be enough I think.
Comment at: clangd/TUScheduler.cpp:157
+
+CachedAST::~CachedAST() { Owner.remove(*this); }
+
document why
Comment at: clangd/TUScheduler.h:48
+/// kept in memory.
+struct ASTRetentionParams {
+ /// Maximum numbe
r333179 - [analyzer] Move RangeSet related declarations into the RangedConstraintManager header.
Author: mramalho
Date: Thu May 24 05:16:35 2018
New Revision: 333179
URL: http://llvm.org/viewvc/llvm-project?rev=333179&view=rev
Log:
[analyzer] Move RangeSet related declarations into the RangedConstraintManager
header.
Summary: I could also move `RangedConstraintManager.h` under `include/` if you
agree as it seems slightly out of place under `lib/`.
Patch by Réka Kovács
Reviewers: NoQ, george.karpenkov, dcoughlin, rnkovacs
Reviewed By: NoQ
Subscribers: mikhail.ramalho, whisperity, xazax.hun, baloghadamsoftware,
szepet, a.sidorin, dkrupp, cfe-commits
Differential Revision: https://reviews.llvm.org/D45920
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RangedConstraintManager.h
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp?rev=333179&r1=333178&r2=333179&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp Thu May 24
05:16:35 2018
@@ -23,263 +23,171 @@
using namespace clang;
using namespace ento;
-/// A Range represents the closed range [from, to]. The caller must
-/// guarantee that from <= to. Note that Range is immutable, so as not
-/// to subvert RangeSet's immutability.
-namespace {
-class Range : public std::pair {
-public:
- Range(const llvm::APSInt &from, const llvm::APSInt &to)
- : std::pair(&from, &to) {
-assert(from <= to);
- }
- bool Includes(const llvm::APSInt &v) const {
-return *first <= v && v <= *second;
- }
- const llvm::APSInt &From() const { return *first; }
- const llvm::APSInt &To() const { return *second; }
- const llvm::APSInt *getConcreteValue() const {
-return &From() == &To() ? &From() : nullptr;
- }
-
- void Profile(llvm::FoldingSetNodeID &ID) const {
-ID.AddPointer(&From());
-ID.AddPointer(&To());
- }
-};
-
-class RangeTrait : public llvm::ImutContainerInfo {
-public:
- // When comparing if one Range is less than another, we should compare
- // the actual APSInt values instead of their pointers. This keeps the order
- // consistent (instead of comparing by pointer values) and can potentially
- // be used to speed up some of the operations in RangeSet.
- static inline bool isLess(key_type_ref lhs, key_type_ref rhs) {
-return *lhs.first < *rhs.first ||
- (!(*rhs.first < *lhs.first) && *lhs.second < *rhs.second);
- }
-};
-
-/// RangeSet contains a set of ranges. If the set is empty, then
-/// there the value of a symbol is overly constrained and there are no
-/// possible values for that symbol.
-class RangeSet {
- typedef llvm::ImmutableSet PrimRangeSet;
- PrimRangeSet ranges; // no need to make const, since it is an
- // ImmutableSet - this allows default operator=
- // to work.
-public:
- typedef PrimRangeSet::Factory Factory;
- typedef PrimRangeSet::iterator iterator;
-
- RangeSet(PrimRangeSet RS) : ranges(RS) {}
-
- /// Create a new set with all ranges of this set and RS.
- /// Possible intersections are not checked here.
- RangeSet addRange(Factory &F, const RangeSet &RS) {
-PrimRangeSet Ranges(RS.ranges);
-for (const auto &range : ranges)
- Ranges = F.add(Ranges, range);
-return RangeSet(Ranges);
- }
-
- iterator begin() const { return ranges.begin(); }
- iterator end() const { return ranges.end(); }
-
- bool isEmpty() const { return ranges.isEmpty(); }
-
- /// Construct a new RangeSet representing '{ [from, to] }'.
- RangeSet(Factory &F, const llvm::APSInt &from, const llvm::APSInt &to)
- : ranges(F.add(F.getEmptySet(), Range(from, to))) {}
-
- /// Profile - Generates a hash profile of this RangeSet for use
- /// by FoldingSet.
- void Profile(llvm::FoldingSetNodeID &ID) const { ranges.Profile(ID); }
-
- /// getConcreteValue - If a symbol is contrained to equal a specific integer
- /// constant then this method returns that value. Otherwise, it returns
- /// NULL.
- const llvm::APSInt *getConcreteValue() const {
-return ranges.isSingleton() ? ranges.begin()->getConcreteValue() : nullptr;
- }
+void RangeSet::IntersectInRange(BasicValueFactory &BV, Factory &F,
+ const llvm::APSInt &Lower, const llvm::APSInt &Upper,
+ PrimRangeSet &newRanges, PrimRangeSet::iterator &i,
+ PrimRangeSet::iterator &e) const {
+ // There are six cases for each range R in the set:
+ // 1. R is entirely before the intersection range.
+ // 2. R is entirely after the intersection range.
+ // 3. R contains the entire intersection range.
+ // 4. R starts before the intersection range and ends in the middle.
+ // 5. R starts in the middle of the intersection range and ends after it.
+ // 6. R
[PATCH] D47067: Update NRVO logic to support early return
tzik updated this revision to Diff 148417.
tzik added a comment.
Add AST test. computeNRVO unconditionally.
Repository:
rC Clang
https://reviews.llvm.org/D47067
Files:
include/clang/AST/Decl.h
include/clang/Sema/Scope.h
lib/Sema/Scope.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaStmt.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriterDecl.cpp
test/CodeGenCXX/nrvo.cpp
test/SemaCXX/nrvo-ast.cpp
Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+ X();
+ X(const X&);
+ X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ if (b)
+return x;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+ // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+ X y;
+ if (b)
+return y;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+ }
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+ {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+ return x;
+ }
+ exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+ try {
+may_throw();
+return X();
+ } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+ }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+ // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+ X x __attribute__((aligned(8)));
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+ }
+ return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+ } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+ }
+}
+
+template
+struct Y {
+ Y();
+ // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+ // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+ // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+ // CHECK: VarDecl {{.*}} y 'Y' nrvo
+ static Y test_09() {
+Y y;
+return y;
+ }
+};
+
+struct Z {
+ Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template
+A test_10() {
+ B b;
+ return b;
+}
+
+void instantiate() {
+ Y::test_09();
+ test_10();
+ test_10();
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
}
// CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
- // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
- // CHECK: call {{.*}} @_ZN1XC1Ev
- // CHECK: call {{.*}} @_ZN1XC1ERKS_
if (B) {
X y;
return y;
}
- // FIXME: we should NRVO this variable too.
- X x;
+ // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
return x;
}
@@ -191,21 +187,29 @@
}
// CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
X test7(bool b) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
// CHECK-NEXT: ret
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NEXT: ret
if (b) {
X x;
return x;
}
return X();
}
// CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
X test8(bool b) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
// CHECK-NEXT: ret
- if (b) {
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NEXT: ret
+if (b) {
X x;
return x;
} else {
@@ -221,4 +225,37 @@
// CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
// CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+ if (B) {
+// CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+// CHECK-EH: tail call {{.*}} @_ZN1XC1ERKS_
+return x;
+ }
+ // CHECK: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NOT: call {{.*}} @_ZN1XC1ERKS_
+ X y;
+ return y;
+}
+
+// CHECK-LABEL: define {{.*}} void @_Z6test11I1XET_v
+// CHECK-EH-L
[PATCH] D47067: Update NRVO logic to support early return
tzik added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:12760
// to deduce an implicit return type.
- if (FD->getReturnType()->isRecordType() &&
- (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+ if (!FD->getReturnType()->isScalarType())
computeNRVO(Body, getCurFunction());
Quuxplusone wrote:
> rsmith wrote:
> > tzik wrote:
> > > Quuxplusone wrote:
> > > > What is the purpose of this change?
> > > > If it's "no functional change" it should be done separately IMHO; if it
> > > > is supposed to change codegen, then it needs some codegen tests. (From
> > > > looking at the code: maybe this affects codegen on functions that
> > > > return member function pointers by value?)
> > > I think the previous implementation was incorrect.
> > > Though computeNRVO clears ReturnStmt::NRVOCandidate when the
> > > corresponding variable is not NRVO variable, CGStmt checks both of
> > > ReturnStmt::NRVOCandidate and VarDecl::NRVOVariable anyway.
> > > So, computeNRVO took no effect in the previous code, and the absence of
> > > computeNRVO here on function templates did not matter.
> > > Note that there was no chance to call computeNRVO on function template
> > > elsewhere too.
> > >
> > > OTOH in the new implementation, computeNRVO is necessary, and its absence
> > > on function templates matters.
> > >
> > > We can remove computeNRVO here separately as a NFC patch and readd the
> > > new impl to the same place, but it's probably less readable, IMO.
> > What happens if we can't tell whether we have an NRVO candidate or not
> > until instantiation:
> >
> > ```
> > template X f() {
> > T v;
> > return v; // is an NRVO candidate if T is X, otherwise is not
> > }
> > ```
> >
> > (I think this is not hard to handle: the dependent construct here can only
> > affect whether you get NRVO at all, not which variable you perform NRVO on,
> > but I want to check that it is handled properly.)
> Ah, so if I understand correctly — which I probably don't —...
> the parts of this diff related to `isRecordType()` make NRVO more reliable on
> template functions? Because the existing code has lots of checks for
> `isRecordType()` which means that they don't run for dependent
> (as-yet-unknown) types, even if those types are going to turn out to be
> record types occasionally?
>
> I see that line 12838 runs `computeNRVO` unconditionally for *all*
> ObjCMethodDecls, //even// ones with scalar return types. So maybe running
> `computeNRVO` unconditionally right here would also be correct?
> Datapoint: I made a patch to do nothing but replace this condition with `if
> (Body)` and to remove the similar condition guarding `computeNRVO` in
> `Sema::ActOnBlockStmtExpr`. It passed the Clang test suite with flying
> colors. I don't know if this means that the test suite is missing some tests,
> or if it means that the conditions are unnecessary.
>
> IMHO if you're changing this anyway, it would be nice to put the remaining
> condition/optimization `if (getReturnType().isScalarType()) return;` inside
> `computeNRVO` itself, instead of repeating that condition at every call site.
`v` here is marked as an NRVO variable in its function template AST, and that
propagates to the instantiation if the return type is compatible.
https://github.com/llvm-mirror/clang/blob/9c82d4ff6015e4477e924c8a495a834c2fced29e/lib/Sema/SemaTemplateInstantiateDecl.cpp#L743
Comment at: lib/Sema/SemaDecl.cpp:12760
// to deduce an implicit return type.
- if (FD->getReturnType()->isRecordType() &&
- (!getLangOpts().CPlusPlus || !FD->isDependentContext()))
+ if (!FD->getReturnType()->isScalarType())
computeNRVO(Body, getCurFunction());
tzik wrote:
> Quuxplusone wrote:
> > rsmith wrote:
> > > tzik wrote:
> > > > Quuxplusone wrote:
> > > > > What is the purpose of this change?
> > > > > If it's "no functional change" it should be done separately IMHO; if
> > > > > it is supposed to change codegen, then it needs some codegen tests.
> > > > > (From looking at the code: maybe this affects codegen on functions
> > > > > that return member function pointers by value?)
> > > > I think the previous implementation was incorrect.
> > > > Though computeNRVO clears ReturnStmt::NRVOCandidate when the
> > > > corresponding variable is not NRVO variable, CGStmt checks both of
> > > > ReturnStmt::NRVOCandidate and VarDecl::NRVOVariable anyway.
> > > > So, computeNRVO took no effect in the previous code, and the absence of
> > > > computeNRVO here on function templates did not matter.
> > > > Note that there was no chance to call computeNRVO on function template
> > > > elsewhere too.
> > > >
> > > > OTOH in the new implementation, computeNRVO is necessary, and its
> > > > absence on function templates matters.
> > > >
> > > > We can remove computeNRVO here separately as a NFC patch and readd the
> > >
[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes
ilya-biryukov added a comment. Added the test. Comment at: clangd/index/FileIndex.cpp:37 + std::vector TopLevelDecls( + AST.getTranslationUnitDecl()->decls().begin(), + AST.getTranslationUnitDecl()->decls().end()); ioeric wrote: > ilya-biryukov wrote: > > ioeric wrote: > > > Would this give us decls in namespaces? It seems that > > > `AST.getTranslationUnitDecl()->decls()` are only decls that are > > > immediately inside the TU context. > > I'll double check and add a test for that, but I think the indexer visits > > the namespaces that we provide, I'm not sure if we have tests though. > Oh, right! In that case, it seems that a single TU decl which contains all > interesting decls would be sufficient? Is it possible that preamble doesn't > have all available top level decls in TU decl? This seems to be possible for > ParsedAST, which is why we used `getTopLevelDecls`, I think? > In that case, it seems that a single TU decl which contains all interesting > decls would be sufficient? I've tried doing the TU decl only and it does not work, because `indexTopLevelDecl` checks that location of decl that you pass to it is valid xD. Obviously, that's not the case for the TU decl. But it does visit the namespaces recursively, new test also checks for that, so we should be fine with the current code. > This seems to be possible for ParsedAST, which is why we used > getTopLevelDecls, I think? `getTopLevelDecls` was copied from the `ASTUnit` without much thought. IIRC, it allows less deserialization from preamble when traversing the AST for things like "go to def", etc. This is definitely not something we're optimizing for here, since we run on the preamble AST itself. Now that I think about it, I don't think we need to store top-level decls of preamble at all for any of our features... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes
ilya-biryukov updated this revision to Diff 148421.
ilya-biryukov added a comment.
- Added a test for preamble rebuilds.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47272
Files:
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
unittests/clangd/FileIndexTests.cpp
unittests/clangd/TUSchedulerTests.cpp
unittests/clangd/TestTU.cpp
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -43,7 +43,7 @@
SymbolSlab TestTU::headerSymbols() const {
auto AST = build();
- return indexAST(&AST);
+ return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
}
std::unique_ptr TestTU::index() const {
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -42,7 +42,7 @@
TEST_F(TUSchedulerTests, MissingFiles) {
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
auto Added = testPath("added.cpp");
@@ -98,7 +98,7 @@
TUScheduler S(
getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
auto Path = testPath("foo.cpp");
S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
@@ -126,7 +126,7 @@
{
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- /*ASTParsedCallback=*/nullptr,
+ /*PreambleParsedCallback=*/nullptr,
/*UpdateDebounce=*/std::chrono::seconds(1));
// FIXME: we could probably use timeouts lower than 1 second here.
auto Path = testPath("foo.cpp");
@@ -157,7 +157,7 @@
{
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
- /*ASTParsedCallback=*/nullptr,
+ /*PreambleParsedCallback=*/nullptr,
/*UpdateDebounce=*/std::chrono::milliseconds(50));
std::vector Files;
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -7,8 +7,13 @@
//
//===--===//
+#include "ClangdUnit.h"
+#include "TestFS.h"
#include "TestTU.h"
#include "index/FileIndex.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/CompilationDatabase.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -87,7 +92,7 @@
File.HeaderFilename = (Basename + ".h").str();
File.HeaderCode = Code;
auto AST = File.build();
- M.update(File.Filename, &AST);
+ M.update(File.Filename, &AST.getASTContext(), AST.getPreprocessorPtr());
}
TEST(FileIndexTest, IndexAST) {
@@ -129,13 +134,13 @@
Req.Scopes = {"ns::"};
EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
- M.update("f1.cpp", nullptr);
+ M.update("f1.cpp", nullptr, nullptr);
EXPECT_THAT(match(M, Req), UnorderedElementsAre());
}
TEST(FileIndexTest, RemoveNonExisting) {
FileIndex M;
- M.update("no.cpp", nullptr);
+ M.update("no.cpp", nullptr, nullptr);
EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre());
}
@@ -200,6 +205,58 @@
EXPECT_TRUE(SeenMakeVector);
}
+TEST(FileIndexTest, RebuildWithPreamble) {
+ auto FooCpp = testPath("foo.cpp");
+ auto FooH = testPath("foo.h");
+ FileIndex Index;
+ bool IndexUpdated = false;
+ CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
+ std::make_shared(),
+ [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
+ std::shared_ptr PP) {
+ EXPECT_FALSE(IndexUpdated)
+ << "Expected only a single index update";
+ IndexUpdated = true;
+ Index.update(FilePath, &Ctx, std::move(PP));
+ });
+
+ // Preparse ParseInputs.
+ ParseInputs PI;
+ PI.CompileCommand.Directory = testRoot();
+ PI.CompileCommand.Filename = FooCpp;
+ PI.CompileCommand.CommandLine = {"clang", "-xc++", FooCpp};
+
+ llvm::StringMap Files;
+ Files[FooCpp] = "";
+ Files[FooH] = R"cpp(
+namespace ns_in_header {
+ int func_in_header();
+}
+ )cpp";
+ PI.FS = buildTestFS(std::move(Files));
+
+ PI.Contents = R"cpp(
+#in
[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst
probinson added a comment. Can we step back a second and better explain what the problem is? With current Clang the debug info for this function looks okay to me. The store that is "missing" a debug location is homing the formal parameter to its local stack location; this is part of prolog setup, not "real" code. Repository: rC Clang https://reviews.llvm.org/D47097 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lg. Thanks for adding the test! Comment at: clangd/index/FileIndex.cpp:37 + std::vector TopLevelDecls( + AST.getTranslationUnitDecl()->decls().begin(), + AST.getTranslationUnitDecl()->decls().end()); ilya-biryukov wrote: > ioeric wrote: > > ilya-biryukov wrote: > > > ioeric wrote: > > > > Would this give us decls in namespaces? It seems that > > > > `AST.getTranslationUnitDecl()->decls()` are only decls that are > > > > immediately inside the TU context. > > > I'll double check and add a test for that, but I think the indexer visits > > > the namespaces that we provide, I'm not sure if we have tests though. > > Oh, right! In that case, it seems that a single TU decl which contains all > > interesting decls would be sufficient? Is it possible that preamble doesn't > > have all available top level decls in TU decl? This seems to be possible > > for ParsedAST, which is why we used `getTopLevelDecls`, I think? > > In that case, it seems that a single TU decl which contains all interesting > > decls would be sufficient? > I've tried doing the TU decl only and it does not work, because > `indexTopLevelDecl` checks that location of decl that you pass to it is valid > xD. Obviously, that's not the case for the TU decl. But it does visit the > namespaces recursively, new test also checks for that, so we should be fine > with the current code. > > > This seems to be possible for ParsedAST, which is why we used > > getTopLevelDecls, I think? > `getTopLevelDecls` was copied from the `ASTUnit` without much thought. > IIRC, it allows less deserialization from preamble when traversing the AST > for things like "go to def", etc. This is definitely not something we're > optimizing for here, since we run on the preamble AST itself. > Now that I think about it, I don't think we need to store top-level decls of > preamble at all for any of our features... Sounds good. Thanks for the clarification! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333196 - [clangd] Build index on preamble changes instead of the AST changes
Author: ibiryukov
Date: Thu May 24 08:50:15 2018
New Revision: 333196
URL: http://llvm.org/viewvc/llvm-project?rev=333196&view=rev
Log:
[clangd] Build index on preamble changes instead of the AST changes
Summary:
This is more efficient and avoids data races when reading files that
come from the preamble. The staleness can occur when reading a file
from disk that changed after the preamble was built. This can lead to
crashes, e.g. when parsing comments.
We do not to rely on symbols from the main file anyway, since any info
that those provide can always be taken from the AST.
Reviewers: ioeric, sammccall
Reviewed By: ioeric
Subscribers: malaperle, klimek, javed.absar, MaskRay, jkorous, cfe-commits
Differential Revision: https://reviews.llvm.org/D47272
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=333196&r1=333195&r2=333196&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu May 24 08:50:15 2018
@@ -17,6 +17,7 @@
#include "clang/Format/Format.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
#include "clang/Tooling/Refactoring/Rename/RenamingAction.h"
@@ -92,12 +93,14 @@ ClangdServer::ClangdServer(GlobalCompila
// is parsed.
// FIXME(ioeric): this can be slow and we may be able to index on less
// critical paths.
- WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
-FileIdx
-? [this](PathRef Path,
- ParsedAST *AST) { FileIdx->update(Path, AST);
}
-: ASTParsedCallback(),
-Opts.UpdateDebounce) {
+ WorkScheduler(
+ Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
+ FileIdx
+ ? [this](PathRef Path, ASTContext &AST,
+ std::shared_ptr
+ PP) { FileIdx->update(Path, &AST, std::move(PP)); }
+ : PreambleParsedCallback(),
+ Opts.UpdateDebounce) {
if (FileIdx && Opts.StaticIndex) {
MergedIndex = mergeIndex(FileIdx.get(), Opts.StaticIndex);
Index = MergedIndex.get();
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=333196&r1=333195&r2=333196&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Thu May 24 08:50:15 2018
@@ -13,6 +13,8 @@
#include "Logger.h"
#include "SourceCode.h"
#include "Trace.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/LangOptions.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/FrontendActions.h"
@@ -25,7 +27,6 @@
#include "clang/Sema/Sema.h"
#include "clang/Serialization/ASTWriter.h"
#include "clang/Tooling/CompilationDatabase.h"
-#include "clang/Basic/LangOptions.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/CrashRecoveryContext.h"
@@ -86,6 +87,9 @@ private:
class CppFilePreambleCallbacks : public PreambleCallbacks {
public:
+ CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
+ : File(File), ParsedCallback(ParsedCallback) {}
+
std::vector takeTopLevelDeclIDs() {
return std::move(TopLevelDeclIDs);
}
@@ -102,6 +106,13 @@ public:
}
}
+ void AfterExecute(CompilerInstance &CI) override {
+if (!ParsedCallback)
+ return;
+trace::Span Tracer("Running PreambleCallback");
+ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr());
+ }
+
void HandleTopLevelDecl(DeclGroupRef DG) override {
for (Decl *D : DG) {
if (isa(D))
@@ -122,6 +133,8 @@ public:
}
private:
+ PathRef File;
+ PreambleParsedCallback ParsedCallback;
std::vector TopLevelDecls;
std::vector TopLevelDeclIDs;
std::vector Inclusions;
@@ -277,9 +290,9 @@ ParsedAST::P
[PATCH] D47272: [clangd] Build index on preamble changes instead of the AST changes
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE333196: [clangd] Build index on preamble changes instead
of the AST changes (authored by ibiryukov, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47272?vs=148421&id=148424#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47272
Files:
clangd/ClangdServer.cpp
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
unittests/clangd/FileIndexTests.cpp
unittests/clangd/TUSchedulerTests.cpp
unittests/clangd/TestTU.cpp
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -7,8 +7,13 @@
//
//===--===//
+#include "ClangdUnit.h"
+#include "TestFS.h"
#include "TestTU.h"
#include "index/FileIndex.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/CompilationDatabase.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -87,7 +92,7 @@
File.HeaderFilename = (Basename + ".h").str();
File.HeaderCode = Code;
auto AST = File.build();
- M.update(File.Filename, &AST);
+ M.update(File.Filename, &AST.getASTContext(), AST.getPreprocessorPtr());
}
TEST(FileIndexTest, IndexAST) {
@@ -129,13 +134,13 @@
Req.Scopes = {"ns::"};
EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
- M.update("f1.cpp", nullptr);
+ M.update("f1.cpp", nullptr, nullptr);
EXPECT_THAT(match(M, Req), UnorderedElementsAre());
}
TEST(FileIndexTest, RemoveNonExisting) {
FileIndex M;
- M.update("no.cpp", nullptr);
+ M.update("no.cpp", nullptr, nullptr);
EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre());
}
@@ -200,6 +205,58 @@
EXPECT_TRUE(SeenMakeVector);
}
+TEST(FileIndexTest, RebuildWithPreamble) {
+ auto FooCpp = testPath("foo.cpp");
+ auto FooH = testPath("foo.h");
+ FileIndex Index;
+ bool IndexUpdated = false;
+ CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
+ std::make_shared(),
+ [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
+ std::shared_ptr PP) {
+ EXPECT_FALSE(IndexUpdated)
+ << "Expected only a single index update";
+ IndexUpdated = true;
+ Index.update(FilePath, &Ctx, std::move(PP));
+ });
+
+ // Preparse ParseInputs.
+ ParseInputs PI;
+ PI.CompileCommand.Directory = testRoot();
+ PI.CompileCommand.Filename = FooCpp;
+ PI.CompileCommand.CommandLine = {"clang", "-xc++", FooCpp};
+
+ llvm::StringMap Files;
+ Files[FooCpp] = "";
+ Files[FooH] = R"cpp(
+namespace ns_in_header {
+ int func_in_header();
+}
+ )cpp";
+ PI.FS = buildTestFS(std::move(Files));
+
+ PI.Contents = R"cpp(
+#include "foo.h"
+namespace ns_in_source {
+ int func_in_source();
+}
+ )cpp";
+
+ // Rebuild the file.
+ File.rebuild(std::move(PI));
+ ASSERT_TRUE(IndexUpdated);
+
+ // Check the index contains symbols from the preamble, but not from the main
+ // file.
+ FuzzyFindRequest Req;
+ Req.Query = "";
+ Req.Scopes = {"", "ns_in_header::"};
+
+ EXPECT_THAT(
+ match(Index, Req),
+ UnorderedElementsAre("ns_in_header", "ns_in_header::func_in_header"));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -43,7 +43,7 @@
SymbolSlab TestTU::headerSymbols() const {
auto AST = build();
- return indexAST(&AST);
+ return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
}
std::unique_ptr TestTU::index() const {
Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -42,7 +42,7 @@
TEST_F(TUSchedulerTests, MissingFiles) {
TUScheduler S(getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
auto Added = testPath("added.cpp");
@@ -98,7 +98,7 @@
TUScheduler S(
getDefaultAsyncThreadsCount(),
/*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr,
+/*PreambleParsedCallback=*/nullptr,
/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
auto Path = testPath("foo.cpp");
S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
@@ -126,7 +126,7
[PATCH] D47187: [clangd] Skip .inc headers when canonicalizing header #include.
malaperle added subscribers: ioeric, sammccall, malaperle. malaperle added a comment. I think this might have broken the "shared lib" build? Could you double check that you don't need to add "clangDriver" ? ../tools/clang/tools/extra/clangd/index/CanonicalIncludes.cpp:61: error: undefined reference to 'clang::driver::types::lookupTypeForExtension(llvm::StringRef)' ../tools/clang/tools/extra/clangd/index/CanonicalIncludes.cpp:63: error: undefined reference to 'clang::driver::types::onlyPrecompileType(clang::driver::types::ID)' Thanks! From: cfe-commits on behalf of Eric Liu via Phabricator via cfe-commits Sent: Thursday, May 24, 2018 10:44:26 AM To: ioe...@google.com; sammcc...@google.com Cc: mask...@google.com; llvm-comm...@lists.llvm.org; cfe-commits@lists.llvm.org Subject: [PATCH] https://reviews.llvm.org/D47187: [clangd] Skip .inc headers when canonicalizing header #include. This revision was automatically updated to reflect the committed changes. Closed by commit https://reviews.llvm.org/rL333188: [clangd] Skip .inc headers when canonicalizing header #include. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47187 Files: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Repository: rL LLVM https://reviews.llvm.org/D47187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333197 - [clangd] Add clangDriver dependency after r333188
Author: ioeric Date: Thu May 24 08:54:32 2018 New Revision: 333197 URL: http://llvm.org/viewvc/llvm-project?rev=333197&view=rev Log: [clangd] Add clangDriver dependency after r333188 Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=333197&r1=333196&r2=333197&view=diff == --- clang-tools-extra/trunk/clangd/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu May 24 08:54:32 2018 @@ -47,6 +47,7 @@ add_clang_library(clangDaemon clangAST clangASTMatchers clangBasic + clangDriver clangFormat clangFrontend clangIndex ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46943: [clangd] Boost scores for decls from current file in completion
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Nice, simple and will admit refinements later.
Just test nits and a trivial organizational thing.
Comment at: clangd/Quality.cpp:22
+namespace {
+bool hasDeclInMainFile(const Decl &D) {
nit: per coding style use static for functions
(I'm not sure it's a great rule, but since the ns only has this function...)
Comment at: clangd/Quality.h:52
unsigned References = 0;
+ float ProximityScore = 0.0; /// Proximity score, in a [0,1] interval.
this belongs in `SymbolRelevanceSignals` rather than this struct. In principle,
SymbolQualitySignals should all be stuff we can store in a global index (which
is the point of the split). I should probably add a comment to that effect :-)
You could be a little more specific on the semantics, e.g. "Proximity between
the best declaration and the query location. [0-1] score where 1 is closest."
Comment at: unittests/clangd/QualityTests.cpp:96
TEST(QualityTests, SymbolRelevanceSignalsSanity) {
SymbolRelevanceSignals Default;
please add a test for proximity here
Comment at: unittests/clangd/QualityTests.cpp:121
+TEST(QualityTests, BoostCurrentFileDecls) {
+ TestTU Test;
consider merging into SymbolRelevanceSignalsExtraction test, which tests the
same entrypoint.
If not, move up next to that one.
Comment at: unittests/clangd/QualityTests.cpp:129
+ Test.Code = R"cpp(
+#include "foo.h"
+int ::test_func_in_header_and_cpp() {
this is not needed, the `#include` is implicit in TestTU
(and so you don't need to specify HeaderFilename either)
Comment at: unittests/clangd/QualityTests.cpp:155
+
+ /// Check the boost in proximity translates into a better score.
+ EXPECT_LE(FuncInHeader.evaluate(), FuncInCpp.evaluate());
this tests end-to-end, but the other tests verify input -> signals and signal
-> score separately.
I'd prefer to keep (only) doing that, for consistency and because it's
important we know/assert precisely what each half does so we can actually debug.
Comment at: unittests/clangd/TestTU.cpp:80
continue;
-if (Result) {
+if (Result && ND->getCanonicalDecl() != Result) {
ADD_FAILURE() << "Multiple Decls named " << QName;
well, I definitely wanted to flag this as an error (for the tests where this
function was introduced).
Actually I think this is wrong for your test anyway - don't you want to test
that no matter which decl the code completion result refers to, you get the
same boost?
I'd suggest adding a `findDecls()` function that returns a vector,
and implementing `findDecl()` in terms of it. In the
`test_func_in_header_and_cpp` test, you could loop over `findDecls()` and run
the assertions each time.
(I guess findDecls() should assert that the returned vector is non-empty?
Slightly weird but might catch accidentally vacuous tests)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46943
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble
ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall.
Herald added subscribers: jkorous, MaskRay, mehdi_amini, klimek.
They cause lots of deserialization and are not actually used.
The ParsedAST accessor that previously returned those was renamed from
getTopLevelDecls to getLocalTopLevelDecls in order to avoid
confusion.
This change should considerably improve the speed of findDefinition
and other features that try to find AST nodes, corresponding to the
locations in the source code.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47331
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/XRefs.cpp
unittests/clangd/TestTU.cpp
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -74,7 +74,7 @@
const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
const NamedDecl *Result = nullptr;
- for (const Decl *D : AST.getTopLevelDecls()) {
+ for (const Decl *D : AST.getLocalTopLevelDecls()) {
auto *ND = dyn_cast(D);
if (!ND || ND->getNameAsString() != QName)
continue;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -168,7 +168,7 @@
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = true;
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+ indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
DeclMacrosFinder, IndexOpts);
return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()};
@@ -418,7 +418,7 @@
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = true;
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+ indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
DocHighlightsFinder, IndexOpts);
return DocHighlightsFinder.takeHighlights();
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -44,12 +44,10 @@
// Stores Preamble and associated data.
struct PreambleData {
- PreambleData(PrecompiledPreamble Preamble,
- std::vector TopLevelDeclIDs,
- std::vector Diags, std::vector Inclusions);
+ PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
+ std::vector Inclusions);
PrecompiledPreamble Preamble;
- std::vector TopLevelDeclIDs;
std::vector Diags;
// Processes like code completions and go-to-definitions will need #include
// information, and their compile action skips preamble range.
@@ -87,10 +85,9 @@
std::shared_ptr getPreprocessorPtr();
const Preprocessor &getPreprocessor() const;
- /// This function returns all top-level decls, including those that come
- /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
- /// this call might be expensive.
- ArrayRef getTopLevelDecls();
+ /// This function returns top-level decls, present in the main file of the
+ /// AST. The result does not include the decls that come from the preamble.
+ ArrayRef getLocalTopLevelDecls();
const std::vector &getDiagnostics() const;
@@ -106,9 +103,6 @@
std::vector TopLevelDecls, std::vector Diags,
std::vector Inclusions);
-private:
- void ensurePreambleDeclsDeserialized();
-
// In-memory preambles must outlive the AST, it is important that this member
// goes before Clang and Action.
std::shared_ptr Preamble;
@@ -123,7 +117,6 @@
// Data, stored after parsing.
std::vector Diags;
std::vector TopLevelDecls;
- bool PreambleDeclsDeserialized;
std::vector Inclusions;
};
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -90,37 +90,15 @@
CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
: File(File), ParsedCallback(ParsedCallback) {}
- std::vector takeTopLevelDeclIDs() {
-return std::move(TopLevelDeclIDs);
- }
-
std::vector takeInclusions() { return std::move(Inclusions); }
- void AfterPCHEmitted(ASTWriter &Writer) override {
-TopLevelDeclIDs.reserve(TopLevelDecls.size());
-for (Decl *D : TopLevelDecls) {
- // Invalid top-level decls may not have been serialized.
- if (D->isInvalidDecl())
-continue;
- TopLevelDeclIDs.push_back(Writer.getDeclID(D));
-}
- }
-
void AfterExecute(CompilerInstance &CI) override {
if (!ParsedCallback)
return;
trace::Span Tracer("Running PreambleCallback");
ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr());
}
- void HandleTopLevelDecl(DeclGroupRef DG)
r333202 - [bash-completion] Fix tab separation on macOS
Author: benlangmuir
Date: Thu May 24 09:25:40 2018
New Revision: 333202
URL: http://llvm.org/viewvc/llvm-project?rev=333202&view=rev
Log:
[bash-completion] Fix tab separation on macOS
We have a regex that needs to match a tab character in the command
output, but on macOS sed doesn't support '\t', causing it to split on
the 't' character instead. Fix by having bash expand the \t first.
Modified:
cfe/trunk/utils/bash-autocomplete.sh
Modified: cfe/trunk/utils/bash-autocomplete.sh
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/bash-autocomplete.sh?rev=333202&r1=333201&r2=333202&view=diff
==
--- cfe/trunk/utils/bash-autocomplete.sh (original)
+++ cfe/trunk/utils/bash-autocomplete.sh Thu May 24 09:25:40 2018
@@ -38,7 +38,8 @@ _clang()
# expand ~ to $HOME
eval local path=${COMP_WORDS[0]}
- flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e 's/\t.*//' )
+ # Use $'\t' so that bash expands the \t for older versions of sed.
+ flags=$( "$path" --autocomplete="$arg" 2>/dev/null | sed -e $'s/\t.*//' )
# If clang is old that it does not support --autocomplete,
# fall back to the filename completion.
if [[ "$?" != 0 ]]; then
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47273: [bash-completion] Fix tab separation on macOS
benlangmuir closed this revision. benlangmuir added a comment. r333202 https://reviews.llvm.org/D47273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst
gramanas added a comment. In https://reviews.llvm.org/D47097#149, @probinson wrote: > Can we step back a second and better explain what the problem is? With > current Clang the debug info for this function looks okay to me. > The store that is "missing" a debug location is homing the formal parameter > to its local stack location; this is part of prolog setup, not "real" code. Isn't this the reason the artificial debug loc exists? The store in the prolog might not be real code but it should still have the scope that the rest of the function has. Repository: rC Clang https://reviews.llvm.org/D47097 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin
ahatanak added inline comments. Comment at: test/SemaObjC/format-size-spec-nsinteger.m:18 + NSUInteger j = 0; + NSLog(@"max NSInteger = %zi", i); // CHECK: values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead + NSLog(@"max NSUinteger = %zu", j); // CHECK: values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead This test looks identical to test/SemaObjC/format-size-spec-nsinteger-nopedantic.m except for the CHECK lines. You can probably merge the two files if you separate the CHECK lines from the lines that call NSLog (see test/Sema/format-strings-darwin.c for example). Repository: rC Clang https://reviews.llvm.org/D47290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling
Meinersbur added a comment. Yes, this is what I had in mind. Thank you. I am preparing an RFC on what I am trying to do. This should clarify our goals and to what extend `#pragma clang loop` conflicts with it. https://reviews.llvm.org/D47267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst
aprantl added a comment. In https://reviews.llvm.org/D47097#223, @gramanas wrote: > In https://reviews.llvm.org/D47097#149, @probinson wrote: > > > Can we step back a second and better explain what the problem is? With > > current Clang the debug info for this function looks okay to me. > > The store that is "missing" a debug location is homing the formal > > parameter to its local stack location; this is part of prolog setup, not > > "real" code. > > > Isn't this the reason the artificial debug loc exists? The store in the > prolog might not be real code but it should still have the scope that the > rest of the function has. Instructions that are part of the function prologue are supposed to have no debug location, not an artificial one. The funciton prologue ends at the first instruction with a nonempty location. Repository: rC Clang https://reviews.llvm.org/D47097 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333211 - [X86] Fix a bad cast in _mm512_mask_abs_epi32 and _mm512_maskz_abs_epi32.
Author: ctopper
Date: Thu May 24 10:32:49 2018
New Revision: 333211
URL: http://llvm.org/viewvc/llvm-project?rev=333211&view=rev
Log:
[X86] Fix a bad cast in _mm512_mask_abs_epi32 and _mm512_maskz_abs_epi32.
Modified:
cfe/trunk/lib/Headers/avx512fintrin.h
Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=333211&r1=333210&r2=333211&view=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Thu May 24 10:32:49 2018
@@ -1948,7 +1948,7 @@ _mm512_abs_epi32(__m512i __A)
static __inline__ __m512i __DEFAULT_FN_ATTRS
_mm512_mask_abs_epi32 (__m512i __W, __mmask16 __U, __m512i __A)
{
- return (__m512i)__builtin_ia32_selectd_512((__mmask8)__U,
+ return (__m512i)__builtin_ia32_selectd_512(__U,
(__v16si)_mm512_abs_epi32(__A),
(__v16si)__W);
}
@@ -1956,7 +1956,7 @@ _mm512_mask_abs_epi32 (__m512i __W, __mm
static __inline__ __m512i __DEFAULT_FN_ATTRS
_mm512_maskz_abs_epi32 (__mmask16 __U, __m512i __A)
{
- return (__m512i)__builtin_ia32_selectd_512((__mmask8)__U,
+ return (__m512i)__builtin_ia32_selectd_512(__U,
(__v16si)_mm512_abs_epi32(__A),
(__v16si)_mm512_setzero_si512());
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
a.sidorin accepted this revision. a.sidorin added a comment. This revision is now accepted and ready to land. Ok, I got it, thank you! Repository: rC Clang https://reviews.llvm.org/D47058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion
probinson added a comment. I see that the per-file info does track the presence of `# [line] N` but it's not immediately obvious how to query "has any file seen one" which is really what I'd want to know. The file information has various levels of indirection... Repository: rC Clang https://reviews.llvm.org/D47260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148445.
leonardchan added a comment.
- Reverted changes involving name mangling since we will only support c++ for
now. Will concurrently raise an issue on
https://github.com/itanium-cxx-abi/cxx-abi/ to get characters for name mangling.
- Added a flag that needs to be provided to enable usage of fixed point types.
Not including this flag and using fixed point types throws an error. Currently,
this patch allows for these types to be used in all versions of C, but this can
be narrowed down to specific versions of C.
- An error is thrown when using fixed point types in C++.
- Fixed point types are ignored during USRGeneration since the type only gets
mangled in C++.
- Fixed point types their own width and alignment accessors/variables in
TargetInfo.
- Updated debug info to use `DW_ATE_signed_fixed` and `DW_ATE_unsigned_fixed`.
- Added tests mixing _Accum with other type specifiers
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-e
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan marked 6 inline comments as done.
leonardchan added inline comments.
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2684
// Types added here must also be added to EmitFundamentalRTTIDescriptors.
switch (Ty->getKind()) {
rsmith wrote:
> Note this comment :)
Returned false for these types now. Not sure if these types should also be
added to EmitFundamentalRTTIDescriptors since the OCL types do not appear under
there.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem
a.sidorin added a comment. Hm. Should we test `-fms-compatibility` in addition to `-fdelayed-template-parsing`? Repository: rC Clang https://reviews.llvm.org/D46950 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148452.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{fixed point types are only allowed in C}}
+// expected-error@-1{{C++ requires a type specifier for all declarations}}
+long _Accum long_accum; // expected-error{{fixed point types are only allowed in C}}
Index: test/Frontend/fixed_point_errors.c
===
---
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
jfb added a comment. Can you also add a test for `_Bool _Accum`. Also, `-enable-fixed-point -x c++` failing. Comment at: lib/AST/ExprConstant.cpp:7361 +case BuiltinType::ULongAccum: + // GCC does not cover FIXED_POINT_TYPE in it's switch stmt and defaults to + // no_type_class "its" Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; This seems weird because Targets which don't have these values for the non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`. Is there a point in ever having `_Accum` differ in size, width, and alignment from the underlying type? If not, can you set these values after the sub-target has specified its preferences? Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst
vsk added a comment.
In https://reviews.llvm.org/D47097#248, @aprantl wrote:
> In https://reviews.llvm.org/D47097#223, @gramanas wrote:
>
> > In https://reviews.llvm.org/D47097#149, @probinson wrote:
> >
> > > Can we step back a second and better explain what the problem is? With
> > > current Clang the debug info for this function looks okay to me.
> > > The store that is "missing" a debug location is homing the formal
> > > parameter to its local stack location; this is part of prolog setup, not
> > > "real" code.
> >
>
The problem @gramanas is trying to address appears after SROA. SROA invokes
mem2reg, which tries to assign scope information to the phis it creates (see
https://reviews.llvm.org/D45397). Subsequent passes which touch these phis can
use these debug locations. This makes it easier for the debugger to find the
right scope when handling a machine exception.
Store instructions in the prolog without scope information cause mem2reg to
create phis without scope info. E.g:
// foo.c
extern int map[];
void f(int a, int n) {
for (int i = 0; i < n; ++i)
a = map[a];
}
$ clang foo.c -S -emit-llvm -o - -g -O1 -mllvm -opt-bisect-limit=2
..
%a.addr.0 = phi i32 [ %a, %entry ], [ %0, %for.body ]
(Side note: @gramanas, it would help to have the full context/motivation for
changes in the patch summary.)
>> Isn't this the reason the artificial debug loc exists? The store in the
>> prolog might not be real code but it should still have the scope that the
>> rest of the function has.
>
> Instructions that are part of the function prologue are supposed to have no
> debug location, not an artificial one. The funciton prologue ends at the
> first instruction with a nonempty location.
I've been reading through PEI but I'm having a hard time verifying this. How
does llvm determine where the function prologue ends when there isn't any debug
info? What would go wrong if llvm started to behave as if the prologue ended
sooner than it should? Also, why isn't this a problem for the swift compiler,
which appears to always assign debug locations to each instruction?
Repository:
rC Clang
https://reviews.llvm.org/D47097
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47340: [ClangDiagnostics] Silence warning about fallthrough after PrintFatalError
xbolva00 created this revision.
Herald added subscribers: cfe-commits, jkorous, MaskRay, ioeric, ilya-biryukov.
ClangDiagnosticsEmitter.cpp:1047:57: warning: this statement may fall through
[-Wimplicit-fallthrough=]
Builder.PrintFatalError("Unknown modifier type: " + Modifier);
~~^~
ClangDiagnosticsEmitter.cpp:1048:5: note: here
case MT_Select: {
^
Repository:
rC Clang
https://reviews.llvm.org/D47340
Files:
utils/TableGen/ClangDiagnosticsEmitter.cpp
Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1045,6 +1045,7 @@
switch (ModType) {
case MT_Unknown:
Builder.PrintFatalError("Unknown modifier type: " + Modifier);
+ break;
case MT_Select: {
SelectPiece *Select = New(MT_Select);
do {
Index: utils/TableGen/ClangDiagnosticsEmitter.cpp
===
--- utils/TableGen/ClangDiagnosticsEmitter.cpp
+++ utils/TableGen/ClangDiagnosticsEmitter.cpp
@@ -1045,6 +1045,7 @@
switch (ModType) {
case MT_Unknown:
Builder.PrintFatalError("Unknown modifier type: " + Modifier);
+ break;
case MT_Select: {
SelectPiece *Select = New(MT_Select);
do {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47341: [Sema] Disable creating new delayed typos while correcting existing.
vsapsai created this revision.
vsapsai added reviewers: arphaman, majnemer.
NumTypos guard value ~0U doesn't prevent from creating new delayed
typos. When you create new delayed typos during typo correction, value
~0U wraps around to 0. This state is inconsistent and depending on total
number of typos you can hit the assertion
> Assertion failed: (DelayedTypos.empty() && "Uncorrected typos!"), function
> ~Sema, file clang/lib/Sema/Sema.cpp, line 366.
or have infinite typo correction loop.
Fix by disabling typo correction during performing typo correcting
transform. It disables the feature of having typo corrections on top of
other typo corrections because that feature is unreliable.
rdar://problem/38642201
https://reviews.llvm.org/D47341
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaExprCXX.cpp
clang/test/Sema/typo-correction.c
clang/test/SemaCXX/typo-correction-delayed.cpp
Index: clang/test/SemaCXX/typo-correction-delayed.cpp
===
--- clang/test/SemaCXX/typo-correction-delayed.cpp
+++ clang/test/SemaCXX/typo-correction-delayed.cpp
@@ -137,13 +137,12 @@
namespace PR21925 {
struct X {
- int get() { return 7; } // expected-note {{'get' declared here}}
+ int get() { return 7; }
};
void test() {
- X variable; // expected-note {{'variable' declared here}}
+ X variable;
- // expected-error@+2 {{use of undeclared identifier 'variableX'; did you mean 'variable'?}}
- // expected-error@+1 {{no member named 'getX' in 'PR21925::X'; did you mean 'get'?}}
+ // expected-error@+1 {{use of undeclared identifier 'variableX'}}
int x = variableX.getX();
}
}
Index: clang/test/Sema/typo-correction.c
===
--- clang/test/Sema/typo-correction.c
+++ clang/test/Sema/typo-correction.c
@@ -87,3 +87,16 @@
void overloadable_callexpr(int arg) {
func_overloadable(ar); //expected-error{{use of undeclared identifier}}
}
+
+// rdar://problem/38642201
+struct rdar38642201 {
+ int fieldName;
+};
+
+void rdar38642201_callee(int x, int y);
+void rdar38642201_caller() {
+ struct rdar38642201 structVar;
+ rdar38642201_callee(
+ structVarTypo.fieldNameTypo, //expected-error{{use of undeclared identifier 'structVarTypo'}}
+ structVarTypo2.fieldNameTypo2); //expected-error{{use of undeclared identifier 'structVarTypo2'}}
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -7588,7 +7588,7 @@
// handling potentially ambiguous typo corrections as any new TypoExprs will
// have been introduced by the application of one of the correction
// candidates and add little to no value if corrected.
-SemaRef.DisableTypoCorrection = true;
+Sema::DisableTypoCorrectionScope Scope(SemaRef);
while (!AmbiguousTypoExprs.empty()) {
auto TE = AmbiguousTypoExprs.back();
auto Cached = TransformCache[TE];
@@ -7605,7 +7605,6 @@
State.Consumer->restoreSavedPosition();
TransformCache[TE] = Cached;
}
-SemaRef.DisableTypoCorrection = false;
// Ensure that all of the TypoExprs within the current Expr have been found.
if (!Res.isUsable())
@@ -7668,11 +7667,14 @@
if (E && !ExprEvalContexts.empty() && ExprEvalContexts.back().NumTypos &&
(E->isTypeDependent() || E->isValueDependent() ||
E->isInstantiationDependent())) {
+DisableTypoCorrectionScope DisableRecurrentCorrectionScope(*this);
auto TyposInContext = ExprEvalContexts.back().NumTypos;
assert(TyposInContext < ~0U && "Recursive call of CorrectDelayedTyposInExpr");
ExprEvalContexts.back().NumTypos = ~0U;
auto TyposResolved = DelayedTypos.size();
auto Result = TransformTypos(*this, InitDecl, Filter).Transform(E);
+assert(ExprEvalContexts.back().NumTypos == ~0U &&
+ "Unexpected NumTypos modification");
ExprEvalContexts.back().NumTypos = TyposInContext;
TyposResolved -= DelayedTypos.size();
if (Result.isInvalid() || Result.get() != E) {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -7561,6 +7561,22 @@
}
};
+ /// RAII class used to disable typo correction temporarily.
+ class DisableTypoCorrectionScope {
+Sema &SemaRef;
+bool PrevDisableTypoCorrection;
+
+ public:
+explicit DisableTypoCorrectionScope(Sema &SemaRef)
+: SemaRef(SemaRef),
+ PrevDisableTypoCorrection(SemaRef.DisableTypoCorrection) {
+ SemaRef.DisableTypoCorrection = true;
+}
+~DisableTypoCorrectionScope() {
+ SemaRef.DisableTypoCorrection = PrevDisableTypoCorrection;
+}
+ };
+
/// The current instantiation scope used to store local
/// variables.
LocalInstantiationScope *CurrentInstantiationScope
[PATCH] D46052: GNUstep Objective-C ABI version 2
xbolva00 added inline comments.
Comment at: lib/CodeGen/CGObjCGNU.cpp:512
/// used to return an untyped selector (with the types field set to NULL).
- llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
+ virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
const std::string &TypeEncoding);
This causes
CGObjCGNU.cpp:589:16: warning: ‘virtual llvm::Value*
{anonymous}::CGObjCGNU::GetSelector(clang::CodeGen::CodeGenFunction&, const
clang::ObjCMethodDecl*)’ was hidden [-Woverloaded-virtual]
llvm::Value *GetSelector(CodeGenFunction &CGF,
Repository:
rC Clang
https://reviews.llvm.org/D46052
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47109: LWG 2969 "polymorphic_allocator::construct() shouldn't pass resource()"
Quuxplusone added a comment. > Sounds good to me. I don't have commit privs so could you land it for me? @EricWF ping! Repository: rCXX libc++ https://reviews.llvm.org/D47109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47229: Make atomic non-member functions as nonnull
jfb added a comment. Addressed nit. Repository: rC Clang https://reviews.llvm.org/D47229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47229: Make atomic non-member functions as nonnull
jfb updated this revision to Diff 148458.
jfb marked an inline comment as done.
jfb added a comment.
- Address nit.
Repository:
rC Clang
https://reviews.llvm.org/D47229
Files:
lib/Sema/SemaChecking.cpp
test/Sema/atomic-ops.c
Index: test/Sema/atomic-ops.c
===
--- test/Sema/atomic-ops.c
+++ test/Sema/atomic-ops.c
@@ -531,8 +531,80 @@
}
void nullPointerWarning(_Atomic(int) *Ap, int *p, int val) {
- // The 'expected' pointer shouldn't be NULL.
- (void)__c11_atomic_compare_exchange_strong(Ap, NULL, val, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
- (void)atomic_compare_exchange_weak(Ap, ((void*)0), val); // expected-warning {{null passed to a callee that requires a non-null argument}}
- (void)__atomic_compare_exchange_n(p, NULL, val, 0, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ volatile _Atomic(int) vai;
+ _Atomic(int) ai;
+ volatile int vi = 42;
+ int i = 42;
+
+ __c11_atomic_init((volatile _Atomic(int)*)0, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ __c11_atomic_init((_Atomic(int)*)0, 42); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ __c11_atomic_store((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ __c11_atomic_store((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_load((volatile _Atomic(int)*)0, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_load((_Atomic(int)*)0, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_exchange((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_exchange((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_compare_exchange_weak((volatile _Atomic(int)*)0, &i, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_compare_exchange_weak((_Atomic(int)*)0, &i, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_compare_exchange_weak(&vai, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_compare_exchange_weak(&ai, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_compare_exchange_strong((volatile _Atomic(int)*)0, &i, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_compare_exchange_strong((_Atomic(int)*)0, &i, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_compare_exchange_strong(&vai, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_compare_exchange_strong(&ai, (int*)0, 42, memory_order_relaxed, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_fetch_add((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_fetch_add((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_fetch_sub((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_fetch_sub((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_fetch_and((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_fetch_and((_Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null passed to a callee that requires a non-null argument}}
+ (void)__c11_atomic_fetch_or((volatile _Atomic(int)*)0, 42, memory_order_relaxed); // expected-warning {{null pass
[PATCH] D47067: Update NRVO logic to support early return
rsmith added a comment.
Some minor nits. I would also like to see a test for the unoptimized (-O0) IR
generated by Clang for the case where there are two NRVO variables in the same
function. Otherwise, this looks good to go!
Comment at: lib/Sema/Scope.cpp:122-123
+void Scope::setNRVOCandidate(VarDecl *Candidate) {
+ for (auto* D : DeclsInScope) {
+VarDecl* VD = dyn_cast_or_null(D);
+if (VD && VD != Candidate && VD->isNRVOCandidate())
`*` on the right, please. Also `auto` -> `Decl` would be clearer and no longer.
Is `dyn_cast_or_null` really necessary? (Can `DeclsInScope` contain `nullptr`?)
I would expect that just `dyn_cast` would suffice.
Comment at: lib/Sema/SemaDecl.cpp:12619-12620
void Sema::computeNRVO(Stmt *Body, FunctionScopeInfo *Scope) {
- ReturnStmt **Returns = Scope->Returns.data();
+ for (ReturnStmt* Return : Scope->Returns) {
+const VarDecl* Candidate = Return->getNRVOCandidate();
+if (!Candidate)
`*` on the right, please.
Repository:
rC Clang
https://reviews.llvm.org/D47067
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.
rkirsling added a comment. @klimek In our IRC discussion yesterday, I know you expressed disapproval of WebKit's choice, but given its reality, am I correct in concluding that this can be landed? Repository: rC Clang https://reviews.llvm.org/D46024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47301: Warning for framework include violation from Headers to PrivateHeaders
vsapsai added inline comments.
Comment at: include/clang/Basic/DiagnosticGroups.td:34-35
def AutoImport : DiagGroup<"auto-import">;
def FrameworkHdrQuotedInclude :
DiagGroup<"quoted-include-in-framework-header">;
+def FrameworkIncludePrivateFromPublic :
DiagGroup<"framework-include-private-from-public">;
def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">;
It might be convenient for users to have a warning group that will cover
different framework warnings, something like -Wframework-hygiene. But it's out
of scope for this review, more as an idea for future improvements.
Comment at: lib/Lex/HeaderSearch.cpp:625
+static bool isFrameworkStylePath(StringRef Path, bool &IsPrivateHeader,
+ SmallVectorImpl &FrameworkName) {
using namespace llvm::sys;
In this function we accept some paths that aren't valid framework paths. Need
to think more if it is OK or if we want to be stricter.
Comment at: lib/Lex/HeaderSearch.cpp:893
+ // from Foo.framework/PrivateHeaders, since this violates public/private
+ // api boundaries and can cause modular dependency cycles.
+ SmallString<128> ToFramework;
s/api/API/
Comment at: lib/Lex/HeaderSearch.cpp:895
+ SmallString<128> ToFramework;
+ bool IncludeIsFrameworkPrivateHeader = false;
+ if (IsIncluderFromFramework && !IsFrameworkPrivateHeader &&
My opinion on readability is personal, so take it with a grain of salt. What if
you make variable names more consistent, like
* IsIncluderPrivateHeader
* IsIncluderFromFramework
* IsIncludeePrivateHeader
Comment at: test/Modules/framework-public-includes-private.m:33
+
+int bar() { return foo(); }
+
I'm not entirely sure it's not covered by existing test. It might be worth
testing private header including public header and private header including
another private header.
https://reviews.llvm.org/D47301
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47142: [x86] invpcid intrinsic
craig.topper added a comment. LGTM, if you fix the ordering in cpuid.h. https://reviews.llvm.org/D47142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333220 - Improve diagnostics for config mismatches with -fmodule-file.
Author: rsmith
Date: Thu May 24 13:03:51 2018
New Revision: 333220
URL: http://llvm.org/viewvc/llvm-project?rev=333220&view=rev
Log:
Improve diagnostics for config mismatches with -fmodule-file.
Unless the user uses -Wno-module-file-config-mismatch (or -Wno-error=...),
allow the AST reader to produce errors describing the nature of the config
mismatch.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp
cfe/trunk/test/Modules/merge-target-features.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=333220&r1=333219&r2=333220&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Thu May 24
13:03:51 2018
@@ -38,8 +38,8 @@ def err_pch_targetopt_mismatch : Error<
"PCH file was compiled for the %0 '%1' but the current translation "
"unit is being compiled for target '%2'">;
def err_pch_targetopt_feature_mismatch : Error<
-"%select{AST file|current translation unit}0 was compiled with the target "
-"feature'%1' but the %select{current translation unit is|AST file was}0 "
+"%select{AST file was|current translation unit is}0 compiled with the
target "
+"feature '%1' but the %select{current translation unit is|AST file was}0 "
"not">;
def err_pch_langopt_mismatch : Error<"%0 was %select{disabled|enabled}1 in "
"PCH file but is currently %select{disabled|enabled}2">;
Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=333220&r1=333219&r2=333220&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Thu May 24 13:03:51 2018
@@ -1602,15 +1602,22 @@ bool CompilerInstance::loadModuleFile(St
if (!ModuleManager)
createModuleManager();
+ // If -Wmodule-file-config-mismatch is mapped as an error or worse, allow the
+ // ASTReader to diagnose it, since it can produce better errors that we can.
+ bool ConfigMismatchIsRecoverable =
+ getDiagnostics().getDiagnosticLevel(diag::warn_module_config_mismatch,
+ SourceLocation())
+<= DiagnosticsEngine::Warning;
+
auto Listener = llvm::make_unique(*this);
auto &ListenerRef = *Listener;
ASTReader::ListenerScope ReadModuleNamesListener(*ModuleManager,
std::move(Listener));
// Try to load the module file.
- switch (ModuleManager->ReadAST(FileName, serialization::MK_ExplicitModule,
- SourceLocation(),
- ASTReader::ARR_ConfigurationMismatch)) {
+ switch (ModuleManager->ReadAST(
+ FileName, serialization::MK_ExplicitModule, SourceLocation(),
+ ConfigMismatchIsRecoverable ? ASTReader::ARR_ConfigurationMismatch : 0))
{
case ASTReader::Success:
// We successfully loaded the module file; remember the set of provided
// modules so that we don't try to load implicit modules for them.
Modified: cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp?rev=333220&r1=333219&r2=333220&view=diff
==
--- cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp (original)
+++ cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp Thu May 24 13:03:51
2018
@@ -43,7 +43,7 @@
//
// Import the PCH without ASan enabled (we expect an error).
// RUN: not %clang_cc1 -x c -include-pch %t.asan_pch %s -verify 2>&1 |
FileCheck %s --check-prefix=PCH_MISMATCH
-// PCH_MISMATCH: AST file was compiled with the target
feature'-fsanitize=address' but the current translation unit is not
+// PCH_MISMATCH: AST file was compiled with the target feature
'-fsanitize=address' but the current translation unit is not
//
// Emit a PCH with UBSan enabled.
// RUN: %clang_cc1 -x c -fsanitize=null
%S/Inputs/check-for-sanitizer-feature/check.h -emit-pch -o %t.ubsan_pch
Modified: cfe/trunk/test/Modules/merge-target-features.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-target-features.cpp?rev=333220&r1=333219&r2=333220&view=diff
==
--- cfe/trunk/test/Modules/merge-target-features.cpp (original)
+++ cfe/trunk/test/Modules/merge-target-features.cpp Thu May 24 13:03:51 2018
@@ -19,10 +19,9 @@
// RUN: -triple i386-unknown-unknown \
[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin
jfb updated this revision to Diff 148467.
jfb marked an inline comment as done.
jfb added a comment.
- Merge format-size-spec-nsinteger
Repository:
rC Clang
https://reviews.llvm.org/D47290
Files:
include/clang/Analysis/Analyses/FormatString.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Analysis/PrintfFormatString.cpp
lib/Sema/SemaChecking.cpp
test/FixIt/fixit-format-ios-nopedantic.m
test/FixIt/fixit-format-ios.m
test/SemaObjC/format-size-spec-nsinteger.m
Index: test/SemaObjC/format-size-spec-nsinteger.m
===
--- /dev/null
+++ test/SemaObjC/format-size-spec-nsinteger.m
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios -Wno-objc-root-class -fsyntax-only -verify -Wformat-pedantic -DPEDANTIC %s
+
+#if !defined(PEDANTIC)
+// expected-no-diagnostics
+#endif
+
+#if __LP64__
+typedef unsigned long NSUInteger;
+typedef long NSInteger;
+#else
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+#endif
+
+@class NSString;
+
+extern void NSLog(NSString *format, ...);
+
+void testSizeSpecifier() {
+ NSInteger i = 0;
+ NSUInteger j = 0;
+ NSLog(@"max NSInteger = %zi", i);
+ NSLog(@"max NSUinteger = %zu", j);
+
+#if defined(PEDANTIC)
+ // expected-warning@-4 {{values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead}}
+ // expected-warning@-4 {{values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead}}
+#endif
+}
Index: test/FixIt/fixit-format-ios.m
===
--- test/FixIt/fixit-format-ios.m
+++ test/FixIt/fixit-format-ios.m
@@ -1,5 +1,5 @@
// RUN: cp %s %t
-// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -fixit %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat-pedantic -fixit %t
// RUN: grep -v CHECK %t | FileCheck %s
int printf(const char * restrict, ...);
Index: test/FixIt/fixit-format-ios-nopedantic.m
===
--- /dev/null
+++ test/FixIt/fixit-format-ios-nopedantic.m
@@ -0,0 +1,21 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -fsyntax-only -Wformat -Werror -fixit %t
+
+int printf(const char *restrict, ...);
+typedef unsigned int NSUInteger;
+typedef int NSInteger;
+NSUInteger getNSUInteger();
+NSInteger getNSInteger();
+
+void test() {
+ // For thumbv7-apple-ios8.0.0 the underlying type of ssize_t is long
+ // and the underlying type of size_t is unsigned long.
+
+ printf("test 1: %zu", getNSUInteger());
+
+ printf("test 2: %zu %zu", getNSUInteger(), getNSUInteger());
+
+ printf("test 3: %zd", getNSInteger());
+
+ printf("test 4: %zd %zd", getNSInteger(), getNSInteger());
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6594,11 +6594,11 @@
ExprTy = TET->getUnderlyingExpr()->getType();
}
- analyze_printf::ArgType::MatchKind match = AT.matchesType(S.Context, ExprTy);
-
- if (match == analyze_printf::ArgType::Match) {
+ const analyze_printf::ArgType::MatchKind match =
+ AT.matchesType(S.Context, ExprTy);
+ bool pedantic = match == analyze_printf::ArgType::NoMatchPedantic;
+ if (match == analyze_printf::ArgType::Match)
return true;
- }
// Look through argument promotions for our error message's reported type.
// This includes the integral and floating promotions, but excludes array
@@ -6674,6 +6674,11 @@
QualType CastTy;
std::tie(CastTy, CastTyName) = shouldNotPrintDirectly(S.Context, IntendedTy, E);
if (!CastTy.isNull()) {
+ // %zi/%zu are OK to use for NSInteger/NSUInteger of type int
+ // (long in ASTContext). Only complain to pedants.
+ if ((CastTyName == "NSInteger" || CastTyName == "NSUInteger") &&
+ AT.isSizeT() && AT.matchesType(S.Context, CastTy))
+pedantic = true;
IntendedTy = CastTy;
ShouldNotPrintDirectly = true;
}
@@ -6693,10 +6698,10 @@
CharSourceRange SpecRange = getSpecifierRange(StartSpecifier, SpecifierLen);
if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
- unsigned diag = diag::warn_format_conversion_argument_type_mismatch;
- if (match == analyze_format_string::ArgType::NoMatchPedantic) {
-diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
- }
+ unsigned diag =
+ pedantic
+ ? diag::warn_format_conversion_argument_type_mismatch_pedantic
+ : diag::warn_format_conversion_argument_type_mismatch;
// In this case, the specifier is wrong and should be changed to match
// the argument.
EmitFormatDiagnostic(S.PDiag(diag)
@@ -6752,9 +6757,11 @@
Nam
[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin
jfb marked an inline comment as done. jfb added inline comments. Comment at: test/SemaObjC/format-size-spec-nsinteger.m:18 + NSUInteger j = 0; + NSLog(@"max NSInteger = %zi", i); // CHECK: values of type 'NSInteger' should not be used as format arguments; add an explicit cast to 'long' instead + NSLog(@"max NSUinteger = %zu", j); // CHECK: values of type 'NSUInteger' should not be used as format arguments; add an explicit cast to 'unsigned long' instead ahatanak wrote: > This test looks identical to > test/SemaObjC/format-size-spec-nsinteger-nopedantic.m except for the CHECK > lines. You can probably merge the two files if you separate the CHECK lines > from the lines that call NSLog (see test/Sema/format-strings-darwin.c for > example). Cute, I didn't know about this pattern. Repository: rC Clang https://reviews.llvm.org/D47290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"
Quuxplusone created this revision.
Quuxplusone added a reviewer: EricWF.
Herald added subscribers: cfe-commits, JDevlieghere.
https://github.com/cplusplus/draft/commit/6216651aada9bc2f9cefe90edbde4ea9e32251ab
`new_delete_resource().allocate(n, a)` has basically three permissible results:
- Return an appropriately sized and aligned block.
- Throw bad_alloc.
- If `n == 0`, return an unspecified result.
Before this patch, libc++'s `new_delete_resource` would do a fourth and
impermissible thing, which was to return an appropriately sized but
inappropriately under-aligned block. This is now fixed.
(This came up while I was stress-testing `unsynchronized_pool_resource` on my
MacBook. If we can't trust the default resource to return appropriately aligned
blocks, pretty much everything breaks. For similar reasons, I would strongly
support just patching `__libcpp_allocate` directly, but I don't care to die on
that hill, so I made this patch as a ``-specific workaround.)
Repository:
rCXX libc++
https://reviews.llvm.org/D47344
Files:
src/experimental/memory_resource.cpp
Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "experimental/memory_resource"
+#include "memory"
#ifndef _LIBCPP_HAS_NO_ATOMIC_HEADER
#include "atomic"
@@ -23,18 +24,36 @@
// new_delete_resource()
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
: public memory_resource
{
public:
~__new_delete_memory_resource_imp() = default;
protected:
virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+{
+void *result = _VSTD::__libcpp_allocate(__size, __align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+{ _VSTD::__libcpp_deallocate(__p, __align); }
virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
{ return &__other == this; }
Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "experimental/memory_resource"
+#include "memory"
#ifndef _LIBCPP_HAS_NO_ATOMIC_HEADER
#include "atomic"
@@ -23,18 +24,36 @@
// new_delete_resource()
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+static bool is_aligned_to(void *ptr, size_t align)
+{
+void *p2 = ptr;
+size_t space = 1;
+void *result = _VSTD::align(align, 1, p2, space);
+return (result == ptr);
+}
+#endif
+
class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
: public memory_resource
{
public:
~__new_delete_memory_resource_imp() = default;
protected:
virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+{
+void *result = _VSTD::__libcpp_allocate(__size, __align);
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__size != 0 && !is_aligned_to(result, __align)) {
+__throw_bad_alloc();
+}
+#endif
+return result;
+}
virtual void do_deallocate(void * __p, size_t, size_t __align)
-{ _VSTD::__libcpp_deallocate(__p, __align); /* FIXME */ }
+{ _VSTD::__libcpp_deallocate(__p, __align); }
virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
{ return &__other == this; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148481.
leonardchan marked 2 inline comments as done.
leonardchan added a comment.
- Added test case for `_Bool _Accum`
- Getters for the `_Accum` bit widths return values for their corresponding
integral types (ie. `sizeof(short _Accum) == sizeof(short)`). The only case
where this may not happen is if the target architecture uses 16 bits for an
int. N1169 requires that a `signed/unsigned _Accum` hold at least 15 fractional
bits and 4 integral bits. To be able to fit these bits, the size is upgraded to
that of a long which is guaranteed to be large enough to hold them.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_bit_widths.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expecte
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added a comment. In https://reviews.llvm.org/D46084#374, @jfb wrote: > Can you also add a test for `_Bool _Accum`. > > Also, `-enable-fixed-point -x c++` failing. . Done. Also the failing c++ case is under `test/Frontend/fixed_point_errors.cpp` Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333233 - Switch a couple of users of LangOpts::GNUMode to the more appropriate LangOpts::GNUKeywords.
Author: rsmith
Date: Thu May 24 14:51:52 2018
New Revision: 333233
URL: http://llvm.org/viewvc/llvm-project?rev=333233&view=rev
Log:
Switch a couple of users of LangOpts::GNUMode to the more appropriate
LangOpts::GNUKeywords.
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=333233&r1=333232&r2=333233&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu May 24 14:51:52 2018
@@ -1371,8 +1371,8 @@ static void AddTypeSpecifierResults(cons
} else
Results.AddResult(Result("__auto_type", CCP_Type));
- // GNU extensions
- if (LangOpts.GNUMode) {
+ // GNU keywords
+ if (LangOpts.GNUKeywords) {
// FIXME: Enable when we actually support decimal floating point.
//Results.AddResult(Result("_Decimal32"));
//Results.AddResult(Result("_Decimal64"));
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=333233&r1=333232&r2=333233&view=diff
==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu May 24 14:51:52 2018
@@ -4459,7 +4459,7 @@ static void AddKeywordsToConsumer(Sema &
}
}
-if (SemaRef.getLangOpts().GNUMode)
+if (SemaRef.getLangOpts().GNUKeywords)
Consumer.addKeywordResult("typeof");
} else if (CCC.WantFunctionLikeCasts) {
static const char *const CastableTypeSpecs[] = {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added inline comments. Comment at: include/clang/Basic/TargetInfo.h:382 +// enough bits to fit the minumum. +if (getIntWidth() < MinSignedAccumDataBits) + return getLongWidth(); I'm not sure I agree with this interpretation. It's simply not correct to consider 'short' the 'underlying type' of 'short _Accum', 'int' the 'underlying type' of '_Accum', etc. They are wholly independent and should have separate settings altogether. Asserting/ensuring that a target has set an invalid width for its types should be done separately. This currently feels a bit like the TargetInfo is guessing. (For the record, the reason I'm requesting this change is because this implementation does not work for us downstream.) Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333234 - Improve diagonstic for braced-init-list as operand to ?: expression.
Author: rsmith
Date: Thu May 24 15:02:52 2018
New Revision: 333234
URL: http://llvm.org/viewvc/llvm-project?rev=333234&view=rev
Log:
Improve diagonstic for braced-init-list as operand to ?: expression.
Modified:
cfe/trunk/lib/Parse/ParseExpr.cpp
cfe/trunk/test/Parser/cxx11-brace-initializers.cpp
Modified: cfe/trunk/lib/Parse/ParseExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExpr.cpp?rev=333234&r1=333233&r2=333234&view=diff
==
--- cfe/trunk/lib/Parse/ParseExpr.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExpr.cpp Thu May 24 15:02:52 2018
@@ -336,7 +336,17 @@ Parser::ParseRHSOfBinaryExpression(ExprR
// Special case handling for the ternary operator.
ExprResult TernaryMiddle(true);
if (NextTokPrec == prec::Conditional) {
- if (Tok.isNot(tok::colon)) {
+ if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) {
+// Parse a braced-init-list here for error recovery purposes.
+SourceLocation BraceLoc = Tok.getLocation();
+TernaryMiddle = ParseBraceInitializer();
+if (!TernaryMiddle.isInvalid()) {
+ Diag(BraceLoc, diag::err_init_list_bin_op)
+ << /*RHS*/ 1 << PP.getSpelling(OpToken)
+ << Actions.getExprRange(TernaryMiddle.get());
+ TernaryMiddle = ExprError();
+}
+ } else if (Tok.isNot(tok::colon)) {
// Don't parse FOO:BAR as if it were a typo for FOO::BAR.
ColonProtectionRAIIObject X(*this);
@@ -345,11 +355,6 @@ Parser::ParseRHSOfBinaryExpression(ExprR
// In particular, the RHS of the '?' is 'expression', not
// 'logical-OR-expression' as we might expect.
TernaryMiddle = ParseExpression();
-if (TernaryMiddle.isInvalid()) {
- Actions.CorrectDelayedTyposInExpr(LHS);
- LHS = ExprError();
- TernaryMiddle = nullptr;
-}
} else {
// Special case handling of "X ? Y : Z" where Y is empty:
// logical-OR-expression '?' ':' conditional-expression [GNU]
@@ -357,6 +362,12 @@ Parser::ParseRHSOfBinaryExpression(ExprR
Diag(Tok, diag::ext_gnu_conditional_expr);
}
+ if (TernaryMiddle.isInvalid()) {
+Actions.CorrectDelayedTyposInExpr(LHS);
+LHS = ExprError();
+TernaryMiddle = nullptr;
+ }
+
if (!TryConsumeToken(tok::colon, ColonLoc)) {
// Otherwise, we're missing a ':'. Assume that this was a typo that
// the user forgot. If we're not in a macro expansion, we can suggest
@@ -469,6 +480,11 @@ Parser::ParseRHSOfBinaryExpression(ExprR
if (ThisPrec == prec::Assignment) {
Diag(OpToken, diag::warn_cxx98_compat_generalized_initializer_lists)
<< Actions.getExprRange(RHS.get());
+ } else if (ColonLoc.isValid()) {
+Diag(ColonLoc, diag::err_init_list_bin_op)
+ << /*RHS*/1 << ":"
+ << Actions.getExprRange(RHS.get());
+LHS = ExprError();
} else {
Diag(OpToken, diag::err_init_list_bin_op)
<< /*RHS*/1 << PP.getSpelling(OpToken)
Modified: cfe/trunk/test/Parser/cxx11-brace-initializers.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/cxx11-brace-initializers.cpp?rev=333234&r1=333233&r2=333234&view=diff
==
--- cfe/trunk/test/Parser/cxx11-brace-initializers.cpp (original)
+++ cfe/trunk/test/Parser/cxx11-brace-initializers.cpp Thu May 24 15:02:52 2018
@@ -1,5 +1,4 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11
-// expected-no-diagnostics
struct S {
S(int, int) {}
@@ -25,3 +24,6 @@ namespace PR14948 {
template T Q::x {};
}
+
+int conditional1 = 1 ? {} : 0; // expected-error {{initializer list cannot be
used on the right hand side of operator '?'}}
+int conditional2 = 1 ? 0 : {}; // expected-error {{initializer list cannot be
used on the right hand side of operator ':'}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47229: Make atomic non-member functions as nonnull
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Either the previous version of this patch or a version with a `bool` factored
out seem OK to me.
Comment at: lib/Sema/SemaChecking.cpp:3497
+else if (Form == Copy || Form == Xchg) {
+ if (!IsC11 && !IsN)
+// The value pointer is always dereferenced, a nullptr is
undefined.
arphaman wrote:
> Nit: might make more sense to check if `ByValType` is a pointer here instead
> of duplicating the `if` condition from above.
I think the suggestion was to check `ByValType->isPointerType()` here. But...
that doesn't work, because we could have a pointer value being passed by value
(where a null is allowed), rather than a value being passed by address (where a
null is disallowed). I think this comparison is actually less clear than the
`!IsC11 && !IsN` check; factoring out a `bool IsPassedByAddress` or similar
instead would aid readability here.
Repository:
rC Clang
https://reviews.llvm.org/D47229
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44568: Fix emission of phony dependency targets when adding extra deps
vsapsai added inline comments.
Comment at: lib/Frontend/DependencyFile.cpp:182-185
for (const auto &ExtraDep : Opts.ExtraDeps) {
AddFilename(ExtraDep);
+ ++InputFileIndex;
}
This is incorrect if there are duplicates in `Opts.ExtraDeps`. Also please
update the test to have duplicate extra dependencies.
Comment at: test/Frontend/dependency-gen-extradeps-phony.c:6-7
+
+// CHECK: dependency-gen-extradeps-phony.o: 1.extra 2.extra
+// CHECK-NOT: .c:
+// CHECK: 1.extra:
I think it would be better to have a check
// CHECK: dependency-gen-extradeps-phony.c
Because you expect it to be there and it's not that simple to notice the colon
in `.c:`, so it's not immediately clear how CHECK-NOT is applied here.
Comment at: test/Frontend/dependency-gen-extradeps-phony.c:9-11
+// CHECK-NOT: .c:
+// CHECK: 2.extra:
+// CHECK-NOT: .c:
For these repeated CHECK-NOT please consider using `FileCheck
--implicit-check-not`. In this case it's not that important as the test is
small but it can still help to capture your intention more clearly.
Repository:
rC Clang
https://reviews.llvm.org/D44568
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
