[PATCH] D53135: Remove top-level using declaration from header files, as these aliases leak.
rnk added inline comments. Comment at: cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h:147 static std::pair - readIndex(StringRef Path); + readIndex(llvm::StringRef Path); It's preferred to include clang/Basic/LLVM.h instead and use the StringRef in the clang namespace: https://github.com/llvm-mirror/clang/blob/master/include/clang/Basic/LLVM.h Otherwise we'd have llvm::StringRef literally everywhere. Repository: rL LLVM https://reviews.llvm.org/D53135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
djasper added a comment.
In https://reviews.llvm.org/D52676#1268748, @oleg.smolsky wrote:
> In https://reviews.llvm.org/D52676#1268706, @djasper wrote:
>
> > Ok, I think I agree with both of you to a certain extent, but I also think
> > this change as is, is not yet right.
> >
> > First, it does too much. The original very first example in this CL is
> > actually not produced by clang-format (or if it is, I don't know with which
> > flag combination). It is a case where the lambda is the last parameter.
>
>
> Right, I cheated and created that example by hand. My apologies for the
> confusion. I've just pasted real code that I pumped through `clang-format`.
> Please take a look at the updated summary.
>
> > Second, I agree that the original behavior is inconsistent in a way that we
> > have a special cases for when the lambda is the very first parameter, but
> > somehow seem be forgetting about that when it's not the first parameter.
> > I'd be ok with "fixing" that (it's not a clear-cut bug, but I think the
> > alternative behavior would be cleaner). However, I don't think your patch
> > is doing enough there. I think this should be irrespective of bin-packing
> > (it's related but not quite the same thing),
>
> Also there is a special case for multiple lambdas. It forces line breaks.
> That aside, for the single-lambda case, are you suggesting that it is always
> "pulled up", irrespective of its place? That would contradict the direction I
> am trying to take as I like `BinPackArguments: false` and so long lamba args
> go to their own lines. This looks very much in line with what bin packing is,
> but not exactly the same. Obviously, I can add a new option `favor line
> breaks around multi-line lambda`.
I don't think I am. You are right, there is the special case for multi-lambda
functions and I think we should have almost the same for single-lambda
functions. So, I think I agree with you and am in favor of:
someFunction(
a,
[] {
// ...
},
b);
And this is irrespective of BinPacking. I think this is always better and more
consistent with what we'd be doing if "a" was not there. The only caveat is
that I think with BinPacking true or false, we should keep the more compact
formatting if "b" isn't there and the lambda introducer fits entirely on the
first line:
someFunction(a, [] {
// ...
});
> Could you look at the updated summary (high level) and the new tests I added
> (low level) please? Every other test passes, so we have almost the entire
> spec. I can add a few cases where an existing snippet is reformatted with
> `BinPackArguments: false` too.
Not sure I am seeing new test cases and I think at least a few cases are
missing from the entire spec, e.g. the case above.
Also, try to reduce the test cases a bit more:
- They don't need the surrounding functions
- You can force the lambda to be multi-line with a "//" comment
- There is no reason to have the lambda be an argument to a member function, a
free-standing function works just as well
This might seem nit-picky, but in my experience, the more we can reduce the
test cases, the easier to read and the less brittle they become.
>> ...and it should also apply to multiline strings if
>> AlwaysBreakBeforeMultilineStrings is true. It doesn't all have to be done in
>> the same patch, but we should have a plan of what we want the end result to
>> look like and should be willing to get there.
>>
>> Maybe to start with, we need the complete test matrix so that we are
>> definitely on the same page as to what we are trying to do. I imagine, we
>> would need tests for a function with three parameters, where two of the
>> parameters are short and one is a multiline lambda or a multiline string
>> (and have the lambda be the first, second and third parameter). Then we
>> might need those for both bin-packing and no-bin-packing configurations.
Repository:
rC Clang
https://reviews.llvm.org/D52676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
dexonsmith added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
arphaman wrote:
> arphaman wrote:
> > arphaman wrote:
> > > ddunbar wrote:
> > > > What is our feeling w.r.t. _Pragma, which can in theory influence the
> > > > preprocessor. I'm not sure this model can sanely support it?
> > > We need to look into that. In the worst case we can always avoid
> > > minimizing the file if it has a _Pragma anywhere in it.
> > We also have to handle cases like this one:
> >
> > foo.h:
> > ```
> > #define PP _Pragma
> > ```
> >
> > bar.h:
> > ```
> > PP ("foo")
> > ```
> >
> > Unfortunately this can't be handled by just disabling minimization for
> > `foo.h` as PP will be stripped out from `bar.h`. Furthermore, this pattern
> > is quite reasonable, so we should expect it in the code we receive. Right
> > now I can't think of a reasonable solution for this problem.
> >
> > There's also a more "evil" case:
> >
> > ```
> > #define P(A, B) A##B
> >
> > P(_Pra,gma)("foo")
> > ```
> >
> > It would be reasonable to introduce a warning for the use of `_Pragma`
> > token that was created using PP token concatenation and just hope that our
> > users won't use this pattern.
> One possible solution to the first issue is to simply fallback to the regular
> -Eonly invocation if we detect an include of a file that has a macro with a
> `_Pragma` in it. Then we could emit a remark to the user saying that their
> build could be faster if they rewrite their code so that this pattern no
> longer occurs.
Hmm. Our plan for `@import` is to stop supporting code such as:
```
#define IMPORT(M) @import M
IMPORT(LLVMSupport);
```
where the @import is buried. I.e., start erroring out in normal Clang builds.
Perhaps it would be reasonable to similarly disallow `_Pragma` usage that
buries import/include statements. I.e., do not support (even in normal builds)
the following:
```
#define IMPORT(M) _Pragma("clang module import " #M)
IMPORT(LLVMSupport)
```
Possibly, this could be a warning that is error-by-default.
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.
alexshap added a comment. I talked to David @dblaikie offline about this diff yesterday, if I understood correctly this change is reasonable in general, @echristo Eric, would you mind having a look at this diff ? https://reviews.llvm.org/D52296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.
alexshap added a comment. Ping https://reviews.llvm.org/D52296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53263: [CodeGen] Fix places where the return type of a FunctionDecl was being used in place of the function type
bobsayshilol updated this revision to Diff 170136.
bobsayshilol edited the summary of this revision.
bobsayshilol added a comment.
Fixed the added assert to do the right thing. Clang can now build with a debug
Clang built from this patch, and all unittests I could find pass in that built
version too.
https://reviews.llvm.org/D53263
Files:
lib/AST/Decl.cpp
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGBuiltin.cpp
lib/CodeGen/CGNonTrivialStruct.cpp
lib/CodeGen/CGObjC.cpp
lib/CodeGen/CGStmtOpenMP.cpp
lib/CodeGen/ItaniumCXXABI.cpp
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2315,11 +2315,13 @@
FTy, GlobalInitFnName, getTypes().arrangeNullaryFunction(),
SourceLocation());
ASTContext = getContext();
+QualType ReturnTy = Ctx.VoidTy;
+QualType FunctionTy = Ctx.getFunctionType(ReturnTy, llvm::None, {});
FunctionDecl *FD = FunctionDecl::Create(
Ctx, Ctx.getTranslationUnitDecl(), SourceLocation(), SourceLocation(),
-(GlobalInitFnName), Ctx.VoidTy, nullptr, SC_Static,
+(GlobalInitFnName), FunctionTy, nullptr, SC_Static,
false, false);
-CGF.StartFunction(GlobalDecl(FD), getContext().VoidTy, GlobalInitFn,
+CGF.StartFunction(GlobalDecl(FD), ReturnTy, GlobalInitFn,
getTypes().arrangeNullaryFunction(), FunctionArgList(),
SourceLocation(), SourceLocation());
Index: lib/CodeGen/CGStmtOpenMP.cpp
===
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -385,11 +385,11 @@
FunctionDecl *DebugFunctionDecl = nullptr;
if (!FO.UIntPtrCastRequired) {
FunctionProtoType::ExtProtoInfo EPI;
+QualType FunctionTy = Ctx.getFunctionType(Ctx.VoidTy, llvm::None, EPI);
DebugFunctionDecl = FunctionDecl::Create(
Ctx, Ctx.getTranslationUnitDecl(), FO.S->getBeginLoc(),
-SourceLocation(), DeclarationName(), Ctx.VoidTy,
-Ctx.getTrivialTypeSourceInfo(
-Ctx.getFunctionType(Ctx.VoidTy, llvm::None, EPI)),
+SourceLocation(), DeclarationName(), FunctionTy,
+Ctx.getTrivialTypeSourceInfo(FunctionTy),
SC_Static, /*isInlineSpecified=*/false, /*hasWrittenPrototype=*/false);
}
for (const FieldDecl *FD : RD->fields()) {
Index: lib/CodeGen/CGObjC.cpp
===
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -3229,29 +3229,36 @@
ASTContext = getContext();
IdentifierInfo *II
= ().Idents.get("__assign_helper_atomic_property_");
+
+ QualType ReturnTy = C.VoidTy;
+ QualType DestTy = C.getPointerType(Ty);
+ QualType SrcTy = Ty;
+ SrcTy.addConst();
+ SrcTy = C.getPointerType(SrcTy);
+
+ SmallVector ArgTys;
+ ArgTys.push_back(DestTy);
+ ArgTys.push_back(SrcTy);
+ QualType FunctionTy = C.getFunctionType(ReturnTy, ArgTys, {});
+
FunctionDecl *FD = FunctionDecl::Create(C,
C.getTranslationUnitDecl(),
SourceLocation(),
- SourceLocation(), II, C.VoidTy,
+ SourceLocation(), II, FunctionTy,
nullptr, SC_Static,
false,
false);
- QualType DestTy = C.getPointerType(Ty);
- QualType SrcTy = Ty;
- SrcTy.addConst();
- SrcTy = C.getPointerType(SrcTy);
-
FunctionArgList args;
- ImplicitParamDecl DstDecl(getContext(), FD, SourceLocation(), /*Id=*/nullptr,
+ ImplicitParamDecl DstDecl(C, FD, SourceLocation(), /*Id=*/nullptr,
DestTy, ImplicitParamDecl::Other);
args.push_back();
- ImplicitParamDecl SrcDecl(getContext(), FD, SourceLocation(), /*Id=*/nullptr,
+ ImplicitParamDecl SrcDecl(C, FD, SourceLocation(), /*Id=*/nullptr,
SrcTy, ImplicitParamDecl::Other);
args.push_back();
const CGFunctionInfo =
-CGM.getTypes().arrangeBuiltinFunctionDeclaration(C.VoidTy, args);
+CGM.getTypes().arrangeBuiltinFunctionDeclaration(ReturnTy, args);
llvm::FunctionType *LTy = CGM.getTypes().GetFunctionType(FI);
@@ -3262,7 +3269,7 @@
CGM.SetInternalFunctionAttributes(GlobalDecl(), Fn, FI);
- StartFunction(FD, C.VoidTy, Fn, FI, args);
+ StartFunction(FD, ReturnTy, Fn, FI, args);
DeclRefExpr DstExpr(, false, DestTy,
VK_RValue, SourceLocation());
@@ -3301,50 +3308,56 @@
if ((!(PD->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_atomic)))
return nullptr;
llvm::Constant *HelperFn = nullptr;
-
if (hasTrivialGetExpr(PID))
return nullptr;
assert(PID->getGetterCXXConstructor() && "getGetterCXXConstructor - null");
if ((HelperFn =
r344767 - [COFF, ARM64] Enable unit test arm64-microsoft-status-reg.cpp only for aarch64 target
Author: mgrang Date: Thu Oct 18 17:05:26 2018 New Revision: 344767 URL: http://llvm.org/viewvc/llvm-project?rev=344767=rev Log: [COFF, ARM64] Enable unit test arm64-microsoft-status-reg.cpp only for aarch64 target This should unbreak bots broken here: http://lab.llvm.org:8011/builders/clang-cmake-x86_64-sde-avx512-linux/builds/14391 http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/38288 Modified: cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp Modified: cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp?rev=344767=344766=344767=diff == --- cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp (original) +++ cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp Thu Oct 18 17:05:26 2018 @@ -1,3 +1,5 @@ +// REQUIRES: aarch64-registered-target + // RUN: %clang_cc1 -triple arm64-windows -fms-compatibility -emit-llvm -S \ // RUN: -o - %s | FileCheck %s -check-prefix CHECK-ASM ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53238: [Driver] Add -static-{rtlib, stdlib} and make -static-{libgcc, libstdc++} their aliases
MaskRay added a comment. Greeting from a dev meeting attendee :) Repository: rC Clang https://reviews.llvm.org/D53238 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53115: [COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg intrinsics
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344765: [COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg
intrinsics (authored by mgrang, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D53115
Files:
include/clang/Basic/BuiltinsAArch64.def
lib/CodeGen/CGBuiltin.cpp
lib/Headers/intrin.h
lib/Sema/SemaChecking.cpp
test/CodeGen/arm64-microsoft-status-reg.cpp
test/Sema/builtins-microsoft-arm64.c
Index: test/CodeGen/arm64-microsoft-status-reg.cpp
===
--- test/CodeGen/arm64-microsoft-status-reg.cpp
+++ test/CodeGen/arm64-microsoft-status-reg.cpp
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 -triple arm64-windows -fms-compatibility -emit-llvm -S \
+// RUN: -o - %s | FileCheck %s -check-prefix CHECK-ASM
+
+// RUN: %clang_cc1 -triple arm64-windows -fms-compatibility -emit-llvm \
+// RUN: -o - %s | FileCheck %s -check-prefix CHECK-IR
+
+// From winnt.h
+#define ARM64_SYSREG(op0, op1, crn, crm, op2) \
+( ((op0 & 1) << 14) | \
+ ((op1 & 7) << 11) | \
+ ((crn & 15) << 7) | \
+ ((crm & 15) << 3) | \
+ ((op2 & 7) << 0) )
+
+#define ARM64_CNTVCTARM64_SYSREG(3,3,14, 0,2) // Generic Timer counter register
+#define ARM64_PMCCNTR_EL0 ARM64_SYSREG(3,3, 9,13,0) // Cycle Count Register [CP15_PMCCNTR]
+#define ARM64_PMSELR_EL0ARM64_SYSREG(3,3, 9,12,5) // Event Counter Selection Register [CP15_PMSELR]
+#define ARM64_PMXEVCNTR_EL0 ARM64_SYSREG(3,3, 9,13,2) // Event Count Register [CP15_PMXEVCNTR]
+#define ARM64_PMXEVCNTRn_EL0(n) ARM64_SYSREG(3,3,14, 8+((n)/8), (n)%8)// Direct Event Count Register [n/a]
+#define ARM64_TPIDR_EL0 ARM64_SYSREG(3,3,13, 0,2) // Thread ID Register, User Read/Write [CP15_TPIDRURW]
+#define ARM64_TPIDRRO_EL0 ARM64_SYSREG(3,3,13, 0,3) // Thread ID Register, User Read Only [CP15_TPIDRURO]
+#define ARM64_TPIDR_EL1 ARM64_SYSREG(3,0,13, 0,4) // Thread ID Register, Privileged Only [CP15_TPIDRPRW]
+
+void check_ReadWriteStatusReg(int v) {
+ int ret;
+ ret = _ReadStatusReg(ARM64_CNTVCT);
+// CHECK-ASM: mrs x8, CNTVCT_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMCCNTR_EL0);
+// CHECK-ASM: mrs x8, PMCCNTR_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMSELR_EL0);
+// CHECK-ASM: mrs x8, PMSELR_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD4:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMXEVCNTR_EL0);
+// CHECK-ASM: mrs x8, PMXEVCNTR_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD5:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(0));
+// CHECK-ASM: mrs x8, PMEVCNTR0_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD6:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(1));
+// CHECK-ASM: mrs x8, PMEVCNTR1_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD7:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(30));
+// CHECK-ASM: mrs x8, PMEVCNTR30_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD8:.*]])
+
+ ret = _ReadStatusReg(ARM64_TPIDR_EL0);
+// CHECK-ASM: mrs x8, TPIDR_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD9:.*]])
+
+ ret = _ReadStatusReg(ARM64_TPIDRRO_EL0);
+// CHECK-ASM: mrs x8, TPIDRRO_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD10:.*]])
+
+ ret = _ReadStatusReg(ARM64_TPIDR_EL1);
+// CHECK-ASM: mrs x8, TPIDR_EL1
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD11:.*]])
+
+
+ _WriteStatusReg(ARM64_CNTVCT, v);
+// CHECK-ASM: msr S3_3_C14_C0_2, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD2:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMCCNTR_EL0, v);
+// CHECK-ASM: msr PMCCNTR_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD3:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMSELR_EL0, v);
+// CHECK-ASM: msr PMSELR_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD4:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMXEVCNTR_EL0, v);
+// CHECK-ASM: msr PMXEVCNTR_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD5:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMXEVCNTRn_EL0(0), v);
+// CHECK-ASM: msr PMEVCNTR0_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD6:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMXEVCNTRn_EL0(1), v);
+// CHECK-ASM: msr PMEVCNTR1_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD7:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMXEVCNTRn_EL0(30), v);
+// CHECK-ASM: msr PMEVCNTR30_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD8:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_TPIDR_EL0, v);
+// CHECK-ASM: msr TPIDR_EL0, x8
+// CHECK-IR: call void
r344765 - [COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg intrinsics
Author: mgrang
Date: Thu Oct 18 16:35:35 2018
New Revision: 344765
URL: http://llvm.org/viewvc/llvm-project?rev=344765=rev
Log:
[COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg intrinsics
Reviewers: rnk, compnerd, mstorsjo, efriedma, TomTan, haripul, javed.absar
Reviewed By: efriedma
Subscribers: dmajor, kristof.beyls, chrib, cfe-commits
Differential Revision: https://reviews.llvm.org/D53115
Added:
cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp
Modified:
cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/builtins-microsoft-arm64.c
Modified: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAArch64.def?rev=344765=344764=344765=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def Thu Oct 18 16:35:35 2018
@@ -106,6 +106,8 @@ TARGET_HEADER_BUILTIN(_InterlockedXor64,
TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h",
ALL_MS_LANGUAGES, "")
TARGET_HEADER_BUILTIN(__getReg, "ULLii", "nh", "intrin.h", ALL_MS_LANGUAGES,
"")
+TARGET_HEADER_BUILTIN(_ReadStatusReg, "ii", "nh", "intrin.h",
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_WriteStatusReg, "vii", "nh", "intrin.h",
ALL_MS_LANGUAGES, "")
#undef BUILTIN
#undef LANGBUILTIN
Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=344765=344764=344765=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Oct 18 16:35:35 2018
@@ -6667,6 +6667,43 @@ Value *CodeGenFunction::EmitAArch64Built
return EmitSpecialRegisterBuiltin(*this, E, RegisterType, ValueType,
IsRead);
}
+ if (BuiltinID == AArch64::BI_ReadStatusReg ||
+ BuiltinID == AArch64::BI_WriteStatusReg) {
+LLVMContext = CGM.getLLVMContext();
+
+unsigned SysReg =
+ E->getArg(0)->EvaluateKnownConstInt(getContext()).getZExtValue();
+
+std::string SysRegStr;
+llvm::raw_string_ostream(SysRegStr) <<
+ ((1 << 1) | ((SysReg >> 14) & 1)) << ":" <<
+ ((SysReg >> 11) & 7) << ":" <<
+ ((SysReg >> 7) & 15) << ":" <<
+ ((SysReg >> 3) & 15) << ":" <<
+ ( SysReg& 7);
+
+llvm::Metadata *Ops[] = { llvm::MDString::get(Context, SysRegStr) };
+llvm::MDNode *RegName = llvm::MDNode::get(Context, Ops);
+llvm::Value *Metadata = llvm::MetadataAsValue::get(Context, RegName);
+
+llvm::Type *RegisterType = Int64Ty;
+llvm::Type *ValueType = Int32Ty;
+llvm::Type *Types[] = { RegisterType };
+
+if (BuiltinID == AArch64::BI_ReadStatusReg) {
+ llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::read_register, Types);
+ llvm::Value *Call = Builder.CreateCall(F, Metadata);
+
+ return Builder.CreateTrunc(Call, ValueType);
+}
+
+llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::write_register, Types);
+llvm::Value *ArgValue = EmitScalarExpr(E->getArg(1));
+ArgValue = Builder.CreateZExt(ArgValue, RegisterType);
+
+return Builder.CreateCall(F, { Metadata, ArgValue });
+ }
+
// Find out if any arguments are required to be integer constant
// expressions.
unsigned ICEArguments = 0;
Modified: cfe/trunk/lib/Headers/intrin.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=344765=344764=344765=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Thu Oct 18 16:35:35 2018
@@ -870,6 +870,8 @@ __nop(void) {
#if defined(__aarch64__)
unsigned __int64 __getReg(int);
long _InterlockedAdd(long volatile *Addend, long Value);
+int _ReadStatusReg(int);
+void _WriteStatusReg(int, int);
#endif
/**\
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=344765=344764=344765=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Oct 18 16:35:35 2018
@@ -1749,6 +1749,13 @@ bool Sema::CheckAArch64BuiltinFunctionCa
BuiltinID == AArch64::BI__builtin_arm_wsrp)
return SemaBuiltinARMSpecialReg(BuiltinID, TheCall, 0, 5, true);
+ // Only check the valid encoding range. Any constant in this range would be
+ // converted to a register of the form S1_2_C3_C4_5. Let the hardware
[PATCH] D53115: [COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg intrinsics
efriedma accepted this revision. efriedma added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D53115 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line
russellmcc added a comment. Bump! Thanks for your time! https://reviews.llvm.org/D40988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53115: [COFF, ARM64] Add _ReadStatusReg and_WriteStatusReg intrinsics
mgrang updated this revision to Diff 170133.
https://reviews.llvm.org/D53115
Files:
include/clang/Basic/BuiltinsAArch64.def
lib/CodeGen/CGBuiltin.cpp
lib/Headers/intrin.h
lib/Sema/SemaChecking.cpp
test/CodeGen/arm64-microsoft-status-reg.cpp
test/Sema/builtins-microsoft-arm64.c
Index: test/Sema/builtins-microsoft-arm64.c
===
--- test/Sema/builtins-microsoft-arm64.c
+++ test/Sema/builtins-microsoft-arm64.c
@@ -7,3 +7,9 @@
__getReg(-1); // expected-error-re {{argument value {{.*}} is outside the valid range}}
__getReg(32); // expected-error-re {{argument value {{.*}} is outside the valid range}}
}
+
+void check_ReadWriteStatusReg(int v) {
+ int x;
+ _ReadStatusReg(x); // expected-error {{argument to '_ReadStatusReg' must be a constant integer}}
+ _WriteStatusReg(x, v); // expected-error {{argument to '_WriteStatusReg' must be a constant integer}}
+}
Index: test/CodeGen/arm64-microsoft-status-reg.cpp
===
--- /dev/null
+++ test/CodeGen/arm64-microsoft-status-reg.cpp
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 -triple arm64-windows -fms-compatibility -emit-llvm -S \
+// RUN: -o - %s | FileCheck %s -check-prefix CHECK-ASM
+
+// RUN: %clang_cc1 -triple arm64-windows -fms-compatibility -emit-llvm \
+// RUN: -o - %s | FileCheck %s -check-prefix CHECK-IR
+
+// From winnt.h
+#define ARM64_SYSREG(op0, op1, crn, crm, op2) \
+( ((op0 & 1) << 14) | \
+ ((op1 & 7) << 11) | \
+ ((crn & 15) << 7) | \
+ ((crm & 15) << 3) | \
+ ((op2 & 7) << 0) )
+
+#define ARM64_CNTVCTARM64_SYSREG(3,3,14, 0,2) // Generic Timer counter register
+#define ARM64_PMCCNTR_EL0 ARM64_SYSREG(3,3, 9,13,0) // Cycle Count Register [CP15_PMCCNTR]
+#define ARM64_PMSELR_EL0ARM64_SYSREG(3,3, 9,12,5) // Event Counter Selection Register [CP15_PMSELR]
+#define ARM64_PMXEVCNTR_EL0 ARM64_SYSREG(3,3, 9,13,2) // Event Count Register [CP15_PMXEVCNTR]
+#define ARM64_PMXEVCNTRn_EL0(n) ARM64_SYSREG(3,3,14, 8+((n)/8), (n)%8)// Direct Event Count Register [n/a]
+#define ARM64_TPIDR_EL0 ARM64_SYSREG(3,3,13, 0,2) // Thread ID Register, User Read/Write [CP15_TPIDRURW]
+#define ARM64_TPIDRRO_EL0 ARM64_SYSREG(3,3,13, 0,3) // Thread ID Register, User Read Only [CP15_TPIDRURO]
+#define ARM64_TPIDR_EL1 ARM64_SYSREG(3,0,13, 0,4) // Thread ID Register, Privileged Only [CP15_TPIDRPRW]
+
+void check_ReadWriteStatusReg(int v) {
+ int ret;
+ ret = _ReadStatusReg(ARM64_CNTVCT);
+// CHECK-ASM: mrs x8, CNTVCT_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMCCNTR_EL0);
+// CHECK-ASM: mrs x8, PMCCNTR_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMSELR_EL0);
+// CHECK-ASM: mrs x8, PMSELR_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD4:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMXEVCNTR_EL0);
+// CHECK-ASM: mrs x8, PMXEVCNTR_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD5:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(0));
+// CHECK-ASM: mrs x8, PMEVCNTR0_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD6:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(1));
+// CHECK-ASM: mrs x8, PMEVCNTR1_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD7:.*]])
+
+ ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(30));
+// CHECK-ASM: mrs x8, PMEVCNTR30_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD8:.*]])
+
+ ret = _ReadStatusReg(ARM64_TPIDR_EL0);
+// CHECK-ASM: mrs x8, TPIDR_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD9:.*]])
+
+ ret = _ReadStatusReg(ARM64_TPIDRRO_EL0);
+// CHECK-ASM: mrs x8, TPIDRRO_EL0
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD10:.*]])
+
+ ret = _ReadStatusReg(ARM64_TPIDR_EL1);
+// CHECK-ASM: mrs x8, TPIDR_EL1
+// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD11:.*]])
+
+
+ _WriteStatusReg(ARM64_CNTVCT, v);
+// CHECK-ASM: msr S3_3_C14_C0_2, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD2:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMCCNTR_EL0, v);
+// CHECK-ASM: msr PMCCNTR_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD3:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMSELR_EL0, v);
+// CHECK-ASM: msr PMSELR_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD4:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMXEVCNTR_EL0, v);
+// CHECK-ASM: msr PMXEVCNTR_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD5:.*]], i64 {{%.*}})
+
+ _WriteStatusReg(ARM64_PMXEVCNTRn_EL0(0), v);
+// CHECK-ASM: msr PMEVCNTR0_EL0, x8
+// CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD6:.*]], i64 {{%.*}})
+
+
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
arphaman added a comment. In https://reviews.llvm.org/D53354#1267376, @whisperity wrote: > With regards to https://reviews.llvm.org/D53352: you can change the diff > related to a patch whilst keeping discuccion and metadata of the diff. Good point, thanks! > Please add a generic description to the diff for an easy skimming on what you > are accomplishing. I added the description, thanks. > If I get this right, your tool will spit out a CPP file that is only include > directives and perhaps the related conditional logic, or the final output of > your tool is a file list? It's both, as there are two tools in this patch. The first is the `clang-filter-to-includes` tool, which is wrapper around our source minimization optimization (i.e. it spits out a source with `#includes` and other PP directives). The `clang-scan-deps` tool integrates this optimization into a service-like tool that will produce a set of dependencies for a set of compilation commands. Right now it's mainly a benchmark that compares the speed of the fast scanner to the regular preprocessor invocation. > This is different than the `-M` flags in a way that it keeps conditions sane, > rather than spitting out what includes were used if the input, with regards > to the compiler options, was to be compiled? It's supposed to produce identical output to the run of `-Eonly` with the `-MD` flag. Right now we don't know how we want to expose the dependency set to the clients. > Have you checked the tool //Include What You Use//? I'm curious in what way > the mishappenings of that tool present themselves in yours. There were some > challenges not overcome in a good way in IWYU, their readme goes into a bit > more detail on this. I haven't looked into IWYU that much. Could you please elaborate on which mishappenings you think might present themselves here? Repository: rC Clang https://reviews.llvm.org/D53354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
oleg.smolsky added a comment. In https://reviews.llvm.org/D52676#1268706, @djasper wrote: > Ok, I think I agree with both of you to a certain extent, but I also think > this change as is, is not yet right. > > First, it does too much. The original very first example in this CL is > actually not produced by clang-format (or if it is, I don't know with which > flag combination). It is a case where the lambda is the last parameter. Right, I cheated and created that example by hand. My apologies for the confusion. I've just pasted real code that I pumped through `clang-format`. Please take a look at the updated summary. > Second, I agree that the original behavior is inconsistent in a way that we > have a special cases for when the lambda is the very first parameter, but > somehow seem be forgetting about that when it's not the first parameter. I'd > be ok with "fixing" that (it's not a clear-cut bug, but I think the > alternative behavior would be cleaner). However, I don't think your patch is > doing enough there. I think this should be irrespective of bin-packing (it's > related but not quite the same thing), Also there is a special case for multiple lambdas. It forces line breaks. That aside, for the single-lambda case, are you suggesting that it is always "pulled up", irrespective of its place? That would contradict the direction I am trying to take as I like `BinPackArguments: false` and so long lamba args go to their own lines. This looks very much in line with what bin packing is, but not exactly the same. Obviously, I can add a new option `favor line breaks around multi-line lambda`. Could you look at the updated summary (high level) and the new tests I added (low level) please? Every other test passes, so we have almost the entire spec. I can add a few cases where an existing snippet is reformatted with `BinPackArguments: false` too. > ...and it should also apply to multiline strings if > AlwaysBreakBeforeMultilineStrings is true. It doesn't all have to be done in > the same patch, but we should have a plan of what we want the end result to > look like and should be willing to get there. > > Maybe to start with, we need the complete test matrix so that we are > definitely on the same page as to what we are trying to do. I imagine, we > would need tests for a function with three parameters, where two of the > parameters are short and one is a multiline lambda or a multiline string (and > have the lambda be the first, second and third parameter). Then we might need > those for both bin-packing and no-bin-packing configurations. Repository: rC Clang https://reviews.llvm.org/D52676 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51633: [ASTImporter] Added error handling for AST import.
a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.
Hi Balasz,
I cannot find any problems with the latest changes. I suggest you to commit the
patch to make further reviews easier.
Comment at: lib/AST/ASTImporter.cpp:8197
-void ASTImporter::ImportDefinition(Decl *From) {
+Error ASTImporter::ImportDefinition_New(Decl *From) {
Decl *To = Import(From);
ImportDefinitionOrError?
Repository:
rC Clang
https://reviews.llvm.org/D51633
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52676: [clang-format] tweaked another case of lambda formatting
djasper added a comment.
Ok, I think I agree with both of you to a certain extent, but I also think this
change as is, is not yet right.
First, it does too much. The original very first example in this CL is actually
not produced by clang-format (or if it is, I don't know with which flag
combination). It is a case where the lambda is the last parameter. For me, it
actually produces:
void f() {
something.something.something.Method(some_arg, [] {
// the code here incurs
// excessive wrapping
// such as
Method(some_med_arg, some_med_arg);
some_var = some_expr + something;
});
}
And that's an intentional optimization for a very common lambda use case. It
reduces indentation even further and makes some coding patterns much nicer. I
think (but haven't reproduced) that you patch might change the behavior there.
Second, I agree that the original behavior is inconsistent in a way that we
have a special cases for when the lambda is the very first parameter, but
somehow seem be forgetting about that when it's not the first parameter. I'd be
ok with "fixing" that (it's not a clear-cut bug, but I think the alternative
behavior would be cleaner). However, I don't think your patch is doing enough
there. I think this should be irrespective of bin-packing (it's related but not
quite the same thing), and it should also apply to multiline strings if
AlwaysBreakBeforeMultilineStrings is true. It doesn't all have to be done in
the same patch, but we should have a plan of what we want the end result to
look like and should be willing to get there.
Maybe to start with, we need the complete test matrix so that we are definitely
on the same page as to what we are trying to do. I imagine, we would need tests
for a function with three parameters, where two of the parameters are short and
one is a multiline lambda or a multiline string (and have the lambda be the
first, second and third parameter). Then we might need those for both
bin-packing and no-bin-packing configurations.
Repository:
rC Clang
https://reviews.llvm.org/D52676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344762 - [Test] Fix test file for C++98 mode
Author: xbolva00
Date: Thu Oct 18 14:26:01 2018
New Revision: 344762
URL: http://llvm.org/viewvc/llvm-project?rev=344762=rev
Log:
[Test] Fix test file for C++98 mode
Modified:
cfe/trunk/test/SemaCXX/enum.cpp
Modified: cfe/trunk/test/SemaCXX/enum.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enum.cpp?rev=344762=344761=344762=diff
==
--- cfe/trunk/test/SemaCXX/enum.cpp (original)
+++ cfe/trunk/test/SemaCXX/enum.cpp Thu Oct 18 14:26:01 2018
@@ -105,10 +105,12 @@ void PR8089() {
// This is accepted as a GNU extension. In C++98, there was no provision for
// expressions with UB to be non-constant.
-enum { overflow = 123456 * 234567 }; // expected-warning {{overflow in
expression; result is -1106067520 with type 'int'}}
+enum { overflow = 123456 * 234567 };
#if __cplusplus >= 201103L
// expected-warning@-2 {{not an integral constant expression}}
// expected-note@-3 {{value 28958703552 is outside the range of representable
values}}
+#else
+// expected-warning@-5 {{overflow in expression; result is -1106067520 with
type 'int'}}
#endif
// PR28903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344761 - [Diagnostics] Add missing expected warning to test file
Author: xbolva00
Date: Thu Oct 18 14:06:14 2018
New Revision: 344761
URL: http://llvm.org/viewvc/llvm-project?rev=344761=rev
Log:
[Diagnostics] Add missing expected warning to test file
Modified:
cfe/trunk/test/SemaCXX/enum.cpp
Modified: cfe/trunk/test/SemaCXX/enum.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/enum.cpp?rev=344761=344760=344761=diff
==
--- cfe/trunk/test/SemaCXX/enum.cpp (original)
+++ cfe/trunk/test/SemaCXX/enum.cpp Thu Oct 18 14:06:14 2018
@@ -105,7 +105,7 @@ void PR8089() {
// This is accepted as a GNU extension. In C++98, there was no provision for
// expressions with UB to be non-constant.
-enum { overflow = 123456 * 234567 };
+enum { overflow = 123456 * 234567 }; // expected-warning {{overflow in
expression; result is -1106067520 with type 'int'}}
#if __cplusplus >= 201103L
// expected-warning@-2 {{not an integral constant expression}}
// expected-note@-3 {{value 28958703552 is outside the range of representable
values}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344760 - [clang-tidy] readability-uppercase-literal-suffix: specify target for opencl test
Author: lebedevri Date: Thu Oct 18 13:59:03 2018 New Revision: 344760 URL: http://llvm.org/viewvc/llvm-project?rev=344760=rev Log: [clang-tidy] readability-uppercase-literal-suffix: specify target for opencl test I'm not sure if it will actually help or not. ppc64be-clang-lnt-test bot is failing. Modified: clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp Modified: clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp?rev=344760=344759=344760=diff == --- clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp Thu Oct 18 13:59:03 2018 @@ -1,7 +1,7 @@ -// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S -std=cl2.0 -x cl +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -target x86_64-pc-linux-gnu -I %S -std=cl2.0 -x cl // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S -std=cl2.0 -x cl -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S -std=cl2.0 -x cl +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -target x86_64-pc-linux-gnu -I %S -std=cl2.0 -x cl +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -target x86_64-pc-linux-gnu -I %S -std=cl2.0 -x cl #pragma OPENCL EXTENSION cl_khr_fp16 : enable ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
krasimir requested changes to this revision. krasimir added a comment. This revision now requires changes to proceed. As per https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options, could you please provide more info about the need for this option first? https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53410: Add missing PATH_MAX for GNU Hurd support
rsmith added inline comments. Comment at: lib/Basic/FileManager.cpp:514-519 +// For GNU Hurd +#if defined(__GNU__) && !defined(PATH_MAX) +# define PATH_MAX 4096 +#endif + + This doesn't appear to be necessary: the identifier PATH_MAX does not appear later in this file. Comment at: lib/Frontend/ModuleDependencyCollector.cpp:102-106 +// For GNU Hurd +#if defined(__GNU__) && !defined(PATH_MAX) +# define PATH_MAX 4096 +#endif + I think this is generally the wrong way to address this problem on Hurd: https://www.gnu.org/software/hurd/hurd/porting/guidelines.html#PATH_MAX_tt_MAX_PATH_tt_MAXPATHL If `PATH_MAX` isn't defined, we should instead pass `NULL` to `realpath` (and deallocate the result): passing a fixed-size buffer is wrong, because we cannot assume that the real path will fit in 4096 bytes. (If we neither have `PATH_MAX` defined nor a `realpath` that can accept a null pointer as the destination buffer, then `realpath` is unusable on that platform and there's nothing we can do there but return a `false` result to the caller of `real_path`.) Repository: rC Clang https://reviews.llvm.org/D53410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping
krasimir requested changes to this revision. krasimir added a comment. This revision now requires changes to proceed. OK, so this is not a real bug in the sense of non-working current features, it's more like a feature request. As per https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options, I (and I'm sure other reviewers) would like to understand better this use-case before it's added to clang-format. Repository: rC Clang https://reviews.llvm.org/D52527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines
krasimir requested changes to this revision. krasimir added a comment. This revision now requires changes to proceed. In general there's a high bar for adding new style options to clang-format: https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options https://reviews.llvm.org/D53072 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344759: [Diagnostics] Check for integer overflow in array
size expressions (authored by xbolva00, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52750?vs=169409=170119#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52750
Files:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/integer-overflow.c
Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -10851,6 +10851,19 @@
return EvalResult.Val.getInt();
}
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext , SmallVectorImpl *Diag) const {
+ EvalResult EvalResult;
+ EvalResult.Diag = Diag;
+ EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+ bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+ (void)Result;
+ assert(Result && "Could not evaluate expression");
+ assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+ return EvalResult.Val.getInt();
+}
+
void Expr::EvaluateForOverflow(const ASTContext ) const {
bool IsConst;
EvalResult EvalResult;
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -14105,7 +14105,7 @@
// in the non-ICE case.
if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
if (Result)
- *Result = E->EvaluateKnownConstInt(Context);
+ *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
return E;
}
Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -631,8 +631,13 @@
/// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
/// integer. This must be called on an expression that constant folds to an
/// integer.
- llvm::APSInt EvaluateKnownConstInt(const ASTContext ,
-SmallVectorImpl *Diag = nullptr)
const;
+ llvm::APSInt EvaluateKnownConstInt(
+ const ASTContext ,
+ SmallVectorImpl *Diag = nullptr) const;
+
+ llvm::APSInt EvaluateKnownConstIntCheckOverflow(
+ const ASTContext ,
+ SmallVectorImpl *Diag = nullptr) const;
void EvaluateForOverflow(const ASTContext ) const;
Index: cfe/trunk/test/Sema/integer-overflow.c
===
--- cfe/trunk/test/Sema/integer-overflow.c
+++ cfe/trunk/test/Sema/integer-overflow.c
@@ -172,6 +172,9 @@
// expected-warning@+1 {{overflow in expression; result is 536870912 with type
'int'}}
(void)f2(0, f0(4608 * 1024 * 1024));
}
+void check_integer_overflows_in_array_size() {
+ int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression;
result is 536870912 with type 'int'}}
+}
struct s {
unsigned x;
Index: cfe/trunk/lib/AST/ExprConstant.cpp
===
--- cfe/trunk/lib/AST/ExprConstant.cpp
+++ cfe/trunk/lib/AST/ExprConstant.cpp
@@ -10851,6 +10851,19 @@
return EvalResult.Val.getInt();
}
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext , SmallVectorImpl *Diag) const {
+ EvalResult EvalResult;
+ EvalResult.Diag = Diag;
+ EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+ bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+ (void)Result;
+ assert(Result && "Could not evaluate expression");
+ assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+ return EvalResult.Val.getInt();
+}
+
void Expr::EvaluateForOverflow(const ASTContext ) const {
bool IsConst;
EvalResult EvalResult;
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -14105,7 +14105,7 @@
// in the non-ICE case.
if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
if (Result)
- *Result = E->EvaluateKnownConstInt(Context);
+ *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
return E;
}
Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -631,8 +631,13 @@
/// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
/// integer. This must be called on an expression that constant folds to an
/// integer.
- llvm::APSInt EvaluateKnownConstInt(const ASTContext ,
-SmallVectorImpl *Diag = nullptr) const;
+
r344759 - [Diagnostics] Check for integer overflow in array size expressions
Author: xbolva00
Date: Thu Oct 18 13:49:06 2018
New Revision: 344759
URL: http://llvm.org/viewvc/llvm-project?rev=344759=rev
Log:
[Diagnostics] Check for integer overflow in array size expressions
Summary: Fixes PR27439
Reviewers: rsmith, Rakete
Reviewed By: rsmith
Subscribers: Rakete, cfe-commits
Differential Revision: https://reviews.llvm.org/D52750
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/integer-overflow.c
Modified: cfe/trunk/include/clang/AST/Expr.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=344759=344758=344759=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Oct 18 13:49:06 2018
@@ -631,8 +631,13 @@ public:
/// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
/// integer. This must be called on an expression that constant folds to an
/// integer.
- llvm::APSInt EvaluateKnownConstInt(const ASTContext ,
-SmallVectorImpl *Diag = nullptr)
const;
+ llvm::APSInt EvaluateKnownConstInt(
+ const ASTContext ,
+ SmallVectorImpl *Diag = nullptr) const;
+
+ llvm::APSInt EvaluateKnownConstIntCheckOverflow(
+ const ASTContext ,
+ SmallVectorImpl *Diag = nullptr) const;
void EvaluateForOverflow(const ASTContext ) const;
Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=344759=344758=344759=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Oct 18 13:49:06 2018
@@ -10851,6 +10851,19 @@ APSInt Expr::EvaluateKnownConstInt(const
return EvalResult.Val.getInt();
}
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext , SmallVectorImpl *Diag) const {
+ EvalResult EvalResult;
+ EvalResult.Diag = Diag;
+ EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+ bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+ (void)Result;
+ assert(Result && "Could not evaluate expression");
+ assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+ return EvalResult.Val.getInt();
+}
+
void Expr::EvaluateForOverflow(const ASTContext ) const {
bool IsConst;
EvalResult EvalResult;
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=344759=344758=344759=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Oct 18 13:49:06 2018
@@ -14105,7 +14105,7 @@ Sema::VerifyIntegerConstantExpression(Ex
// in the non-ICE case.
if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
if (Result)
- *Result = E->EvaluateKnownConstInt(Context);
+ *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
return E;
}
Modified: cfe/trunk/test/Sema/integer-overflow.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/integer-overflow.c?rev=344759=344758=344759=diff
==
--- cfe/trunk/test/Sema/integer-overflow.c (original)
+++ cfe/trunk/test/Sema/integer-overflow.c Thu Oct 18 13:49:06 2018
@@ -172,6 +172,9 @@ void check_integer_overflows_in_function
// expected-warning@+1 {{overflow in expression; result is 536870912 with type
'int'}}
(void)f2(0, f0(4608 * 1024 * 1024));
}
+void check_integer_overflows_in_array_size() {
+ int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression;
result is 536870912 with type 'int'}}
+}
struct s {
unsigned x;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker
xbolva00 added inline comments. Comment at: lib/Driver/ToolChains/Gnu.cpp:241 + case llvm::Triple::thumbeb: +IsBigEndian = true; + case llvm::Triple::arm: Gnu.cpp:241:17: warning: this statement may fall through [-Wimplicit-fallthrough=] IsBigEndian = true; Repository: rC Clang https://reviews.llvm.org/D52784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r344758 - [clang-tidy] readability-uppercase-literal-suffix: specify target for fp tests
Author: lebedevri Date: Thu Oct 18 13:27:10 2018 New Revision: 344758 URL: http://llvm.org/viewvc/llvm-project?rev=344758=rev Log: [clang-tidy] readability-uppercase-literal-suffix: specify target for fp tests __float128 isn't universally avaliable. Modified: clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp Modified: clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp?rev=344758=344757=344758=diff == --- clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp Thu Oct 18 13:27:10 2018 @@ -1,7 +1,7 @@ -// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -target x86_64-pc-linux-gnu -I %S // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -target x86_64-pc-linux-gnu -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -target x86_64-pc-linux-gnu -I %S #include "readability-uppercase-literal-suffix.h" Modified: clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp?rev=344758=344757=344758=diff == --- clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp Thu Oct 18 13:27:10 2018 @@ -1,7 +1,7 @@ -// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S +// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -target x86_64-pc-linux-gnu -I %S // RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S -// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -target x86_64-pc-linux-gnu -I %S +// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -target x86_64-pc-linux-gnu -I %S #include "readability-uppercase-literal-suffix.h" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53410: Add missing PATH_MAX for GNU Hurd support
sylvestre.ledru created this revision.
sylvestre.ledru added a reviewer: rnk.
Herald added a subscriber: krytarowski.
If you have a better place to put them, don't hesitate to let me know :)
Patch by Svante Signell & myself
Repository:
rC Clang
https://reviews.llvm.org/D53410
Files:
lib/Basic/FileManager.cpp
lib/Frontend/ModuleDependencyCollector.cpp
Index: lib/Frontend/ModuleDependencyCollector.cpp
===
--- lib/Frontend/ModuleDependencyCollector.cpp
+++ lib/Frontend/ModuleDependencyCollector.cpp
@@ -99,6 +99,11 @@
}
+// For GNU Hurd
+#if defined(__GNU__) && !defined(PATH_MAX)
+# define PATH_MAX 4096
+#endif
+
// TODO: move this to Support/Path.h and check for HAVE_REALPATH?
static bool real_path(StringRef SrcPath, SmallVectorImpl ) {
#ifdef LLVM_ON_UNIX
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -511,6 +511,12 @@
UniqueRealFiles.erase(Entry->getUniqueID());
}
+// For GNU Hurd
+#if defined(__GNU__) && !defined(PATH_MAX)
+# define PATH_MAX 4096
+#endif
+
+
void FileManager::GetUniqueIDMapping(
SmallVectorImpl ) const {
UIDToFiles.clear();
Index: lib/Frontend/ModuleDependencyCollector.cpp
===
--- lib/Frontend/ModuleDependencyCollector.cpp
+++ lib/Frontend/ModuleDependencyCollector.cpp
@@ -99,6 +99,11 @@
}
+// For GNU Hurd
+#if defined(__GNU__) && !defined(PATH_MAX)
+# define PATH_MAX 4096
+#endif
+
// TODO: move this to Support/Path.h and check for HAVE_REALPATH?
static bool real_path(StringRef SrcPath, SmallVectorImpl ) {
#ifdef LLVM_ON_UNIX
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -511,6 +511,12 @@
UniqueRealFiles.erase(Entry->getUniqueID());
}
+// For GNU Hurd
+#if defined(__GNU__) && !defined(PATH_MAX)
+# define PATH_MAX 4096
+#endif
+
+
void FileManager::GetUniqueIDMapping(
SmallVectorImpl ) const {
UIDToFiles.clear();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52949: [Diagnostics] Implement -Wsizeof-pointer-div
xbolva00 added a comment.
Second thought, I don't think we should recommend std::size here (maybe it
should be okay for clang static analyzers)
uint32_t data[] = {10, 20, 30, 40};
len = sizeof(data)/sizeof(*data); // "warn" on valid code to recommend
std::size? I dont agree with such behaviour.
@rsmith?
https://reviews.llvm.org/D52949
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344757: [clang-tidy] Non-private member variables in classes
(MISRA, CppCoreGuidelines… (authored by lebedevri, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D52771?vs=170112=170114#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52771
Files:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
clang-tools-extra/trunk/test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Index: clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
+++ clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
@@ -0,0 +1,46 @@
+//===--- NonPrivateMemberVariablesInClassesCheck.h - clang-tidy -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONPRIVATEMEMBERVARIABLESINCLASSESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONPRIVATEMEMBERVARIABLESINCLASSESCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// This checker finds classes that not only contain the data
+/// (non-static member variables), but also have logic (non-static member
+/// functions), and diagnoses all member variables that have any other scope
+/// other than `private`. They should be made `private`, and manipulated
+/// exclusively via the member functions.
+///
+/// Optionally, classes with all member variables being `public` could be
+/// ignored and optionally all `public` member variables could be ignored.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-non-private-member-variables-in-classes.html
+class NonPrivateMemberVariablesInClassesCheck : public ClangTidyCheck {
+public:
+ NonPrivateMemberVariablesInClassesCheck(StringRef Name,
+ ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult ) override;
+
+private:
+ const bool IgnoreClassesWithAllMemberVariablesBeingPublic;
+ const bool IgnorePublicMemberVariables;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONPRIVATEMEMBERVARIABLESINCLASSESCHECK_H
Index: clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
@@ -0,0 +1,93 @@
+//===--- NonPrivateMemberVariablesInClassesCheck.cpp - clang-tidy -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "NonPrivateMemberVariablesInClassesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+AST_MATCHER(CXXRecordDecl, hasMethods) {
+ return std::distance(Node.method_begin(), Node.method_end()) != 0;
+}
+
+AST_MATCHER(CXXRecordDecl, hasNonStaticMethod) {
+ return hasMethod(unless(isStaticStorageClass()))
+ .matches(Node, Finder, Builder);
+}
+
+AST_MATCHER(CXXRecordDecl, hasNonPublicMemberVariable) {
+ return cxxRecordDecl(has(fieldDecl(unless(isPublic()
+ .matches(Node, Finder, Builder);
+}
+
+AST_POLYMORPHIC_MATCHER_P(boolean, AST_POLYMORPHIC_SUPPORTED_TYPES(Stmt, Decl),
+ bool, Boolean) {
+ return Boolean;
+}
+
+} // namespace
+
[clang-tools-extra] r344757 - [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
Author: lebedevri
Date: Thu Oct 18 13:16:44 2018
New Revision: 344757
URL: http://llvm.org/viewvc/llvm-project?rev=344757=rev
Log:
[clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines,
HICPP)
Summary:
Finds classes that not only contain the data (non-static member variables),
but also have logic (non-static member functions), and diagnoses all member
variables that have any other scope other than `private`. They should be
made `private`, and manipulated exclusively via the member functions.
Optionally, classes with all member variables being `public` could be
ignored, and optionally all `public` member variables could be ignored.
Options
---
* IgnoreClassesWithAllMemberVariablesBeingPublic
Allows to completely ignore classes if **all** the member variables in that
class have `public` visibility.
* IgnorePublicMemberVariables
Allows to ignore (not diagnose) **all** the member variables with `public`
visibility scope.
References:
* MISRA 11-0-1 Member data in non-POD class types shall be private.
*
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#c2-use-class-if-the-class-has-an-invariant-use-struct-if-the-data-members-can-vary-independently
*
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rc-private
*
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rh-protected
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, zinovy.nis, cfe-commits, rnkovacs, nemanjai,
mgorny, xazax.hun, kbarton
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52771
Added:
clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
clang-tools-extra/trunk/test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified:
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=344757=344756=344757=diff
==
---
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
(original)
+++
clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
Thu Oct 18 13:16:44 2018
@@ -10,6 +10,7 @@
#include "../ClangTidy.h"
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
+#include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
#include "../misc/UnconventionalAssignOperatorCheck.h"
#include "../readability/MagicNumbersCheck.h"
#include "AvoidGotoCheck.h"
@@ -47,6 +48,8 @@ public:
CheckFactories.registerCheck(
"cppcoreguidelines-narrowing-conversions");
CheckFactories.registerCheck("cppcoreguidelines-no-malloc");
+
CheckFactories.registerCheck(
+"cppcoreguidelines-non-private-member-variables-in-classes");
CheckFactories.registerCheck(
"cppcoreguidelines-owning-memory");
CheckFactories.registerCheck(
@@ -75,6 +78,16 @@ public:
CheckFactories.registerCheck(
"cppcoreguidelines-c-copy-assignment-signature");
}
+
+ ClangTidyOptions getModuleOptions() override {
+ClangTidyOptions Options;
+ClangTidyOptions::OptionMap = Options.CheckOptions;
+
+Opts["cppcoreguidelines-non-private-member-variables-in-classes."
+ "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "1";
+
+return Options;
+ }
};
// Register the LLVMTidyModule using this statically initialized variable.
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=344757=344756=344757=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Thu Oct 18 13:16:44
2018
@@ -6,6 +6,7 @@ add_clang_library(clangTidyMiscModule
MisplacedConstCheck.cpp
NewDeleteOverloadsCheck.cpp
NonCopyableObjects.cpp
+ NonPrivateMemberVariablesInClassesCheck.cpp
RedundantExpressionCheck.cpp
StaticAssertCheck.cpp
[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions
xbolva00 added a comment. Is PrecisionConversion acceptable? If not, any suggestions for name? https://reviews.llvm.org/D52835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
lebedev.ri updated this revision to Diff 170112.
lebedev.ri added a comment.
Rebased to fix a merge conflict in release notes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52771
Files:
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -0,0 +1,380 @@
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s cppcoreguidelines-non-private-member-variables-in-classes %t -- --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+
+////
+
+// Only data, do not warn
+
+struct S0 {
+ int S0_v0;
+
+public:
+ int S0_v1;
+
+protected:
+ int S0_v2;
+
+private:
+ int S0_v3;
+};
+
+class S1 {
+ int S1_v0;
+
+public:
+ int S1_v1;
+
+protected:
+ int S1_v2;
+
+private:
+ int S1_v3;
+};
+
+////
+
+// All functions are static, do not warn.
+
+struct S2 {
+ static void S2_m0();
+ int S2_v0;
+
+public:
+ static void S2_m1();
+ int S2_v1;
+
+protected:
+ static void S2_m3();
+ int S2_v2;
+
+private:
+ static void S2_m4();
+ int S2_v3;
+};
+
+class S3 {
+ static void S3_m0();
+ int S3_v0;
+
+public:
+ static void S3_m1();
+ int S3_v1;
+
+protected:
+ static void S3_m3();
+ int S3_v2;
+
+private:
+ static void S3_m4();
+ int S3_v3;
+};
+
+////
+
+// union != struct/class. do not diagnose.
+
+union U0 {
+ void U0_m0();
+ int U0_v0;
+
+public:
+ void U0_m1();
+ int U0_v1;
+
+protected:
+ void U0_m2();
+ int U0_v2;
+
+private:
+ void U0_m3();
+ int U0_v3;
+};
+
+////
+
+// Has non-static method with default visibility.
+
+struct S4 {
+ void S4_m0();
+
+ int S4_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v0' has public visibility
+public:
+ int S4_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v1' has public visibility
+protected:
+ int S4_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S4_v2' has protected visibility
+private:
+ int S4_v3;
+};
+
+class S5 {
+ void S5_m0();
+
+ int S5_v0;
+
+public:
+ int S5_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S5_v1' has public visibility
+protected:
+ int S5_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S5_v2' has protected visibility
+private:
+ int S5_v3;
+};
+
+////
+
+// Has non-static method with public visibility.
+
+struct S6 {
+ int S6_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S6_v0' has public visibility
+public:
+ void
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344755: [clang-tidy] Add new
readability-uppercase-literal-suffix check (CERT DCL16-C… (authored
by lebedevri, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D52670?vs=170108=170111#toc
Repository:
rL LLVM
https://reviews.llvm.org/D52670
Files:
clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl16-c.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
clang-tools-extra/trunk/test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix.h
Index: clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
@@ -35,6 +35,7 @@
#include "../readability/BracesAroundStatementsCheck.h"
#include "../readability/FunctionSizeCheck.h"
#include "../readability/IdentifierNamingCheck.h"
+#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "ExceptionBaseclassCheck.h"
#include "MultiwayPathsCoveredCheck.h"
#include "NoAssemblerCheck.h"
@@ -100,6 +101,8 @@
"hicpp-use-nullptr");
CheckFactories.registerCheck(
"hicpp-use-override");
+CheckFactories.registerCheck(
+"hicpp-uppercase-literal-suffix");
CheckFactories.registerCheck(
"hicpp-vararg");
}
Index: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
===
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
@@ -27,6 +27,18 @@
bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager ,
const LangOptions ,
StringRef FlagName);
+
+// Check if the range is entirely contained within a macro argument.
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+const SourceManager *SM);
+
+// Check if the range contains any locations from a macro expansion.
+bool rangeContainsMacroExpansion(SourceRange Range, const SourceManager *SM);
+
+// Can a fix-it be issued for this whole Range?
+// FIXME: false-negative if the entire range is fully expanded from a macro.
+bool rangeCanBeFixed(SourceRange Range, const SourceManager *SM);
+
} // namespace utils
} // namespace tidy
} // namespace clang
Index: clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
===
--- clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
+++ clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
@@ -67,6 +67,32 @@
return true;
}
+bool rangeIsEntirelyWithinMacroArgument(SourceRange Range,
+const SourceManager *SM) {
+ // Check if the range is entirely contained within a macro argument.
+ SourceLocation MacroArgExpansionStartForRangeBegin;
+ SourceLocation MacroArgExpansionStartForRangeEnd;
+ bool RangeIsEntirelyWithinMacroArgument =
+ SM &&
+ SM->isMacroArgExpansion(Range.getBegin(),
+ ) &&
+ SM->isMacroArgExpansion(Range.getEnd(),
+ ) &&
+
[clang-tools-extra] r344755 - [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
Author: lebedevri
Date: Thu Oct 18 13:06:40 2018
New Revision: 344755
URL: http://llvm.org/viewvc/llvm-project?rev=344755=rev
Log:
[clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT
DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
Summary:
Detects when the integral literal or floating point (decimal or hexadecimal)
literal has non-uppercase suffix, and suggests to make the suffix uppercase,
with fix-it.
All valid combinations of suffixes are supported.
```
auto x = 1; // OK, no suffix.
auto x = 1u; // warning: integer literal suffix 'u' is not upper-case
auto x = 1U; // OK, suffix is uppercase.
...
```
References:
* [[ https://wiki.sei.cmu.edu/confluence/pages/viewpage.action?pageId=87152241
| CERT DCL16-C ]]
* MISRA C:2012, 7.3 - The lowercase character "l" shall not be used in a
literal suffix
* MISRA C++:2008, 2-13-4 - Literal suffixes shall be upper case
Reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun
Reviewed By: aaron.ballman
Subscribers: Eugene.Zelenko, mgorny, rnkovacs, cfe-commits
Tags: #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D52670
Added:
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/UppercaseLiteralSuffixCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/cert-dcl16-c.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
clang-tools-extra/trunk/test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-uppercase-literal-suffix.h
Modified:
clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.cpp
clang-tools-extra/trunk/clang-tidy/utils/ASTUtils.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp?rev=344755=344754=344755=diff
==
--- clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CERTTidyModule.cpp Thu Oct 18
13:06:40 2018
@@ -16,6 +16,7 @@
#include "../misc/StaticAssertCheck.h"
#include "../misc/ThrowByValueCatchByReferenceCheck.h"
#include "../performance/MoveConstructorInitCheck.h"
+#include "../readability/UppercaseLiteralSuffixCheck.h"
#include "CommandProcessorCheck.h"
#include "DontModifyStdNamespaceCheck.h"
#include "FloatLoopCounter.h"
@@ -65,6 +66,8 @@ public:
// C checkers
// DCL
CheckFactories.registerCheck("cert-dcl03-c");
+CheckFactories.registerCheck(
+"cert-dcl16-c");
// ENV
CheckFactories.registerCheck("cert-env33-c");
// FLP
@@ -78,6 +81,13 @@ public:
CheckFactories.registerCheck(
"cert-msc32-c");
}
+
+ ClangTidyOptions getModuleOptions() override {
+ClangTidyOptions Options;
+ClangTidyOptions::OptionMap = Options.CheckOptions;
+Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU";
+return Options;
+ }
};
} // namespace cert
Modified: clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt?rev=344755=344754=344755=diff
==
--- clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/cert/CMakeLists.txt Thu Oct 18 13:06:40
2018
@@ -24,5 +24,6 @@ add_clang_library(clangTidyCERTModule
clangTidyGoogleModule
[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. We can leave the cleanup of the expression evaluator to another change. https://reviews.llvm.org/D52750 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Thanks for the changes! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
lebedev.ri updated this revision to Diff 170109.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.
Don't use `auto` in `getModuleOptions()`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52771
Files:
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -0,0 +1,380 @@
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s cppcoreguidelines-non-private-member-variables-in-classes %t -- --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+
+////
+
+// Only data, do not warn
+
+struct S0 {
+ int S0_v0;
+
+public:
+ int S0_v1;
+
+protected:
+ int S0_v2;
+
+private:
+ int S0_v3;
+};
+
+class S1 {
+ int S1_v0;
+
+public:
+ int S1_v1;
+
+protected:
+ int S1_v2;
+
+private:
+ int S1_v3;
+};
+
+////
+
+// All functions are static, do not warn.
+
+struct S2 {
+ static void S2_m0();
+ int S2_v0;
+
+public:
+ static void S2_m1();
+ int S2_v1;
+
+protected:
+ static void S2_m3();
+ int S2_v2;
+
+private:
+ static void S2_m4();
+ int S2_v3;
+};
+
+class S3 {
+ static void S3_m0();
+ int S3_v0;
+
+public:
+ static void S3_m1();
+ int S3_v1;
+
+protected:
+ static void S3_m3();
+ int S3_v2;
+
+private:
+ static void S3_m4();
+ int S3_v3;
+};
+
+////
+
+// union != struct/class. do not diagnose.
+
+union U0 {
+ void U0_m0();
+ int U0_v0;
+
+public:
+ void U0_m1();
+ int U0_v1;
+
+protected:
+ void U0_m2();
+ int U0_v2;
+
+private:
+ void U0_m3();
+ int U0_v3;
+};
+
+////
+
+// Has non-static method with default visibility.
+
+struct S4 {
+ void S4_m0();
+
+ int S4_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v0' has public visibility
+public:
+ int S4_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v1' has public visibility
+protected:
+ int S4_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S4_v2' has protected visibility
+private:
+ int S4_v3;
+};
+
+class S5 {
+ void S5_m0();
+
+ int S5_v0;
+
+public:
+ int S5_v1;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S5_v1' has public visibility
+protected:
+ int S5_v2;
+ // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S5_v2' has protected visibility
+private:
+ int S5_v3;
+};
+
+////
+
+// Has non-static method with public visibility.
+
+struct S6 {
+ int S6_v0;
+ // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S6_v0' has
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri updated this revision to Diff 170108.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.
CERT: also handle "lu", "llu".
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52670
Files:
clang-tidy/cert/CERTTidyModule.cpp
clang-tidy/cert/CMakeLists.txt
clang-tidy/hicpp/HICPPTidyModule.cpp
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tidy/readability/UppercaseLiteralSuffixCheck.h
clang-tidy/utils/ASTUtils.cpp
clang-tidy/utils/ASTUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cert-dcl16-c.rst
docs/clang-tidy/checks/hicpp-uppercase-literal-suffix.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst
test/clang-tidy/cert-uppercase-literal-suffix-integer.cpp
test/clang-tidy/readability-uppercase-literal-suffix-floating-point-opencl-half.cpp
test/clang-tidy/readability-uppercase-literal-suffix-floating-point.cpp
test/clang-tidy/readability-uppercase-literal-suffix-hexadecimal-floating-point.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-custom-list.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-macro.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer-ms.cpp
test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
test/clang-tidy/readability-uppercase-literal-suffix.h
Index: test/clang-tidy/readability-uppercase-literal-suffix.h
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix.h
@@ -0,0 +1,16 @@
+template
+struct integral_constant {
+ static constexpr T value = v;
+ typedef T value_type;
+ typedef integral_constant type; // using injected-class-name
+ constexpr operator value_type() const noexcept { return value; }
+};
+
+using false_type = integral_constant;
+using true_type = integral_constant;
+
+template
+struct is_same : false_type {};
+
+template
+struct is_same : true_type {};
Index: test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-uppercase-literal-suffix-integer.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -I %S
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -fix -- -I %S
+// RUN: clang-tidy %t.cpp -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -I %S
+
+#include "readability-uppercase-literal-suffix.h"
+
+void integer_suffix() {
+ static constexpr auto v0 = __LINE__; // synthetic
+ static_assert(v0 == 9 || v0 == 5, "");
+
+ static constexpr auto v1 = __cplusplus; // synthetic, long
+
+ static constexpr auto v2 = 1; // no literal
+ static_assert(is_same::value, "");
+ static_assert(v2 == 1, "");
+
+ // Unsigned
+
+ static constexpr auto v3 = 1u;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'u', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v3 = 1u;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}}
+ // CHECK-FIXES: static constexpr auto v3 = 1U;
+ static_assert(is_same::value, "");
+ static_assert(v3 == 1, "");
+
+ static constexpr auto v4 = 1U; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v4 == 1, "");
+
+ // Long
+
+ static constexpr auto v5 = 1l;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'l', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v5 = 1l;
+ // CHECK-MESSAGES-NEXT: ^~
+ // CHECK-MESSAGES-NEXT: {{^ *}}L{{$}}
+ // CHECK-FIXES: static constexpr auto v5 = 1L;
+ static_assert(is_same::value, "");
+ static_assert(v5 == 1, "");
+
+ static constexpr auto v6 = 1L; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v6 == 1, "");
+
+ // Long Long
+
+ static constexpr auto v7 = 1ll;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'll', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v7 = 1ll;
+ // CHECK-MESSAGES-NEXT: ^~~
+ // CHECK-MESSAGES-NEXT: {{^ *}}LL{{$}}
+ // CHECK-FIXES: static constexpr auto v7 = 1LL;
+ static_assert(is_same::value, "");
+ static_assert(v7 == 1, "");
+
+ static constexpr auto v8 = 1LL; // OK.
+ static_assert(is_same::value, "");
+ static_assert(v8 == 1, "");
+
+ // Unsigned Long
+
+ static constexpr auto v9 = 1ul;
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'ul', which is not uppercase
+ // CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1ul;
+ // CHECK-MESSAGES-NEXT: ^~~
+ // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}}
+ // CHECK-FIXES:
[PATCH] D53308: [Fixed Point Arithmetic] Fixed Point to Boolean Cast
rjmccall added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9599 + return false; +return Success(Val.getInt().getBoolValue(), E); + } I know you haven't really done constant-evaluation yet, but I think you should at least be setting up operations like this correctly: 1. There should be an `EvaluateFixedPoint` function that evaluates an expression as a fixed-point value, just like `EvaluateFloat` and the others, which can be structured to use `FixedPointExprEvaluator`. 2. There should be a `getBoolValue` method on `APFixedPoint`. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2026 +return EmitScalarConversion(Visit(E), E->getType(), DestTy, +CE->getExprLoc()); Why are you pushing these casts through `EmitScalarConversion` when the cast kind already tells you which operation you're doing? Repository: rC Clang https://reviews.llvm.org/D53308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
aaron.ballman added a comment. Hmm, the latest patch only seems to have the changes to the test but not the implementation? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268572, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268569, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > > > > > I talked to someone at CERT responsible for maintaining DCL16-C to get > > > their opinion on tightening the wording of the rule and their stated > > > intent is: > > > > > > Thank you! > > > > > "If the first character is 'ell', it should be capitalized. The other > > > ells need not be, and the yew's need not be capitalized either." > > > > > > e.g., > > > 11lu -> diagnose > > > 11ul -> fine > > > 11llu -> diagnose > > > 11lLu -> diagnose > > > 11Llu -> fine > > > 11ul -> fine > > > > > > That said, the author (and I) agree that it'd be perfectly okay to > > > diagnose things like `11Llu` and recommend `11LLU` as a replacement. > > > > Ok, nothing unexpected. > > So the full revised list is: "L;LL:LU;LLU". > > > Agreed. It might be reasonable to add the above as some extra test cases for > the CERT check if they're not already covered elsewhere. Those exist as the test cases for CERT (although, i only test the integer case for CERT, not float.), i will only need to adjust the expected output. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268569, @lebedev.ri wrote: > In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > > > I talked to someone at CERT responsible for maintaining DCL16-C to get > > their opinion on tightening the wording of the rule and their stated intent > > is: > > > Thank you! > > > "If the first character is 'ell', it should be capitalized. The other ells > > need not be, and the yew's need not be capitalized either." > > > > e.g., > > 11lu -> diagnose > > 11ul -> fine > > 11llu -> diagnose > > 11lLu -> diagnose > > 11Llu -> fine > > 11ul -> fine > > > > That said, the author (and I) agree that it'd be perfectly okay to diagnose > > things like `11Llu` and recommend `11LLU` as a replacement. > > Ok, nothing unexpected. > So the full revised list is: "L;LL:LU;LLU". Agreed. It might be reasonable to add the above as some extra test cases for the CERT check if they're not already covered elsewhere. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268564, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268372, @lebedev.ri wrote: > > > In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > > > > > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > > > > > > > - Apply minor wording nits. > > > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not > > > > **anything** else (not even `llu`). > > > > > > > > > I'll find out about the DCL16-C recommendation, as I suspect the intent > > > is to cover `lu` and `llu` but not `ul` and `ull`. > > > > > > I agree, i've thought so too. > > > > That will open an interesting question: in `lu`, `l` should be upper-case. > > What about `u`? We can't keep it as-is. > > We will either consistently upper-case it, or consistently lower-case it. > > I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`? > > > I talked to someone at CERT responsible for maintaining DCL16-C to get their > opinion on tightening the wording of the rule and their stated intent is: Thank you! > "If the first character is 'ell', it should be capitalized. The other ells > need not be, and the yew's need not be capitalized either." > > e.g., > 11lu -> diagnose > 11ul -> fine > 11llu -> diagnose > 11lLu -> diagnose > 11Llu -> fine > 11ul -> fine > > That said, the author (and I) agree that it'd be perfectly okay to diagnose > things like `11Llu` and recommend `11LLU` as a replacement. Ok, nothing unexpected. So the full revised list is: "L;LL:LU;LLU". Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
arphaman added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
arphaman wrote:
> arphaman wrote:
> > ddunbar wrote:
> > > What is our feeling w.r.t. _Pragma, which can in theory influence the
> > > preprocessor. I'm not sure this model can sanely support it?
> > We need to look into that. In the worst case we can always avoid minimizing
> > the file if it has a _Pragma anywhere in it.
> We also have to handle cases like this one:
>
> foo.h:
> ```
> #define PP _Pragma
> ```
>
> bar.h:
> ```
> PP ("foo")
> ```
>
> Unfortunately this can't be handled by just disabling minimization for
> `foo.h` as PP will be stripped out from `bar.h`. Furthermore, this pattern is
> quite reasonable, so we should expect it in the code we receive. Right now I
> can't think of a reasonable solution for this problem.
>
> There's also a more "evil" case:
>
> ```
> #define P(A, B) A##B
>
> P(_Pra,gma)("foo")
> ```
>
> It would be reasonable to introduce a warning for the use of `_Pragma` token
> that was created using PP token concatenation and just hope that our users
> won't use this pattern.
One possible solution to the first issue is to simply fallback to the regular
-Eonly invocation if we detect an include of a file that has a macro with a
`_Pragma` in it. Then we could emit a remark to the user saying that their
build could be faster if they rewrite their code so that this pattern no longer
occurs.
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268372, @lebedev.ri wrote: > In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > > > > > - Apply minor wording nits. > > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not > > > **anything** else (not even `llu`). > > > > > > I'll find out about the DCL16-C recommendation, as I suspect the intent is > > to cover `lu` and `llu` but not `ul` and `ull`. > > > I agree, i've thought so too. > > That will open an interesting question: in `lu`, `l` should be upper-case. > What about `u`? We can't keep it as-is. > We will either consistently upper-case it, or consistently lower-case it. > I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`? I talked to someone at CERT responsible for maintaining DCL16-C to get their opinion on tightening the wording of the rule and their stated intent is: "If the first character is 'ell', it should be capitalized. The other ells need not be, and the yew's need not be capitalized either." e.g., 11lu -> diagnose 11ul -> fine 11llu -> diagnose 11lLu -> diagnose 11Llu -> fine 11ul -> fine That said, the author (and I) agree that it'd be perfectly okay to diagnose things like `11Llu` and recommend `11LLU` as a replacement. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
arphaman added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
arphaman wrote:
> ddunbar wrote:
> > What is our feeling w.r.t. _Pragma, which can in theory influence the
> > preprocessor. I'm not sure this model can sanely support it?
> We need to look into that. In the worst case we can always avoid minimizing
> the file if it has a _Pragma anywhere in it.
We also have to handle cases like this one:
foo.h:
```
#define PP _Pragma
```
bar.h:
```
PP ("foo")
```
Unfortunately this can't be handled by just disabling minimization for `foo.h`
as PP will be stripped out from `bar.h`. Furthermore, this pattern is quite
reasonable, so we should expect it in the code we receive. Right now I can't
think of a reasonable solution for this problem.
There's also a more "evil" case:
```
#define P(A, B) A##B
P(_Pra,gma)("foo")
```
It would be reasonable to introduce a warning for the use of `_Pragma` token
that was created using PP token concatenation and just hope that our users
won't use this pattern.
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53406: [clangd] Provide excuses for bad code completions, based on diagnostics. C++ API only.
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric.
In some circumstances we provide bad completions or no completions, because of
problems in the code. This can be puzzling and opaque to users.
If we can tell the user that this is the case and why, they'll be happy.
Experiments with go language servers have shown this to be a big win.
Two major problems:
- Sema doesn't provide the information we need
- LSP provides no protocol mechanims, and editors don't have UI for this
This patch attempts to guess the information, looking at diagnostics on the
line.
Other heuristics are possible (e.g. using completion context). It's unclear how
useful or successful they will be. This is mostly a quick hack to test
viability.
This is exposed as an extension of the C++ API only (not bound to LSP).
The idea is to test viability with a non-public editor that happens to have the
right UI already (and doesn't use LSP), and experiment with approaches.
If something fairly simple works well, we'll keep this and expose an LSP
extension.
If it's not useful, we should drop this idea.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53406
Files:
clangd/ClangdLSPServer.cpp
clangd/CodeComplete.cpp
clangd/CodeComplete.h
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2176,6 +2176,50 @@
AllOf(Qualifier("nx::"), Named("Clangd2";
}
+TEST(CompletionTest, Excuses) {
+ struct {
+const char *Code;
+const char *Excuse;
+ } Tests[] = {
+ {
+ R"cpp(
+class X;
+X* x;
+int main() {
+ x->^
+}
+ )cpp",
+ "member access into incomplete type 'X'",
+ },
+ {
+ R"cpp(
+// Error is on same line.
+template struct X;
+X x; ^
+ )cpp",
+ "implicit instantiation of undefined template 'X'",
+ },
+ {
+ R"cpp(
+// Error is on previous line.
+template struct X;
+X x;
+^
+ )cpp",
+ "",
+ },
+ {
+ R"cpp(
+// Just a warning (declaration does not declare anything).
+int; x^
+ )cpp",
+ "",
+ },
+ };
+ for (const auto : Tests)
+EXPECT_EQ(completions(Test.Code).Excuse, Test.Excuse) << Test.Code;
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/CodeComplete.h
===
--- clangd/CodeComplete.h
+++ clangd/CodeComplete.h
@@ -196,6 +196,10 @@
std::vector Completions;
bool HasMore = false;
CodeCompletionContext::Kind Context = CodeCompletionContext::CCC_Other;
+
+ // Our best guess at why completions might be poor (human-readable string).
+ // Only set if we suspect completions are poor *and* we know why.
+ std::string Excuse;
};
raw_ostream <<(raw_ostream &, const CodeCompleteResult &);
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -671,6 +671,42 @@
return false;
}
+// Tries to come up with a convincing excuse for our bad completion results,
+// based on the diagnostics near the completion point.
+class ExcuseMaker : public DiagnosticConsumer {
+ unsigned StartOfLine, Offset;
+ std::string
+ SmallString<256> ExcuseBuf;
+
+ public:
+ ExcuseMaker(StringRef Code, unsigned Offset, std::string )
+ : Offset(Offset), Excuse(Excuse) {
+ assert(Offset <= Code.size());
+ for (StartOfLine = Offset;
+ StartOfLine > 0 && Code[StartOfLine - 1] != '\n'; --StartOfLine)
+ ;
+ }
+
+ void HandleDiagnostic(DiagnosticsEngine::Level Level,
+ const clang::Diagnostic ) override {
+ // We accept >= error diagnostics, before the cursor but on the same line.
+ if (Level < DiagnosticsEngine::Error || !Diag.hasSourceManager())
+ return;
+ auto& SM = Diag.getSourceManager();
+ auto Loc = SM.getDecomposedLoc(Diag.getLocation());
+ if (Loc.first != SM.getMainFileID())
+ return;
+ unsigned DiagLoc = Loc.second;
+ if (DiagLoc < StartOfLine || DiagLoc > Offset)
+ return;
+ ExcuseBuf.clear();
+ Diag.FormatDiagnostic(ExcuseBuf);
+ }
+
+ ~ExcuseMaker() { Excuse = ExcuseBuf.str(); }
+};
+
+
// 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
r344750 - Add check-clang-python to the Clang tests directory in IDEs; NFC.
Author: aaronballman
Date: Thu Oct 18 10:47:18 2018
New Revision: 344750
URL: http://llvm.org/viewvc/llvm-project?rev=344750=rev
Log:
Add check-clang-python to the Clang tests directory in IDEs; NFC.
Modified:
cfe/trunk/bindings/python/tests/CMakeLists.txt
Modified: cfe/trunk/bindings/python/tests/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/bindings/python/tests/CMakeLists.txt?rev=344750=344749=344750=diff
==
--- cfe/trunk/bindings/python/tests/CMakeLists.txt (original)
+++ cfe/trunk/bindings/python/tests/CMakeLists.txt Thu Oct 18 10:47:18 2018
@@ -8,6 +8,7 @@ add_custom_target(check-clang-python
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/..)
set(RUN_PYTHON_TESTS TRUE)
+set_target_properties(check-clang-python PROPERTIES FOLDER "Clang tests")
# Do not try to run if libclang was built with ASan because
# the sanitizer library will likely be loaded too late to perform
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344749 - Add language standard aliases for -std=c18, -std=gnu18, and -std=iso9899:2018.
Author: aaronballman
Date: Thu Oct 18 10:42:41 2018
New Revision: 344749
URL: http://llvm.org/viewvc/llvm-project?rev=344749=rev
Log:
Add language standard aliases for -std=c18, -std=gnu18, and -std=iso9899:2018.
As described in D40225, the C17 standard was balloted and approved in 2017, but
the ISO publication process delayed the actual publication until 2018. WG14
considers the release to be C17 and describes it as such, but users can still
be confused by the publication year which is why -std=c18 adds value. These
aliases map to c17 and are all supported by GCC 8.x with the same behavior.
Note that the value of __STDC_VERSION__ remains at 201710L.
Modified:
cfe/trunk/include/clang/Frontend/LangStandards.def
cfe/trunk/test/Driver/unknown-std.c
cfe/trunk/test/Preprocessor/c17.c
Modified: cfe/trunk/include/clang/Frontend/LangStandards.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/LangStandards.def?rev=344749=344748=344749=diff
==
--- cfe/trunk/include/clang/Frontend/LangStandards.def (original)
+++ cfe/trunk/include/clang/Frontend/LangStandards.def Thu Oct 18 10:42:41 2018
@@ -82,9 +82,12 @@ LANGSTANDARD(c17, "c17",
C, "ISO C 2017",
LineComment | C99 | C11 | C17 | Digraphs | HexFloat)
LANGSTANDARD_ALIAS(c17, "iso9899:2017")
+LANGSTANDARD_ALIAS(c17, "c18")
+LANGSTANDARD_ALIAS(c17, "iso9899:2018")
LANGSTANDARD(gnu17, "gnu17",
C, "ISO C 2017 with GNU extensions",
LineComment | C99 | C11 | C17 | Digraphs | GNUMode | HexFloat)
+LANGSTANDARD_ALIAS(gnu17, "gnu18")
// C++ modes
LANGSTANDARD(cxx98, "c++98",
Modified: cfe/trunk/test/Driver/unknown-std.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/unknown-std.c?rev=344749=344748=344749=diff
==
--- cfe/trunk/test/Driver/unknown-std.c (original)
+++ cfe/trunk/test/Driver/unknown-std.c Thu Oct 18 10:42:41 2018
@@ -14,8 +14,8 @@
// CHECK-NEXT: note: use 'gnu99' for 'ISO C 1999 with GNU extensions' standard
// CHECK-NEXT: note: use 'c11' or 'iso9899:2011' for 'ISO C 2011' standard
// CHECK-NEXT: note: use 'gnu11' for 'ISO C 2011 with GNU extensions' standard
-// CHECK-NEXT: note: use 'c17' or 'iso9899:2017' for 'ISO C 2017' standard
-// CHECK-NEXT: note: use 'gnu17' for 'ISO C 2017 with GNU extensions' standard
+// CHECK-NEXT: note: use 'c17', 'iso9899:2017', 'c18', or 'iso9899:2018' for
'ISO C 2017' standard
+// CHECK-NEXT: note: use 'gnu17' or 'gnu18' for 'ISO C 2017 with GNU
extensions' standard
// Make sure that no other output is present.
// CHECK-NOT: {{^.+$}}
Modified: cfe/trunk/test/Preprocessor/c17.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/c17.c?rev=344749=344748=344749=diff
==
--- cfe/trunk/test/Preprocessor/c17.c (original)
+++ cfe/trunk/test/Preprocessor/c17.c Thu Oct 18 10:42:41 2018
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c17 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c18 %s
// expected-no-diagnostics
_Static_assert(__STDC_VERSION__ == 201710L, "Incorrect __STDC_VERSION__");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
arphaman added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
ddunbar wrote:
> What is our feeling w.r.t. _Pragma, which can in theory influence the
> preprocessor. I'm not sure this model can sanely support it?
We need to look into that. In the worst case we can always avoid minimizing the
file if it has a _Pragma anywhere in it.
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53207: Fix bug 26547 - alignof should return ABI alignment, not preferred alignment
ubsan added a comment. I'm having real difficulty with this - I get really odd errors in seemingly unrelated tests when I change things. Repository: rC Clang https://reviews.llvm.org/D53207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype
ddunbar added inline comments.
Comment at: lib/Lex/FilterToIncludes.cpp:628
+ First = Id.Last;
+ auto Kind = llvm::StringSwitch(Id.Name)
+ .Case("include", pp_include)
What is our feeling w.r.t. _Pragma, which can in theory influence the
preprocessor. I'm not sure this model can sanely support it?
Repository:
rC Clang
https://reviews.llvm.org/D53354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53392: [RISCV] Collect library directories and triples for riscv64 triple too
jrtc27 added inline comments. Comment at: test/Driver/riscv64-toolchain.c:71 +// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \ +// RUN: -target riscv64-linux-unknown-elf \ +// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \ This (and below) are the only instances of `CPU-linux-unknown-elf` in the whole of clang (and there are only two instances of `CPU-linux-elf`, both in `test/Modules/pch_container.m`), apart from the riscv32 ones they were copied from. Perhaps they should be `riscv64-linux-unknown-gnu` instead? Notably, when parsed as `CPU-company-kernel-OS` this in fact sets *company* to linux and *kernel* to unknown, whereas if you really mean the full 4-tuple should the linux and unknown not be interchanged? Comment at: test/Driver/riscv64-toolchain.c:87 +// RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \ +// RUN: -target riscv64-linux-unknown-elf -march=rv64imafd -mabi=lp64d \ +// RUN: --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \ `riscv64-linux-unknown-gnu` as before. Repository: rC Clang https://reviews.llvm.org/D53392 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53392: [RISCV] Collect library directories and triples for riscv64 triple too
jrtc27 added inline comments.
Comment at: lib/Driver/ToolChains/Gnu.cpp:1912
- static const char *const RISCV32LibDirs[] = {"/lib", "/lib32"};
+ static const char *const RISCVLibDirs[] = {"/lib", "/lib32"};
static const char *const RISCVTriples[] = {"riscv32-unknown-linux-gnu",
Surely this should remain as `RISCV32LibDirs`? Also, should we reverse the
order (put `"/lib32"` first) to match every single other architecture?
Comment at: lib/Driver/ToolChains/Gnu.cpp:1913
+ static const char *const RISCVLibDirs[] = {"/lib", "/lib32"};
static const char *const RISCVTriples[] = {"riscv32-unknown-linux-gnu",
"riscv32-unknown-elf"};
`RISCV32Triples`? Also, why do we have no `"riscv32-linux-gnu"` entry like the
other architectures?
Comment at: lib/Driver/ToolChains/Gnu.cpp:1915
"riscv32-unknown-elf"};
+ static const char *const RISCV64LibDirs[] = {"/lib", "/lib64"};
+ static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
Order as for 32.
Comment at: lib/Driver/ToolChains/Gnu.cpp:1916
+ static const char *const RISCV64LibDirs[] = {"/lib", "/lib64"};
+ static const char *const RISCV64Triples[] = {"riscv64-unknown-linux-gnu",
+ "riscv64-unknown-elf"};
`riscv64-linux-gnu`?
Repository:
rC Clang
https://reviews.llvm.org/D53392
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
curdeius updated this revision to Diff 170101. curdeius added a comment. Applied changes as per comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 Files: test/clang-tidy/readability-else-after-return-if-constexpr.cpp Index: test/clang-tidy/readability-else-after-return-if-constexpr.cpp === --- test/clang-tidy/readability-else-after-return-if-constexpr.cpp +++ test/clang-tidy/readability-else-after-return-if-constexpr.cpp @@ -6,7 +6,7 @@ return; else return; - // CHECK-MESSAGES: [[@LINE-2]]:3: warning: + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return' if constexpr (sizeof(int) > 4) return; @@ -20,4 +20,3 @@ else return; } -// CHECK-NOT: warning: Index: test/clang-tidy/readability-else-after-return-if-constexpr.cpp === --- test/clang-tidy/readability-else-after-return-if-constexpr.cpp +++ test/clang-tidy/readability-else-after-return-if-constexpr.cpp @@ -6,7 +6,7 @@ return; else return; - // CHECK-MESSAGES: [[@LINE-2]]:3: warning: + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: do not use 'else' after 'return' if constexpr (sizeof(int) > 4) return; @@ -20,4 +20,3 @@ else return; } -// CHECK-NOT: warning: ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52742: [analyzer][PlistMacroExpansion] Part 1.: New expand-macros flag
Szelethus marked 3 inline comments as done.
Szelethus added inline comments.
Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:469
+DisplayMacroExpansions =
+getBooleanOption("expand-macros", /*Default=*/false);
+ return DisplayMacroExpansions.getValue();
NoQ wrote:
> Should we say something about plists in the option name?
Sure, but should my `AnalyzerOptions` refactoring effort go through first, I
intend emit warnings if flags aren't set correctly (eg. the output isn't set to
plist but this flag is enabled). Should probably rename `"serialize-stats"` too
then.
http://lists.llvm.org/pipermail/cfe-dev/2018-October/059842.html
Comment at:
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist:44-54
+
+ kindmacro_expansion
+ location
+
+ line26
+ col3
+ file0
NoQ wrote:
> Because we're adding an element of an `` rather than a key of a
> ``, I'm not entirely sure this is backwards compatible. Clients may
> crash if they iterate over the `path` array and encounter an unexpected
> element kind. Is it going to be bad for your use case if we put expansions
> into a separate array alongside the `path` array?
It shouldn't be :)
https://reviews.llvm.org/D52742
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53404: [clangd] Set workspace root when initializing ClangdServer, disallow mutation.
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Rename instance variable to WorkspaceRoot to match what we call it internally.
Add fixme to set it automatically. Don't do it yet, clients have assumptions
that the constructor won't access the FS.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53404
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/FindSymbolsTests.cpp
Index: unittests/clangd/FindSymbolsTests.cpp
===
--- unittests/clangd/FindSymbolsTests.cpp
+++ unittests/clangd/FindSymbolsTests.cpp
@@ -42,6 +42,7 @@
ClangdServer::Options optsForTests() {
auto ServerOpts = ClangdServer::optsForTest();
+ ServerOpts.WorkspaceRoot = testRoot();
ServerOpts.BuildDynamicSymbolIndex = true;
ServerOpts.URISchemes = {"unittest", "file"};
return ServerOpts;
@@ -53,7 +54,6 @@
: Server(CDB, FSProvider, DiagConsumer, optsForTests()) {
// Make sure the test root directory is created.
FSProvider.Files[testPath("unused")] = "";
-Server.setRootPath(testRoot());
}
protected:
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -86,6 +86,11 @@
/// If set, use this index to augment code completion results.
SymbolIndex *StaticIndex = nullptr;
+/// Clangd's workspace root. Relevant for "workspace" operations not bound
+/// to a particular file.
+/// FIXME: If not set, should use the current working directory.
+llvm::Optional WorkspaceRoot;
+
/// The resource directory is used to find internal headers, overriding
/// defaults and -resource-dir compiler flag).
/// If None, ClangdServer calls CompilerInvocation::GetResourcePath() to
@@ -116,9 +121,6 @@
const FileSystemProvider ,
DiagnosticsConsumer , const Options );
- /// Set the root path of the workspace.
- void setRootPath(PathRef RootPath);
-
/// Add a \p File to the list of tracked C++ files or update the contents if
/// \p File is already tracked. Also schedules parsing of the AST for it on a
/// separate thread. When the parsing is complete, DiagConsumer passed in
@@ -255,8 +257,7 @@
CachedCompletionFuzzyFindRequestByFile;
mutable std::mutex CachedCompletionFuzzyFindRequestMutex;
- // If set, this represents the workspace path.
- llvm::Optional RootPath;
+ llvm::Optional WorkspaceRoot;
std::shared_ptr PCHs;
/// Used to serialize diagnostic callbacks.
/// FIXME(ibiryukov): get rid of an extra map and put all version counters
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -109,6 +109,7 @@
? new FileIndex(Opts.URISchemes,
Opts.HeavyweightDynamicSymbolIndex)
: nullptr),
+ WorkspaceRoot(Opts.WorkspaceRoot),
PCHs(std::make_shared()),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
// parsed file and rebuild the file index synchronously each time an AST
@@ -131,18 +132,6 @@
Index = nullptr;
}
-void ClangdServer::setRootPath(PathRef RootPath) {
- auto FS = FSProvider.getFileSystem();
- auto Status = FS->status(RootPath);
- if (!Status)
-elog("Failed to get status for RootPath {0}: {1}", RootPath,
- Status.getError().message());
- else if (Status->isDirectory())
-this->RootPath = RootPath;
- else
-elog("The provided RootPath {0} is not a directory.", RootPath);
-}
-
void ClangdServer::addDocument(PathRef File, StringRef Contents,
WantDiagnostics WantDiags) {
DocVersion Version = ++InternalVersion[File];
@@ -495,7 +484,7 @@
void ClangdServer::workspaceSymbols(
StringRef Query, int Limit, Callback> CB) {
CB(clangd::getWorkspaceSymbols(Query, Limit, Index,
- RootPath ? *RootPath : ""));
+ WorkspaceRoot.getValueOr("")));
}
void ClangdServer::documentSymbols(
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -261,6 +261,10 @@
void ClangdLSPServer::onInitialize(const InitializeParams ,
Callback Reply) {
+ if (Params.rootUri && *Params.rootUri)
+ClangdServerOpts.WorkspaceRoot = Params.rootUri->file();
+ else if (Params.rootPath && !Params.rootPath->empty())
+ClangdServerOpts.WorkspaceRoot = *Params.rootPath;
if (Server)
return Reply(make_error("server already initialized",
[PATCH] D52730: [analyzer] ConversionChecker: handle floating point
donat.nagy added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:164 +// double and possibly long double on some systems +RepresentsUntilExp = 53; break; + case 32: xazax.hun wrote: > A link to the source of these number would be useful. How are these > calculated. Also, as far as I know the current C++ standard does not require > anything about how floating points are represented in an implementation. So > it would be great to somehow refer to the representation used by clang rather > than hardcoding these values. (Note that I am perfectly fine with warning for > implementation defined behavior as the original version also warn for such in > case of integers.) I took these magic numbers from the IEEE 754 standard; I completely agree that their introduction is far from being elegant. Unfortunately it seems that referring to the representation used by clang seems to be somewhat difficult, see e.g. this old [[ https://stackoverflow.com/questions/13780931/how-do-i-get-llvm-types-from-clang | stackoverflow answer ]]. In the Z3 solver a similar problem was solved by defining a static function ([[ https://clang.llvm.org/doxygen/Z3ConstraintManager_8cpp_source.html#l00269 | getFloatSemantics() ]]) which uses a switch to translate the bit width of a floating point type into an llvm::fltSemantics value (which contains the precision value as a field). I could imagine three solutions: - reimplementing the logic getFloatSemantics, - moving getFloatSemantics to some utility library and using it from there, - keeping the current code, with comments describing my assumptions and referencing the IEEE standard. Which of these is the best? Note: According to the documentation [[ https://releases.llvm.org/2.5/docs/LangRef.html#t_floating | the floating point types ]] supported by the LLVM IR are just float, double and some high precision extension types (which are handled by the `default:` branch of my code). Unfortunately, I do not know what happens to the [[ https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point | `_Float16` ]] half-width float type. Repository: rC Clang https://reviews.llvm.org/D52730 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Some nitty ideas about the log message, but whatever you think is best.
Comment at: clangd/XRefs.cpp:43
+ if (Line >= SymbolLocation::Position::MaxLine)
+log("Get an overflowed line");
+ return Line;
Log message could use more context. And I think it'd be really useful to print
the whole location here.
bikeshed: you could add a method `bool Position::hasOverflow()`, and then write
below
```
if (LSPLoc.range.start.hasOverflow() || LSPLoc.range.end.hasOverflow())
log("Possible overflow in symbol location: {0}", LSPLoc);
```
Distinguishing between line/column overflow isn't important if you're printing
the full range anyway.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53400
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53389: [clangd] Clear the semantic of RefSlab::size.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344745: [clangd] Clear the semantic of RefSlab::size.
(authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53389?vs=170063=170094#toc
Repository:
rL LLVM
https://reviews.llvm.org/D53389
Files:
clangd/index/Background.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Serialization.cpp
Index: clangd/index/Background.cpp
===
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -174,9 +174,9 @@
Action->EndSourceFile();
log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
- Symbols.size(), Refs.size());
+ Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
- SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
+ SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique(std::move(Symbols)),
Index: clangd/index/Serialization.cpp
===
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -496,18 +496,18 @@
}
}
- size_t SymSize = Symbols.size();
- size_t RefSize = Refs.size();
+ size_t NumSym = Symbols.size();
+ size_t NumRefs = Refs.numRefs();
+
trace::Span Tracer("BuildIndex");
auto Index =
UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
: MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2} bytes\n"
- " - number of symbos: {3}\n"
+ " - number of symbols: {3}\n"
" - number of refs: {4}\n",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
- Index->estimateMemoryUsage(),
- SymSize, RefSize);
+ Index->estimateMemoryUsage(), NumSym, NumRefs);
return Index;
}
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -407,7 +407,9 @@
const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
+ /// Gets the number of symbols.
size_t size() const { return Refs.size(); }
+ size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }
size_t bytes() const {
@@ -431,11 +433,14 @@
};
private:
- RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena)
- : Arena(std::move(Arena)), Refs(std::move(Refs)) {}
+ RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena,
+ size_t NumRefs)
+ : Arena(std::move(Arena)), Refs(std::move(Refs)), NumRefs(NumRefs) {}
llvm::BumpPtrAllocator Arena;
std::vector Refs;
+ // Number of all references.
+ size_t NumRefs = 0;
};
struct FuzzyFindRequest {
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -172,17 +172,19 @@
// Reallocate refs on the arena to reduce waste and indirections when reading.
std::vector>> Result;
Result.reserve(Refs.size());
+ size_t NumRefs = 0;
for (auto : Refs) {
auto = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
+NumRefs += SymRefs.size();
auto *Array = Arena.Allocate(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef(Array, SymRefs.size()));
}
- return RefSlab(std::move(Result), std::move(Arena));
+ return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}
void SwapIndex::reset(std::unique_ptr Index) {
Index: clangd/index/FileIndex.cpp
===
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -63,9 +63,9 @@
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
- " ref slab: {4} symbols, {5} bytes",
+ " ref slab: {4} symbols, {5} refs, {6} bytes",
FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
- Refs.bytes());
+ Refs.numRefs(), Refs.bytes());
return {std::move(Syms), std::move(Refs)};
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53389: [clangd] Clear the semantic of RefSlab::size.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344745: [clangd] Clear the semantic of RefSlab::size.
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53389
Files:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
Index: clang-tools-extra/trunk/clangd/index/Background.cpp
===
--- clang-tools-extra/trunk/clangd/index/Background.cpp
+++ clang-tools-extra/trunk/clangd/index/Background.cpp
@@ -174,9 +174,9 @@
Action->EndSourceFile();
log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
- Symbols.size(), Refs.size());
+ Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
- SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
+ SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique(std::move(Symbols)),
Index: clang-tools-extra/trunk/clangd/index/Serialization.cpp
===
--- clang-tools-extra/trunk/clangd/index/Serialization.cpp
+++ clang-tools-extra/trunk/clangd/index/Serialization.cpp
@@ -496,18 +496,18 @@
}
}
- size_t SymSize = Symbols.size();
- size_t RefSize = Refs.size();
+ size_t NumSym = Symbols.size();
+ size_t NumRefs = Refs.numRefs();
+
trace::Span Tracer("BuildIndex");
auto Index =
UseDex ? dex::Dex::build(std::move(Symbols), std::move(Refs), URISchemes)
: MemIndex::build(std::move(Symbols), std::move(Refs));
vlog("Loaded {0} from {1} with estimated memory usage {2} bytes\n"
- " - number of symbos: {3}\n"
+ " - number of symbols: {3}\n"
" - number of refs: {4}\n",
UseDex ? "Dex" : "MemIndex", SymbolFilename,
- Index->estimateMemoryUsage(),
- SymSize, RefSize);
+ Index->estimateMemoryUsage(), NumSym, NumRefs);
return Index;
}
Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++ clang-tools-extra/trunk/clangd/index/Index.h
@@ -407,7 +407,9 @@
const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
+ /// Gets the number of symbols.
size_t size() const { return Refs.size(); }
+ size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }
size_t bytes() const {
@@ -431,11 +433,14 @@
};
private:
- RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena)
- : Arena(std::move(Arena)), Refs(std::move(Refs)) {}
+ RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena,
+ size_t NumRefs)
+ : Arena(std::move(Arena)), Refs(std::move(Refs)), NumRefs(NumRefs) {}
llvm::BumpPtrAllocator Arena;
std::vector Refs;
+ // Number of all references.
+ size_t NumRefs = 0;
};
struct FuzzyFindRequest {
Index: clang-tools-extra/trunk/clangd/index/Index.cpp
===
--- clang-tools-extra/trunk/clangd/index/Index.cpp
+++ clang-tools-extra/trunk/clangd/index/Index.cpp
@@ -172,17 +172,19 @@
// Reallocate refs on the arena to reduce waste and indirections when reading.
std::vector>> Result;
Result.reserve(Refs.size());
+ size_t NumRefs = 0;
for (auto : Refs) {
auto = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
+NumRefs += SymRefs.size();
auto *Array = Arena.Allocate(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef(Array, SymRefs.size()));
}
- return RefSlab(std::move(Result), std::move(Arena));
+ return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}
void SwapIndex::reset(std::unique_ptr Index) {
Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp
@@ -63,9 +63,9 @@
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
- " ref slab: {4} symbols, {5} bytes",
+ " ref slab: {4} symbols, {5} refs, {6} bytes",
FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
- Refs.bytes());
+ Refs.numRefs(), Refs.bytes());
[clang-tools-extra] r344745 - [clangd] Clear the semantic of RefSlab::size.
Author: hokein
Date: Thu Oct 18 08:33:20 2018
New Revision: 344745
URL: http://llvm.org/viewvc/llvm-project?rev=344745=rev
Log:
[clangd] Clear the semantic of RefSlab::size.
Summary:
The RefSlab::size can easily cause confusions, it returns the number of
different symbols, rahter than the number of all references.
- add numRefs() method and cache it, since calculating it everytime is
nontrivial.
- clear misused places.
Reviewers: sammccall
Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet,
cfe-commits
Differential Revision: https://reviews.llvm.org/D53389
Modified:
clang-tools-extra/trunk/clangd/index/Background.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Serialization.cpp
Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=344745=344744=344745=diff
==
--- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Oct 18 08:33:20 2018
@@ -174,9 +174,9 @@ llvm::Error BackgroundIndex::index(tooli
Action->EndSourceFile();
log("Indexed {0} ({1} symbols, {2} refs)", Inputs.CompileCommand.Filename,
- Symbols.size(), Refs.size());
+ Symbols.size(), Refs.numRefs());
SPAN_ATTACH(Tracer, "symbols", int(Symbols.size()));
- SPAN_ATTACH(Tracer, "refs", int(Refs.size()));
+ SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
// FIXME: partition the symbols by file rather than TU, to avoid duplication.
IndexedSymbols.update(AbsolutePath,
llvm::make_unique(std::move(Symbols)),
Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=344745=344744=344745=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Thu Oct 18 08:33:20 2018
@@ -63,9 +63,9 @@ indexSymbols(ASTContext , std::share
auto Refs = Collector.takeRefs();
vlog("index AST for {0} (main={1}): \n"
" symbol slab: {2} symbols, {3} bytes\n"
- " ref slab: {4} symbols, {5} bytes",
+ " ref slab: {4} symbols, {5} refs, {6} bytes",
FileName, IsIndexMainAST, Syms.size(), Syms.bytes(), Refs.size(),
- Refs.bytes());
+ Refs.numRefs(), Refs.bytes());
return {std::move(Syms), std::move(Refs)};
}
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=344745=344744=344745=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Thu Oct 18 08:33:20 2018
@@ -172,17 +172,19 @@ RefSlab RefSlab::Builder::build() && {
// Reallocate refs on the arena to reduce waste and indirections when
reading.
std::vector>> Result;
Result.reserve(Refs.size());
+ size_t NumRefs = 0;
for (auto : Refs) {
auto = Sym.second;
llvm::sort(SymRefs);
// FIXME: do we really need to dedup?
SymRefs.erase(std::unique(SymRefs.begin(), SymRefs.end()), SymRefs.end());
+NumRefs += SymRefs.size();
auto *Array = Arena.Allocate(SymRefs.size());
std::uninitialized_copy(SymRefs.begin(), SymRefs.end(), Array);
Result.emplace_back(Sym.first, ArrayRef(Array, SymRefs.size()));
}
- return RefSlab(std::move(Result), std::move(Arena));
+ return RefSlab(std::move(Result), std::move(Arena), NumRefs);
}
void SwapIndex::reset(std::unique_ptr Index) {
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=344745=344744=344745=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Thu Oct 18 08:33:20 2018
@@ -407,7 +407,9 @@ public:
const_iterator begin() const { return Refs.begin(); }
const_iterator end() const { return Refs.end(); }
+ /// Gets the number of symbols.
size_t size() const { return Refs.size(); }
+ size_t numRefs() const { return NumRefs; }
bool empty() const { return Refs.empty(); }
size_t bytes() const {
@@ -431,11 +433,14 @@ public:
};
private:
- RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena)
- : Arena(std::move(Arena)), Refs(std::move(Refs)) {}
+ RefSlab(std::vector Refs, llvm::BumpPtrAllocator Arena,
+ size_t NumRefs)
+ : Arena(std::move(Arena)),
[PATCH] D53400: [clangd] Remove the overflow log.
hokein updated this revision to Diff 170091.
hokein added a comment.
Add log in XRefs.cpp.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53400
Files:
clangd/XRefs.cpp
clangd/index/Index.cpp
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
constexpr uint32_t SymbolLocation::Position::MaxColumn;
void SymbolLocation::Position::setLine(uint32_t L) {
if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
Line = MaxLine;
return;
}
Line = L;
}
void SymbolLocation::Position::setColumn(uint32_t Col) {
if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
Column = MaxColumn;
return;
}
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -37,6 +37,20 @@
return nullptr;
}
+uint32_t getLine(const SymbolLocation::Position Pos) {
+ uint32_t Line = Pos.line();
+ if (Line >= SymbolLocation::Position::MaxLine)
+log("Get an overflowed line");
+ return Line;
+}
+
+uint32_t getColumn(const SymbolLocation::Position Pos) {
+ uint32_t Column = Pos.column();
+ if (Column >= SymbolLocation::Position::MaxColumn)
+log("Get an overflowed column");
+ return Column;
+}
+
// Convert a SymbolLocation to LSP's Location.
// HintPath is used to resolve the path of URI.
// FIXME: figure out a good home for it, and share the implementation with
@@ -57,10 +71,10 @@
}
Location LSPLoc;
LSPLoc.uri = URIForFile(*Path);
- LSPLoc.range.start.line = Loc.Start.line();
- LSPLoc.range.start.character = Loc.Start.column();
- LSPLoc.range.end.line = Loc.End.line();
- LSPLoc.range.end.character = Loc.End.column();
+ LSPLoc.range.start.line = getLine(Loc.Start);
+ LSPLoc.range.start.character = getColumn(Loc.Start);
+ LSPLoc.range.end.line = getLine(Loc.End);
+ LSPLoc.range.end.character = getColumn(Loc.End);
return LSPLoc;
}
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
constexpr uint32_t SymbolLocation::Position::MaxColumn;
void SymbolLocation::Position::setLine(uint32_t L) {
if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
Line = MaxLine;
return;
}
Line = L;
}
void SymbolLocation::Position::setColumn(uint32_t Col) {
if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
Column = MaxColumn;
return;
}
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -37,6 +37,20 @@
return nullptr;
}
+uint32_t getLine(const SymbolLocation::Position Pos) {
+ uint32_t Line = Pos.line();
+ if (Line >= SymbolLocation::Position::MaxLine)
+log("Get an overflowed line");
+ return Line;
+}
+
+uint32_t getColumn(const SymbolLocation::Position Pos) {
+ uint32_t Column = Pos.column();
+ if (Column >= SymbolLocation::Position::MaxColumn)
+log("Get an overflowed column");
+ return Column;
+}
+
// Convert a SymbolLocation to LSP's Location.
// HintPath is used to resolve the path of URI.
// FIXME: figure out a good home for it, and share the implementation with
@@ -57,10 +71,10 @@
}
Location LSPLoc;
LSPLoc.uri = URIForFile(*Path);
- LSPLoc.range.start.line = Loc.Start.line();
- LSPLoc.range.start.character = Loc.Start.column();
- LSPLoc.range.end.line = Loc.End.line();
- LSPLoc.range.end.character = Loc.End.column();
+ LSPLoc.range.start.line = getLine(Loc.Start);
+ LSPLoc.range.start.character = getColumn(Loc.Start);
+ LSPLoc.range.end.line = getLine(Loc.End);
+ LSPLoc.range.end.character = getColumn(Loc.End);
return LSPLoc;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: clangd/ClangdLSPServer.cpp:558
json::Object{
{"uri", URIForFile{File}},
+{"diagnostics", std::move(LSPDiagnostics)},
nit: we can reuse `URI` insteading of calling `URIForFile` again.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53391
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
sammccall added a comment. After looking at the examples, I'm reassured that we don't really care about tracking precise locations of those references :-) I'd suggest adding the logging just to where the field is *read* in XRefs.cpp (check if it's max-value). That would cover a bunch of the cases where an incorrect location is likely to be visible. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53400 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r344742 - [X86][Tests] Make sure tls-direct-seg-refs tests only run where supported
Author: kristina
Date: Thu Oct 18 07:44:25 2018
New Revision: 344742
URL: http://llvm.org/viewvc/llvm-project?rev=344742=rev
Log:
[X86][Tests] Make sure tls-direct-seg-refs tests only run where supported
This flag is only supported for x86 targets, make sure the tests only run
for those.
Modified:
cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c
cfe/trunk/test/Driver/indirect-tls-seg-refs.c
Modified: cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c?rev=344742=344741=344742=diff
==
--- cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c (original)
+++ cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c Thu Oct 18 07:44:25 2018
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -mno-tls-direct-seg-refs | FileCheck %s
-check-prefix=NO-TLSDIRECT
-// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s -check-prefix=TLSDIRECT
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-unknown-linux -o -
-mno-tls-direct-seg-refs | FileCheck %s -check-prefix=NO-TLSDIRECT
+// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-unknown-linux -o - | FileCheck
%s -check-prefix=TLSDIRECT
// NO-TLSDIRECT: attributes #{{[0-9]+}} = {{{.*}} "indirect-tls-seg-refs"
// TLSDIRECT-NOT: attributes #{{[0-9]+}} = {{{.*}} "indirect-tls-seg-refs"
Modified: cfe/trunk/test/Driver/indirect-tls-seg-refs.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/indirect-tls-seg-refs.c?rev=344742=344741=344742=diff
==
--- cfe/trunk/test/Driver/indirect-tls-seg-refs.c (original)
+++ cfe/trunk/test/Driver/indirect-tls-seg-refs.c Thu Oct 18 07:44:25 2018
@@ -1,7 +1,8 @@
-// RUN: %clang -### %s 2>&1 | FileCheck %s -check-prefix=TLSDIRECT
-// RUN: %clang -### -mno-tls-direct-seg-refs -mtls-direct-seg-refs %s 2>&1 |
FileCheck %s -check-prefix=TLSDIRECT
-// RUN: %clang -### -mtls-direct-seg-refs -mno-tls-direct-seg-refs %s 2>&1 |
FileCheck %s -check-prefix=NO-TLSDIRECT
-// REQUIRES: clang-driver
+// REQUIRES: clang-driver, x86-registered-target
+
+// RUN: %clang -### -target x86_64-unknown-linux %s 2>&1 | FileCheck %s
-check-prefix=TLSDIRECT
+// RUN: %clang -### -target x86_64-unknown-linux -mno-tls-direct-seg-refs
-mtls-direct-seg-refs %s 2>&1 | FileCheck %s -check-prefix=TLSDIRECT
+// RUN: %clang -### -target x86_64-unknown-linux -mtls-direct-seg-refs
-mno-tls-direct-seg-refs %s 2>&1 | FileCheck %s -check-prefix=NO-TLSDIRECT
// NO-TLSDIRECT: -mno-tls-direct-seg-refs
// TLSDIRECT-NOT: -mno-tls-direct-seg-refs
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53400: [clangd] Remove the overflow log.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric,
ilya-biryukov.
LLVM codebase has generated files (all are build/Target/XXX/*.inc) that
exceed the MaxLine & MaxColumn. Printing these log would be noisy.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53400
Files:
clangd/index/Index.cpp
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
constexpr uint32_t SymbolLocation::Position::MaxColumn;
void SymbolLocation::Position::setLine(uint32_t L) {
if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
Line = MaxLine;
return;
}
Line = L;
}
void SymbolLocation::Position::setColumn(uint32_t Col) {
if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
Column = MaxColumn;
return;
}
Index: clangd/index/Index.cpp
===
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -23,15 +23,13 @@
constexpr uint32_t SymbolLocation::Position::MaxColumn;
void SymbolLocation::Position::setLine(uint32_t L) {
if (L > MaxLine) {
-log("Set an overflowed Line {0}", L);
Line = MaxLine;
return;
}
Line = L;
}
void SymbolLocation::Position::setColumn(uint32_t Col) {
if (Col > MaxColumn) {
-log("Set an overflowed Column {0}", Col);
Column = MaxColumn;
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53398: [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344741: [clangd] Enforce rules around initialize
request, and create ClangdServer… (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53398
Files:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/test/clangd/exit-with-shutdown.test
clang-tools-extra/trunk/test/clangd/exit-without-shutdown.test
clang-tools-extra/trunk/test/clangd/initialize-sequence.test
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h
@@ -183,12 +183,10 @@
// Store of the current versions of the open documents.
DraftStore DraftMgr;
- // Server must be the last member of the class to allow its destructor to exit
- // the worker thread that may otherwise run an async callback on partially
- // destructed instance of ClangdLSPServer.
- // Set in construtor and destroyed when run() finishes. To ensure all worker
- // threads exit before run() returns.
- std::unique_ptr Server;
+ // The ClangdServer is created by the "initialize" LSP method.
+ // It is destroyed before run() returns, to ensure worker threads exit.
+ ClangdServer::Options ClangdServerOpts;
+ llvm::Optional Server;
};
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp
@@ -103,7 +103,7 @@
DiagnosticsConsumer ,
const Options )
: CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
- ResourceDir(Opts.ResourceDir ? Opts.ResourceDir->str()
+ ResourceDir(Opts.ResourceDir ? *Opts.ResourceDir
: getStandardResourceDir()),
DynamicIdx(Opts.BuildDynamicSymbolIndex
? new FileIndex(Opts.URISchemes,
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -87,15 +87,18 @@
// - logging of inbound messages
// - cancellation handling
// - basic call tracing
+// MessageHandler ensures that initialize() is called before any other handler.
class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
public:
MessageHandler(ClangdLSPServer ) : Server(Server) {}
bool onNotify(StringRef Method, json::Value Params) override {
log("<-- {0}", Method);
if (Method == "exit")
return false;
-if (Method == "$/cancelRequest")
+if (!Server.Server)
+ elog("Notification {0} before initialization", Method);
+else if (Method == "$/cancelRequest")
onCancel(std::move(Params));
else if (auto Handler = Notifications.lookup(Method))
Handler(std::move(Params));
@@ -106,7 +109,11 @@
bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
log("<-- {0}({1})", Method, ID);
-if (auto Handler = Calls.lookup(Method))
+if (!Server.Server && Method != "initialize") {
+ elog("Call {0} before initialization.", Method);
+ Server.reply(ID, make_error("server not initialized",
+ErrorCode::ServerNotInitialized));
+} else if (auto Handler = Calls.lookup(Method))
Handler(std::move(Params), std::move(ID));
else
Server.reply(ID, llvm::make_error("method not found",
@@ -254,6 +261,11 @@
void ClangdLSPServer::onInitialize(const InitializeParams ,
Callback Reply) {
+ if (Server)
+return Reply(make_error("server already initialized",
+ ErrorCode::InvalidRequest));
+ Server.emplace(CDB.getCDB(), FSProvider,
+ static_cast(*this), ClangdServerOpts);
if (Params.initializationOptions) {
const ClangdInitializationOptions = *Params.initializationOptions;
@@ -663,8 +675,7 @@
std::move(CompileCommandsDir))),
CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
SupportedCompletionItemKinds(defaultCompletionItemKinds()),
- Server(new ClangdServer(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this,
- Opts)) {
+ ClangdServerOpts(Opts) {
// clang-format off
MsgHandler->bind("initialize", ::onInitialize);
MsgHandler->bind("shutdown", ::onShutdown);
@@ -694,8 +705,6 @@
[clang-tools-extra] r344741 - [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.
Author: sammccall
Date: Thu Oct 18 07:41:50 2018
New Revision: 344741
URL: http://llvm.org/viewvc/llvm-project?rev=344741=rev
Log:
[clangd] Enforce rules around "initialize" request, and create ClangdServer
lazily.
Summary:
LSP is a slightly awkward map to C++ object lifetimes: the initialize request
is part of the protocol and provides information that doesn't change over the
lifetime of the server.
Until now, we handled this by initializing ClangdServer and ClangdLSPServer
right away, and making anything that can be set in the "initialize" request
mutable.
With this patch, we create ClangdLSPServer immediately, but defer creating
ClangdServer until "initialize". This opens the door to passing the relevant
initialize params in the constructor and storing them immutably.
(That change isn't actually done in this patch).
To make this safe, we have the MessageDispatcher enforce that the "initialize"
method is called before any other (as required by LSP). That way each method
handler can assume Server is initialized, as today.
As usual, while implementing this I found places where our test cases violated
the protocol.
Reviewers: ioeric
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53398
Added:
clang-tools-extra/trunk/test/clangd/initialize-sequence.test
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/test/clangd/exit-with-shutdown.test
clang-tools-extra/trunk/test/clangd/exit-without-shutdown.test
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344741=344740=344741=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct 18 07:41:50 2018
@@ -87,6 +87,7 @@ CompletionItemKindBitset defaultCompleti
// - logging of inbound messages
// - cancellation handling
// - basic call tracing
+// MessageHandler ensures that initialize() is called before any other handler.
class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
public:
MessageHandler(ClangdLSPServer ) : Server(Server) {}
@@ -95,7 +96,9 @@ public:
log("<-- {0}", Method);
if (Method == "exit")
return false;
-if (Method == "$/cancelRequest")
+if (!Server.Server)
+ elog("Notification {0} before initialization", Method);
+else if (Method == "$/cancelRequest")
onCancel(std::move(Params));
else if (auto Handler = Notifications.lookup(Method))
Handler(std::move(Params));
@@ -106,7 +109,11 @@ public:
bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
log("<-- {0}({1})", Method, ID);
-if (auto Handler = Calls.lookup(Method))
+if (!Server.Server && Method != "initialize") {
+ elog("Call {0} before initialization.", Method);
+ Server.reply(ID, make_error("server not initialized",
+ErrorCode::ServerNotInitialized));
+} else if (auto Handler = Calls.lookup(Method))
Handler(std::move(Params), std::move(ID));
else
Server.reply(ID, llvm::make_error("method not found",
@@ -254,6 +261,11 @@ void ClangdLSPServer::reply(llvm::json::
void ClangdLSPServer::onInitialize(const InitializeParams ,
Callback Reply) {
+ if (Server)
+return Reply(make_error("server already initialized",
+ ErrorCode::InvalidRequest));
+ Server.emplace(CDB.getCDB(), FSProvider,
+ static_cast(*this), ClangdServerOpts);
if (Params.initializationOptions) {
const ClangdInitializationOptions = *Params.initializationOptions;
@@ -663,8 +675,7 @@ ClangdLSPServer::ClangdLSPServer(class T
std::move(CompileCommandsDir))),
CCOpts(CCOpts), SupportedSymbolKinds(defaultSymbolKinds()),
SupportedCompletionItemKinds(defaultCompletionItemKinds()),
- Server(new ClangdServer(CDB.getCDB(), FSProvider, /*DiagConsumer=*/*this,
- Opts)) {
+ ClangdServerOpts(Opts) {
// clang-format off
MsgHandler->bind("initialize", ::onInitialize);
MsgHandler->bind("shutdown", ::onShutdown);
@@ -694,8 +705,6 @@ ClangdLSPServer::ClangdLSPServer(class T
ClangdLSPServer::~ClangdLSPServer() = default;
bool ClangdLSPServer::run() {
- assert(Server);
-
// Run the Language Server loop.
bool CleanExit = true;
if (auto Err = Transp.loop(*MsgHandler)) {
@@ -705,7 +714,6 @@ bool ClangdLSPServer::run() {
// Destroy ClangdServer to ensure all worker threads finish.
[PATCH] D53399: [clangd] Ensure that we reply to each call exactly once. NFC (I think!)
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, jfb, arphaman, jkorous,
MaskRay, ilya-biryukov.
In debug builds, getting this wrong will trigger asserts.
In production builds, it will send an error reply if none was sent,
and drop redundant replies. (And log).
No tests because this is always a programming error.
(We did have some cases of this, but I fixed them with the new dispatcher).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53399
Files:
clangd/ClangdLSPServer.cpp
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -106,11 +106,14 @@
bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
log("<-- {0}({1})", Method, ID);
+// Calls can be canceled by the client. Add cancellation context.
+WithContext WithCancel(cancelableRequestContext(ID));
+ReplyOnce Reply(ID, Method, );
if (auto Handler = Calls.lookup(Method))
- Handler(std::move(Params), std::move(ID));
+ Handler(std::move(Params), std::move(Reply));
else
- Server.reply(ID, llvm::make_error("method not found",
- ErrorCode::MethodNotFound));
+ Reply(llvm::make_error("method not found",
+ ErrorCode::MethodNotFound));
return true;
}
@@ -124,34 +127,31 @@
}
// Bind an LSP method name to a call.
- template
+ template
void bind(const char *Method,
-void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
+void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
Calls[Method] = [Method, Handler, this](json::Value RawParams,
-json::Value ID) {
+ReplyOnce Reply) {
Param P;
if (!fromJSON(RawParams, P)) {
elog("Failed to decode {0} request.", Method);
-Server.reply(ID, make_error("failed to decode request",
- ErrorCode::InvalidRequest));
+Reply(make_error("failed to decode request",
+ ErrorCode::InvalidRequest));
return;
}
trace::Span Tracer(Method);
SPAN_ATTACH(Tracer, "Params", RawParams);
auto *Trace = Tracer.Args; // We attach reply from another thread.
- // Calls can be canceled by the client. Add cancellation context.
- WithContext WithCancel(cancelableRequestContext(ID));
- // FIXME: this function should assert it's called exactly once.
- (Server.*Handler)(P, [this, ID, Trace](llvm::Expected Result) {
-if (Result) {
+ (Server.*Handler)(P, [Trace, Reply](llvm::Expected Res) {
+if (Res) {
if (Trace)
-(*Trace)["Reply"] = *Result;
- Server.reply(ID, json::Value(std::move(*Result)));
+(*Trace)["Reply"] = *Res;
+ Reply(json::Value(std::move(*Res)));
} else {
- auto Err = Result.takeError();
+ auto Err = Res.takeError();
if (Trace)
(*Trace)["Error"] = llvm::to_string(Err);
- Server.reply(ID, std::move(Err));
+ Reply(std::move(Err));
}
});
};
@@ -174,8 +174,47 @@
}
private:
+ // Function object to reply to an LSP call.
+ // Each instance must be called exactly once, otherwise:
+ // - the bug is logged, and (in debug mode) an assert will fire
+ // - if there was no reply, an error reply is sent
+ // - if there were multiple replies, only the first is sent
+ class ReplyOnce {
+struct State {
+ std::atomic Replied = {false};
+ json::Value ID;
+ std::string Method;
+ ClangdLSPServer *Server;
+
+ State(const json::Value , StringRef Method, ClangdLSPServer *Server)
+ : ID(ID), Method(Method), Server(Server) {}
+ ~State() {
+if (!Replied) {
+ elog("No reply to message {0}({1})", Method, ID);
+ assert(false && "must reply to all calls!");
+ Server->reply(ID, make_error("server failed to reply",
+ ErrorCode::InternalError));
+}
+ }
+};
+std::shared_ptr MyState;
+
+ public:
+ReplyOnce(const json::Value , StringRef Method, ClangdLSPServer *Server)
+: MyState(std::make_shared(ID, Method, Server)) {}
+
+void operator()(llvm::Expected Reply) const {
+ if (MyState->Replied.exchange(true)) {
+elog("Replied twice to message {0}({1})", MyState->Method, MyState->ID);
+assert(false && "must reply to each call only once!");
+return;
+ }
+ MyState->Server->reply(MyState->ID, std::move(Reply));
+}
+ };
+
llvm::StringMap> Notifications;
- llvm::StringMap> Calls;
+ llvm::StringMap> Calls;
// Method calls may
r344740 - [OPENMP] Move OMPClausePrinter to OpenMPClause.h/OpenMPClause.cpp - NFC. Differential Revision: https://reviews.llvm.org/D53102
Author: plyster
Date: Thu Oct 18 07:28:23 2018
New Revision: 344740
URL: http://llvm.org/viewvc/llvm-project?rev=344740=rev
Log:
[OPENMP] Move OMPClausePrinter to OpenMPClause.h/OpenMPClause.cpp - NFC.
Differential Revision: https://reviews.llvm.org/D53102
Modified:
cfe/trunk/include/clang/AST/OpenMPClause.h
cfe/trunk/lib/AST/OpenMPClause.cpp
cfe/trunk/lib/AST/StmtPrinter.cpp
Modified: cfe/trunk/include/clang/AST/OpenMPClause.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OpenMPClause.h?rev=344740=344739=344740=diff
==
--- cfe/trunk/include/clang/AST/OpenMPClause.h (original)
+++ cfe/trunk/include/clang/AST/OpenMPClause.h Thu Oct 18 07:28:23 2018
@@ -5203,6 +5203,22 @@ class OMPClauseVisitor :
template
class ConstOMPClauseVisitor :
public OMPClauseVisitorBase {};
+
+class OMPClausePrinter final : public OMPClauseVisitor {
+ raw_ostream
+ const PrintingPolicy
+
+ /// Process clauses with list of variables.
+ template void VisitOMPClauseList(T *Node, char StartSym);
+
+public:
+ OMPClausePrinter(raw_ostream , const PrintingPolicy )
+ : OS(OS), Policy(Policy) {}
+
+#define OPENMP_CLAUSE(Name, Class) void Visit##Class(Class *S);
+#include "clang/Basic/OpenMPKinds.def"
+};
+
} // namespace clang
#endif // LLVM_CLANG_AST_OPENMPCLAUSE_H
Modified: cfe/trunk/lib/AST/OpenMPClause.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/OpenMPClause.cpp?rev=344740=344739=344740=diff
==
--- cfe/trunk/lib/AST/OpenMPClause.cpp (original)
+++ cfe/trunk/lib/AST/OpenMPClause.cpp Thu Oct 18 07:28:23 2018
@@ -14,6 +14,7 @@
#include "clang/AST/OpenMPClause.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/DeclOpenMP.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/Support/Casting.h"
@@ -1049,3 +1050,434 @@ OMPIsDevicePtrClause *OMPIsDevicePtrClau
return new (Mem) OMPIsDevicePtrClause(NumVars, NumUniqueDeclarations,
NumComponentLists, NumComponents);
}
+
+//===--===//
+// OpenMP clauses printing methods
+//===--===//
+
+void OMPClausePrinter::VisitOMPIfClause(OMPIfClause *Node) {
+ OS << "if(";
+ if (Node->getNameModifier() != OMPD_unknown)
+OS << getOpenMPDirectiveName(Node->getNameModifier()) << ": ";
+ Node->getCondition()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPFinalClause(OMPFinalClause *Node) {
+ OS << "final(";
+ Node->getCondition()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPNumThreadsClause(OMPNumThreadsClause *Node) {
+ OS << "num_threads(";
+ Node->getNumThreads()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPSafelenClause(OMPSafelenClause *Node) {
+ OS << "safelen(";
+ Node->getSafelen()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPSimdlenClause(OMPSimdlenClause *Node) {
+ OS << "simdlen(";
+ Node->getSimdlen()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPCollapseClause(OMPCollapseClause *Node) {
+ OS << "collapse(";
+ Node->getNumForLoops()->printPretty(OS, nullptr, Policy, 0);
+ OS << ")";
+}
+
+void OMPClausePrinter::VisitOMPDefaultClause(OMPDefaultClause *Node) {
+ OS << "default("
+ << getOpenMPSimpleClauseTypeName(OMPC_default, Node->getDefaultKind())
+ << ")";
+}
+
+void OMPClausePrinter::VisitOMPProcBindClause(OMPProcBindClause *Node) {
+ OS << "proc_bind("
+ << getOpenMPSimpleClauseTypeName(OMPC_proc_bind, Node->getProcBindKind())
+ << ")";
+}
+
+void OMPClausePrinter::VisitOMPUnifiedAddressClause(OMPUnifiedAddressClause *)
{
+ OS << "unified_address";
+}
+
+void OMPClausePrinter::VisitOMPUnifiedSharedMemoryClause(
+OMPUnifiedSharedMemoryClause *) {
+ OS << "unified_shared_memory";
+}
+
+void OMPClausePrinter::VisitOMPReverseOffloadClause(OMPReverseOffloadClause *)
{
+ OS << "reverse_offload";
+}
+
+void OMPClausePrinter::VisitOMPDynamicAllocatorsClause(
+OMPDynamicAllocatorsClause *) {
+ OS << "dynamic_allocators";
+}
+
+void OMPClausePrinter::VisitOMPScheduleClause(OMPScheduleClause *Node) {
+ OS << "schedule(";
+ if (Node->getFirstScheduleModifier() != OMPC_SCHEDULE_MODIFIER_unknown) {
+OS << getOpenMPSimpleClauseTypeName(OMPC_schedule,
+Node->getFirstScheduleModifier());
+if (Node->getSecondScheduleModifier() != OMPC_SCHEDULE_MODIFIER_unknown) {
+ OS << ", ";
+ OS << getOpenMPSimpleClauseTypeName(OMPC_schedule,
+ Node->getSecondScheduleModifier());
+}
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
nruslan added a comment. @kristina : Thank you very much for taking care of the patchsets! Repository: rC Clang https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check
hwright added inline comments.
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:34
+llvm::APSInt ApInt(/*BitWidth=*/64, /*isUnsigned=*/false);
+ApInt = static_cast(value);
+if (is_negative)
JonasToth wrote:
> Wouldn't it make more sense to use `std::uint64_t` instead to correspond to
> the line above? And where does the signedness difference come from?
> (`/*isUnsigned=*/false`)
I don't remember where the signedness difference came from, so I've made this
`std::int64_t`.
Comment at: clang-tidy/abseil/DurationFactoryFloatCheck.cpp:69
+ // Macros are ignored.
+ if (Arg->getBeginLoc().isMacroID())
+return;
JonasToth wrote:
> maybe `assert` instead, as your comment above suggests that macros are
> already filtered out?
This is a different check than above.
In the first case, we want to be sure we aren't replacing cases inside of a
macro, such as:
```
#define SECONDS(x) absl::Seconds(x)
SECONDS(1.0)
```
In this one, we want to avoid changing the argument if it is itself a macro:
```
#define VAL 1.0
absl::Seconds(VAL);
```
So it is a separate check, not just a re-assertion of the first one.
Comment at: test/clang-tidy/abseil-duration-factory-float.cpp:32
+void ConvertFloatTest() {
+ absl::Duration d;
+
JonasToth wrote:
> Could you also provide test cases with hexadecimal floating literals, which
> are C++17? The thousand-separators could be checked as well (dunno the
> correct term right now, but the `1'000'000` feature).
> Please add test cases for negative literals, too.
Added the hexadecimal floating literal tests.
The testing infrastructure wouldn't build the test source with `3'000` as a
literal argument. (There's also an argument that by the time we get to the
AST, that distinction is gone anyway and this test isn't the place to check
comprehensive literal parsing.)
I've also added a negative literal test, to illustrate that we don't yet handle
that case (which is in practice pretty rare). I'd rather add it in a separate
change.
https://reviews.llvm.org/D53339
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53339: [clang-tidy] Add the abseil-duration-factory-float check
hwright updated this revision to Diff 170083.
hwright marked 6 inline comments as done.
hwright added a comment.
Addressed reviewer comments.
https://reviews.llvm.org/D53339
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/DurationFactoryFloatCheck.cpp
clang-tidy/abseil/DurationFactoryFloatCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-duration-factory-float.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-duration-factory-float.cpp
Index: test/clang-tidy/abseil-duration-factory-float.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-factory-float.cpp
@@ -0,0 +1,115 @@
+// RUN: %check_clang_tidy %s abseil-duration-factory-float %t
+
+// Mimic the implementation of absl::Duration
+namespace absl {
+
+class Duration {};
+
+Duration Nanoseconds(long long);
+Duration Microseconds(long long);
+Duration Milliseconds(long long);
+Duration Seconds(long long);
+Duration Minutes(long long);
+Duration Hours(long long);
+
+#define GENERATE_DURATION_FACTORY_OVERLOADS(NAME) \
+ Duration NAME(float n); \
+ Duration NAME(double n);\
+ template\
+ Duration NAME(T n);
+
+GENERATE_DURATION_FACTORY_OVERLOADS(Nanoseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Microseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Milliseconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Seconds);
+GENERATE_DURATION_FACTORY_OVERLOADS(Minutes);
+GENERATE_DURATION_FACTORY_OVERLOADS(Hours);
+#undef GENERATE_DURATION_FACTORY_OVERLOADS
+
+} // namespace absl
+
+void ConvertFloatTest() {
+ absl::Duration d;
+
+ d = absl::Seconds(60.0);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(60);
+ d = absl::Minutes(300.0);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Minutes [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Minutes(300);
+
+ d = absl::Milliseconds(1e2);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Milliseconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Milliseconds(100);
+ d = absl::Seconds(3.0f);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(3);
+ d = absl::Seconds(3.);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(3);
+ d = absl::Seconds(0x3.p0);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(3);
+ d = absl::Seconds(0x3.p1);
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(6);
+
+
+ // Ignored expressions
+ d = absl::Seconds(.001);
+ d = absl::Seconds(.100);
+ d = ::absl::Seconds(1);
+ d = ::absl::Minutes(1);
+ d = ::absl::Hours(1);
+ d = absl::Seconds(0x3.4p1);
+
+ // Negative literals (we don't yet handle this case)
+ d = absl::Seconds(-3.0);
+
+ // This is bigger than we can safely fit in a positive int32, so we ignore it.
+ d = absl::Seconds(1e12);
+
+ int x;
+ d = absl::Seconds(x);
+ float y;
+ d = absl::Minutes(y);
+
+#define SECONDS(x) absl::Seconds(x)
+ SECONDS(60);
+#undef SECONDS
+#define THIRTY 30.0
+ d = absl::Seconds(THIRTY);
+#undef THIRTY
+}
+
+template
+void InTemplate() {
+ absl::Duration d;
+
+ d = absl::Seconds(N); // 1
+ // ^ No replacement here.
+
+ d = absl::Minutes(1.0); // 2
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Minutes [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Minutes(1); // 2
+}
+
+void Instantiate() {
+ InTemplate<60>();
+ InTemplate<1>();
+}
+
+void ConvertStaticCastTest() {
+ absl::Duration d;
+
+ d = absl::Seconds(static_cast(5));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Seconds [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Seconds(5);
+
+ d = absl::Minutes(static_cast(5));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: Use integer version of absl::Minutes [abseil-duration-factory-float]
+ // CHECK-FIXES: absl::Minutes(5);
+
+ // This should not be flagged
+ d = absl::Seconds(static_cast(5.0));
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
.. toctree::
abseil-duration-division
+ abseil-duration-factory-float
abseil-faster-strsplit-delimiter
abseil-no-internal-dependencies
abseil-no-namespace
Index: docs/clang-tidy/checks/abseil-duration-factory-float.rst
[PATCH] D53372: [clang-tidy] Resolve readability-else-after-return false positive for constexpr if.
aaron.ballman added inline comments. Comment at: test/clang-tidy/readability-else-after-return-if-constexpr.cpp:9 +return; + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: + Please add some of the warning text -- any warning will match this. Comment at: test/clang-tidy/readability-else-after-return-if-constexpr.cpp:23 +} +// CHECK-NOT: warning: This seems unnecessary. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344739: Add support for -mno-tls-direct-seg-refs to Clang
(authored by kristina, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53102?vs=169224=170082#toc
Repository:
rC Clang
https://reviews.llvm.org/D53102
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CGCall.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/indirect-tls-seg-refs.c
test/Driver/indirect-tls-seg-refs.c
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -62,6 +62,8 @@
CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
+ ///< is specified.
CODEGENOPT(DisableTailCalls , 1, 0) ///< Do not emit tail calls.
CODEGENOPT(NoEscapingBlockTailCalls, 1, 0) ///< Do not emit tail calls from
///< escaping blocks.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2011,6 +2011,8 @@
def mno_pascal_strings : Flag<["-"], "mno-pascal-strings">,
Alias;
def mno_red_zone : Flag<["-"], "mno-red-zone">, Group;
+def mno_tls_direct_seg_refs : Flag<["-"], "mno-tls-direct-seg-refs">,
Group, Flags<[CC1Option]>,
+ HelpText<"Disable direct TLS access through segment registers">;
def mno_relax_all : Flag<["-"], "mno-relax-all">, Group;
def mno_rtd: Flag<["-"], "mno-rtd">, Group;
def mno_soft_float : Flag<["-"], "mno-soft-float">, Group;
@@ -2171,6 +2173,8 @@
def moslib_EQ : Joined<["-"], "moslib=">, Group;
def mpascal_strings : Flag<["-"], "mpascal-strings">, Alias;
def mred_zone : Flag<["-"], "mred-zone">, Group;
+def mtls_direct_seg_refs : Flag<["-"], "mtls-direct-seg-refs">, Group,
+ HelpText<"Enable direct TLS access through segment registers (default)">;
def mregparm_EQ : Joined<["-"], "mregparm=">, Group;
def mrelax_all : Flag<["-"], "mrelax-all">, Group,
Flags<[CC1Option,CC1AsOption]>,
HelpText<"(integrated-as) Relax all machine instructions">;
Index: test/CodeGen/indirect-tls-seg-refs.c
===
--- test/CodeGen/indirect-tls-seg-refs.c
+++ test/CodeGen/indirect-tls-seg-refs.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -mno-tls-direct-seg-refs | FileCheck %s
-check-prefix=NO-TLSDIRECT
+// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s -check-prefix=TLSDIRECT
+
+// NO-TLSDIRECT: attributes #{{[0-9]+}} = {{{.*}} "indirect-tls-seg-refs"
+// TLSDIRECT-NOT: attributes #{{[0-9]+}} = {{{.*}} "indirect-tls-seg-refs"
+
+void test1() {
+}
Index: test/Driver/indirect-tls-seg-refs.c
===
--- test/Driver/indirect-tls-seg-refs.c
+++ test/Driver/indirect-tls-seg-refs.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### %s 2>&1 | FileCheck %s -check-prefix=TLSDIRECT
+// RUN: %clang -### -mno-tls-direct-seg-refs -mtls-direct-seg-refs %s 2>&1 |
FileCheck %s -check-prefix=TLSDIRECT
+// RUN: %clang -### -mtls-direct-seg-refs -mno-tls-direct-seg-refs %s 2>&1 |
FileCheck %s -check-prefix=NO-TLSDIRECT
+// REQUIRES: clang-driver
+
+// NO-TLSDIRECT: -mno-tls-direct-seg-refs
+// TLSDIRECT-NOT: -mno-tls-direct-seg-refs
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1709,6 +1709,8 @@
if (CodeGenOpts.DisableRedZone)
FuncAttrs.addAttribute(llvm::Attribute::NoRedZone);
+ if (CodeGenOpts.IndirectTlsSegRefs)
+FuncAttrs.addAttribute("indirect-tls-seg-refs");
if (CodeGenOpts.NoImplicitFloat)
FuncAttrs.addAttribute(llvm::Attribute::NoImplicitFloat);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1739,6 +1739,10 @@
Args.hasArg(options::OPT_fapple_kext))
CmdArgs.push_back("-disable-red-zone");
+ if (!Args.hasFlag(options::OPT_mtls_direct_seg_refs,
+options::OPT_mno_tls_direct_seg_refs, true))
+CmdArgs.push_back("-mno-tls-direct-seg-refs");
+
// Default to avoid implicit floating-point for kernel/kext code, but allow
// that to be overridden with -mno-soft-float.
bool NoImplicitFloat = (Args.hasArg(options::OPT_mkernel) ||
Index: lib/Frontend/CompilerInvocation.cpp
===
r344739 - Add support for -mno-tls-direct-seg-refs to Clang
Author: kristina
Date: Thu Oct 18 07:07:02 2018
New Revision: 344739
URL: http://llvm.org/viewvc/llvm-project?rev=344739=rev
Log:
Add support for -mno-tls-direct-seg-refs to Clang
This patch exposes functionality added in rL344723 to the Clang driver/frontend
as a flag and adds appropriate metadata.
Driver tests pass:
```
ninja check-clang-driver
-snip-
Expected Passes: 472
Expected Failures : 3
Unsupported Tests : 65
```
Odd failure in CodeGen tests but unrelated to this:
```
ninja check-clang-codegen
-snip-
/SourceCache/llvm-trunk-8.0/tools/clang/test/CodeGen/builtins-wasm.c:87:10:
error: cannot compile this builtin function yet
-snip-
Failing Tests (1):
Clang :: CodeGen/builtins-wasm.c
Expected Passes: 1250
Expected Failures : 2
Unsupported Tests : 120
Unexpected Failures: 1
```
Original commit:
[X86] Support for the mno-tls-direct-seg-refs flag
Allows to disable direct TLS segment access (%fs or %gs). GCC supports a
similar flag, it can be useful in some circumstances, e.g. when a thread
context block needs to be updated directly from user space. More info and
specific use cases: https://bugs.llvm.org/show_bug.cgi?id=16145
Patch by nruslan (Ruslan Nikolaev).
Differential Revision: https://reviews.llvm.org/D53102
Added:
cfe/trunk/test/CodeGen/indirect-tls-seg-refs.c
cfe/trunk/test/Driver/indirect-tls-seg-refs.c
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=344739=344738=344739=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Oct 18 07:07:02 2018
@@ -2011,6 +2011,8 @@ def mno_global_merge : Flag<["-"], "mno-
def mno_pascal_strings : Flag<["-"], "mno-pascal-strings">,
Alias;
def mno_red_zone : Flag<["-"], "mno-red-zone">, Group;
+def mno_tls_direct_seg_refs : Flag<["-"], "mno-tls-direct-seg-refs">,
Group, Flags<[CC1Option]>,
+ HelpText<"Disable direct TLS access through segment registers">;
def mno_relax_all : Flag<["-"], "mno-relax-all">, Group;
def mno_rtd: Flag<["-"], "mno-rtd">, Group;
def mno_soft_float : Flag<["-"], "mno-soft-float">, Group;
@@ -2171,6 +2173,8 @@ def momit_leaf_frame_pointer : Flag<["-"
def moslib_EQ : Joined<["-"], "moslib=">, Group;
def mpascal_strings : Flag<["-"], "mpascal-strings">, Alias;
def mred_zone : Flag<["-"], "mred-zone">, Group;
+def mtls_direct_seg_refs : Flag<["-"], "mtls-direct-seg-refs">, Group,
+ HelpText<"Enable direct TLS access through segment registers (default)">;
def mregparm_EQ : Joined<["-"], "mregparm=">, Group;
def mrelax_all : Flag<["-"], "mrelax-all">, Group,
Flags<[CC1Option,CC1AsOption]>,
HelpText<"(integrated-as) Relax all machine instructions">;
Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=344739=344738=344739=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Thu Oct 18 07:07:02 2018
@@ -62,6 +62,8 @@ CODEGENOPT(ExperimentalNewPassManager, 1
CODEGENOPT(DebugPassManager, 1, 0) ///< Prints debug information for the new
///< pass manager.
CODEGENOPT(DisableRedZone, 1, 0) ///< Set when -mno-red-zone is enabled.
+CODEGENOPT(IndirectTlsSegRefs, 1, 0) ///< Set when -mno-tls-direct-seg-refs
+ ///< is specified.
CODEGENOPT(DisableTailCalls , 1, 0) ///< Do not emit tail calls.
CODEGENOPT(NoEscapingBlockTailCalls, 1, 0) ///< Do not emit tail calls from
///< escaping blocks.
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=344739=344738=344739=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Thu Oct 18 07:07:02 2018
@@ -1709,6 +1709,8 @@ void CodeGenModule::ConstructDefaultFnAt
if (CodeGenOpts.DisableRedZone)
FuncAttrs.addAttribute(llvm::Attribute::NoRedZone);
+ if (CodeGenOpts.IndirectTlsSegRefs)
+FuncAttrs.addAttribute("indirect-tls-seg-refs");
if (CodeGenOpts.NoImplicitFloat)
FuncAttrs.addAttribute(llvm::Attribute::NoImplicitFloat);
Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=344739=344738=344739=diff
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
nruslan added a comment. In https://reviews.llvm.org/D53102#1268364, @kristina wrote: > By the way, out of curiosity is this for anything specific (alternative libc > or some user-mode-scheduling implementation)? Not nitpicking, just curious > since it's an interesting topic in general and it's frustrating that the > architecture is so limited in terms of registers that can be used for > TLS/related data unlike newer ARM/ARM64 architectures. > > Also FWIW `%gs` is generally free to use under x86_64 Linux which is where I > usually place my thread control blocks which doesn't interfere with libc > which uses `%fs`. (Just started build/test job, waiting on that for now) @kristina : Yes, there are some recent use cases mentioned in the bug report (see the link above). Also, I know that it was used for x86 (32-bit only) Xen guests where it would be recommended to compile an OS with this flag. I also used it in my fork of the NPTL pthread library in VirtuOS to support M:N threading model: http://sigops.org/sosp/sosp13/papers/p116-nikolaev.pdf https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53398: [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Looks good! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53398: [clangd] Enforce rules around "initialize" request, and create ClangdServer lazily.
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
LSP is a slightly awkward map to C++ object lifetimes: the initialize request
is part of the protocol and provides information that doesn't change over the
lifetime of the server.
Until now, we handled this by initializing ClangdServer and ClangdLSPServer
right away, and making anything that can be set in the "initialize" request
mutable.
With this patch, we create ClangdLSPServer immediately, but defer creating
ClangdServer until "initialize". This opens the door to passing the relevant
initialize params in the constructor and storing them immutably.
(That change isn't actually done in this patch).
To make this safe, we have the MessageDispatcher enforce that the "initialize"
method is called before any other (as required by LSP). That way each method
handler can assume Server is initialized, as today.
As usual, while implementing this I found places where our test cases violated
the protocol.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53398
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
test/clangd/exit-with-shutdown.test
test/clangd/exit-without-shutdown.test
test/clangd/initialize-sequence.test
Index: test/clangd/initialize-sequence.test
===
--- /dev/null
+++ test/clangd/initialize-sequence.test
@@ -0,0 +1,21 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"workspace/symbol","params":{"query":"std::basic_ostringstream"}}
+# CHECK: "error": {
+# CHECK-NEXT:"code": -32002
+# CHECK-NEXT:"message": "server not initialized"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 0,
+---
+{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","id":2,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+# CHECK: "error": {
+# CHECK-NEXT:"code": -32600
+# CHECK-NEXT:"message": "server already initialized"
+# CHECK-NEXT: },
+# CHECK-NEXT: "id": 2,
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
+
Index: test/clangd/exit-without-shutdown.test
===
--- test/clangd/exit-without-shutdown.test
+++ test/clangd/exit-without-shutdown.test
@@ -1,2 +1,4 @@
# RUN: not clangd -lit-test < %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
{"jsonrpc":"2.0","method":"exit"}
Index: test/clangd/exit-with-shutdown.test
===
--- test/clangd/exit-with-shutdown.test
+++ test/clangd/exit-with-shutdown.test
@@ -1,4 +1,6 @@
# RUN: clangd -lit-test < %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
{"jsonrpc":"2.0","id":3,"method":"shutdown"}
---
{"jsonrpc":"2.0","method":"exit"}
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -90,7 +90,7 @@
/// defaults and -resource-dir compiler flag).
/// If None, ClangdServer calls CompilerInvocation::GetResourcePath() to
/// obtain the standard resource directory.
-llvm::Optional ResourceDir = llvm::None;
+llvm::Optional ResourceDir = llvm::None;
/// Time to wait after a new file version before computing diagnostics.
std::chrono::steady_clock::duration UpdateDebounce =
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -103,7 +103,7 @@
DiagnosticsConsumer ,
const Options )
: CDB(CDB), DiagConsumer(DiagConsumer), FSProvider(FSProvider),
- ResourceDir(Opts.ResourceDir ? Opts.ResourceDir->str()
+ ResourceDir(Opts.ResourceDir ? *Opts.ResourceDir
: getStandardResourceDir()),
DynamicIdx(Opts.BuildDynamicSymbolIndex
? new FileIndex(Opts.URISchemes,
Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -183,12 +183,10 @@
// Store of the current versions of the open documents.
DraftStore DraftMgr;
- // Server must be the last member of the class to allow its destructor to exit
- // the worker thread that may otherwise run an async callback on partially
- // destructed instance of ClangdLSPServer.
- // Set in construtor and destroyed when run()
[PATCH] D53397: [MinGW] Link to correct openmp library
SquallATF created this revision.
Herald added subscribers: cfe-commits, guansong.
Repository:
rC Clang
https://reviews.llvm.org/D53397
Files:
lib/Driver/ToolChains/MinGW.cpp
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -220,8 +220,24 @@
CmdArgs.push_back("-lssp_nonshared");
CmdArgs.push_back("-lssp");
}
- if (Args.hasArg(options::OPT_fopenmp))
-CmdArgs.push_back("-lgomp");
+
+ if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+ options::OPT_fno_openmp, false)) {
+switch (TC.getDriver().getOpenMPRuntime(Args)) {
+case Driver::OMPRT_OMP:
+ CmdArgs.push_back("-lomp");
+ break;
+case Driver::OMPRT_IOMP5:
+ CmdArgs.push_back("-liomp5md");
+ break;
+case Driver::OMPRT_GOMP:
+ CmdArgs.push_back("-lgomp");
+ break;
+case Driver::OMPRT_Unknown:
+ // Already diagnosed.
+ break;
+}
+ }
AddLibGCC(Args, CmdArgs);
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -220,8 +220,24 @@
CmdArgs.push_back("-lssp_nonshared");
CmdArgs.push_back("-lssp");
}
- if (Args.hasArg(options::OPT_fopenmp))
-CmdArgs.push_back("-lgomp");
+
+ if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+ options::OPT_fno_openmp, false)) {
+switch (TC.getDriver().getOpenMPRuntime(Args)) {
+case Driver::OMPRT_OMP:
+ CmdArgs.push_back("-lomp");
+ break;
+case Driver::OMPRT_IOMP5:
+ CmdArgs.push_back("-liomp5md");
+ break;
+case Driver::OMPRT_GOMP:
+ CmdArgs.push_back("-lgomp");
+ break;
+case Driver::OMPRT_Unknown:
+ // Already diagnosed.
+ break;
+}
+ }
AddLibGCC(Args, CmdArgs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53395: [OPENMP] Move OMPClausePrinter to OpenMPClause.h/OpenMPClause.cpp - NFC
ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang https://reviews.llvm.org/D53395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
lebedev.ri added a comment. In https://reviews.llvm.org/D52670#1268347, @aaron.ballman wrote: > In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > > > - Apply minor wording nits. > > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not > > **anything** else (not even `llu`). > > > I'll find out about the DCL16-C recommendation, as I suspect the intent is to > cover `lu` and `llu` but not `ul` and `ull`. I agree, i've thought so too. That will open an interesting question: in `lu`, `l` should be upper-case. What about `u`? We can't keep it as-is. We will either consistently upper-case it, or consistently lower-case it. I.e. given `[lL][uU]`, should we *always* produce `Lu`, or `LU`? > I've pinged the authors Thank you. > and can hopefully get an answer quickly, but if not, I'm fine with fixing > that after the patch goes in. Thank you. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
lebedev.ri added a comment. In https://reviews.llvm.org/D52771#1268367, @aaron.ballman wrote: > LGTM aside from a minor nit. Thank you for the review! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor nit. Comment at: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp:84 +ClangTidyOptions Options; +auto = Options.CheckOptions; + Don't use `auto` here. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
kristina added a comment. By the way, out of curiosity is this for anything specific (alternative libc or some user-mode-scheduling implementation)? Not nitpicking, just curious since it's an interesting topic in general and it's frustrating that the architecture is so limited in terms of registers that can be used for TLS/related data unlike newer ARM/ARM64 architectures. Also FWIW `%gs` is generally free to use under x86_64 Linux which is where I usually place my thread control blocks which doesn't interfere with libc which uses `%fs`. (Just started build/test job, waiting on that for now) https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)
aaron.ballman added a comment. In https://reviews.llvm.org/D52670#1268170, @lebedev.ri wrote: > - Apply minor wording nits. > - For `cert-dcl16-c`, **only** consider `L`, `LL` suffixes, not **anything** > else (not even `llu`). I'll find out about the DCL16-C recommendation, as I suspect the intent is to cover `lu` and `llu` but not `ul` and `ull`. I've pinged the authors and can hopefully get an answer quickly, but if not, I'm fine with fixing that after the patch goes in. Comment at: clang-tidy/cert/CERTTidyModule.cpp:87 +ClangTidyOptions Options; +auto = Options.CheckOptions; +Opts["cert-dcl16-c.NewSuffixes"] = "L;LL"; Don't use `auto` here. Comment at: docs/clang-tidy/checks/readability-uppercase-literal-suffix.rst:6 + +`cert-dcl16-c` redirects here as an alias for this check. + You should consider calling out the CERT behavioral differences. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52988: [analyzer][PlistMacroExpansion] Part 5.: Support for # and ##
Szelethus updated this revision to Diff 170073.
Szelethus added a comment.
Fixed an issue where if `##` had spaces around it, those spaces weren't removed.
https://reviews.llvm.org/D52988
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -278,9 +278,17 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// TODO: Should expand correctly.
// CHECK: nameDECLARE_FUNC_AND_SET_TO_NULL
-// CHECK: expansionvoid generated_##whatever(); ptr = nullptr;
+// CHECK: expansionvoid generated_whatever(); ptr = nullptr;
+
+void macroArgContainsHashHashInStringTest() {
+ int *a;
+ TO_NULL_AND_PRINT(a, "Will this ## cause a crash?");
+ *a = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: nameTO_NULL_AND_PRINT
+// CHECK: expansiona = 0; print( Will this ## cause a crash?)
#define PRINT_STR(str, ptr) \
print(#str); \
@@ -292,6 +300,30 @@
*ptr = 5; // expected-warning{{Dereference of null pointer}}
}
-// TODO: Should expand correctly.
// CHECK: namePRINT_STR
-// CHECK: expansionprint(#Hello); ptr = nullptr
+// CHECK: expansionprint(Hello); ptr = nullptr
+
+void macroArgContainsHashInStringTest() {
+ int *a;
+ TO_NULL_AND_PRINT(a, "Will this # cause a crash?");
+ *a = 5; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: nameTO_NULL_AND_PRINT
+// CHECK: expansiona = 0; print( Will this # cause a crash?)
+
+#define CONCAT(a, b, c) \
+ a ## _ ## b ## _ ## c
+
+void CONCAT(llvm, clang, ento)(int **ptr) {
+ *ptr = nullptr;
+}
+
+void spaceAroundHashHashTest() {
+ int* a;
+ CONCAT(llvm, clang, ento)();
+ *a = 3; // expected-warning{{Dereference of null pointer}}
+}
+
+// CHECK: nameCONCAT
+// CHECK: expansionllvm_clang_ento
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -3755,7 +3755,7 @@
file0
nameDECLARE_FUNC_AND_SET_TO_NULL
- expansionvoid generated_##whatever(); ptr = nullptr;
+ expansionvoid generated_whatever(); ptr = nullptr;
kindevent
@@ -3887,25 +3887,192 @@
start
- line290
+ line285
col3
file0
- line290
+ line285
col5
file0
end
- line291
+ line286
col3
file0
- line291
+ line286
+ col19
+ file0
+
+
+
+
+
+
+ kindmacro_expansion
+ location
+
+ line286
+ col3
+ file0
+
+ nameTO_NULL_AND_PRINT
+ expansiona = 0; print( Will this ## cause a crash?)
+
+
+ kindevent
+ location
+
+ line286
+ col3
+ file0
+
+ ranges
+
+
+
+ line286
+ col3
+ file0
+
+
+ line286
+ col53
+ file0
+
+
+
+ depth0
+ extended_message
+ Null pointer value stored to a
+ message
+ Null pointer value stored to a
+
+
+ kindcontrol
+ edges
+
+
+start
+
+
+ line287
+ col3
+ file0
+
+
+ line287
+ col3
+ file0
+
+
+end
+
+
+ line287
+ col6
+ file0
+
+
+ line287
+ col6
+ file0
+
+
+
+
+
+
+ kindevent
+ location
+
+ line287
+ col6
+ file0
+
+ ranges
+
+
+
+ line287
+ col4
+ file0
+
+
+ line287
+ col4
+ file0
+
+
+
+ depth0
+ extended_message
+ Dereference of null pointer (loaded from variable a)
+ message
+ Dereference of null pointer (loaded from variable a)
+
+
+ descriptionDereference of null pointer (loaded from variable a)
+ categoryLogic error
+ typeDereference of null pointer
+ check_namecore.NullDereference
+
+ issue_hash_content_of_line_in_context6817572ced27cb7d28fc87b2aba75fb4
+ issue_context_kindfunction
+ issue_contextmacroArgContainsHashHashInStringTest
+ issue_hash_function_offset3
[PATCH] D53392: [RISCV] Collect library directories and triples for riscv64 triple too
edward-jones created this revision.
edward-jones added a reviewer: asb.
Herald added subscribers: cfe-commits, jocewei, PkmX, rkruppe, the_o,
brucehoult, MartinMosbeck, rogfer01, mgrang, zzheng, jrtc27, shiva0217,
kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar, arichardson,
emaste.
Herald added a reviewer: espindola.
When setting up library and tools paths when detecting an accompanying GCC
installation only riscv32 was handled. As a consequence when targetting riscv64
neither the linker nor libraries would be found. This adds handling and tests
for riscv64.
Repository:
rC Clang
https://reviews.llvm.org/D53392
Files:
lib/Driver/ToolChains/Gnu.cpp
test/Driver/Inputs/basic_riscv64_tree/bin/riscv64-unknown-elf-ld
test/Driver/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/crtbegin.o
test/Driver/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/crtend.o
test/Driver/Inputs/basic_riscv64_tree/riscv64-unknown-elf/include/c++/8.0.1/.keep
test/Driver/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib/crt0.o
test/Driver/riscv64-toolchain.c
Index: test/Driver/riscv64-toolchain.c
===
--- test/Driver/riscv64-toolchain.c
+++ test/Driver/riscv64-toolchain.c
@@ -3,6 +3,102 @@
// RUN: %clang %s -### -no-canonical-prefixes -target riscv64 2>&1 | FileCheck -check-prefix=CC1 %s
// CC1: clang{{.*}} "-cc1" "-triple" "riscv64"
+// RUN: %clang %s -### -no-canonical-prefixes \
+// RUN: -target riscv64-unknown-elf \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN: --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 \
+// RUN: | FileCheck -check-prefix=C-RV64-BAREMETAL-LP64 %s
+
+// C-RV64-BAREMETAL-LP64: "-fuse-init-array"
+// C-RV64-BAREMETAL-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../bin{{/|}}riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-LP64: "--sysroot={{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf"
+// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib{{/|}}crt0.o"
+// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
+// C-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// C-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
+
+// RUN: %clang %s -### -no-canonical-prefixes \
+// RUN: -target riscv64-unknown-elf \
+// RUN: --sysroot= \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree 2>&1 \
+// RUN: | FileCheck -check-prefix=C-RV64-BAREMETAL-NOSYSROOT-LP64 %s
+
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-fuse-init-array"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../bin{{/|}}riscv64-unknown-elf-ld"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../riscv64-unknown-elf/lib{{/|}}crt0.o"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../riscv64-unknown-elf{{/|}}lib"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// C-RV64-BAREMETAL-NOSYSROOT-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
+
+// RUN: %clangxx %s -### -no-canonical-prefixes \
+// RUN: -target riscv64-unknown-elf -stdlib=libstdc++ \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN: --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 \
+// RUN: | FileCheck -check-prefix=CXX-RV64-BAREMETAL-LP64 %s
+
+// CXX-RV64-BAREMETAL-LP64: "-fuse-init-array"
+// CXX-RV64-BAREMETAL-LP64: "-internal-isystem" "{{.*}}Inputs/basic_riscv64_tree/riscv64-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV64-BAREMETAL-LP64: "{{.*}}Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1/../../../../bin{{/|}}riscv64-unknown-elf-ld"
+// CXX-RV64-BAREMETAL-LP64: "--sysroot={{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf"
+// CXX-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib{{/|}}crt0.o"
+// CXX-RV64-BAREMETAL-LP64: "{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// CXX-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/riscv64-unknown-elf/lib"
+// CXX-RV64-BAREMETAL-LP64: "-L{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1"
[PATCH] D53387: [clangd] Lay JSONRPCDispatcher to rest.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344737: [clangd] Lay JSONRPCDispatcher to rest. (authored
by sammccall, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53387?vs=170038=170072#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53387
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatcher.h
clangd/ProtocolHandlers.cpp
clangd/ProtocolHandlers.h
clangd/TUScheduler.cpp
clangd/tool/ClangdMain.cpp
test/clangd/crash-non-added-files.test
test/clangd/delimited-input-comment-at-the-end.test
test/clangd/fixits-command.test
test/clangd/rename.test
test/clangd/spaces-in-delimited-input.test
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -9,13 +9,14 @@
#include "ClangdLSPServer.h"
#include "Diagnostics.h"
-#include "JSONRPCDispatcher.h"
#include "SourceCode.h"
+#include "Trace.h"
#include "URI.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
using namespace clang::clangd;
using namespace clang;
@@ -80,7 +81,179 @@
} // namespace
-void ClangdLSPServer::onInitialize(InitializeParams ) {
+// MessageHandler dispatches incoming LSP messages.
+// It handles cross-cutting concerns:
+// - serializes/deserializes protocol objects to JSON
+// - logging of inbound messages
+// - cancellation handling
+// - basic call tracing
+class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
+public:
+ MessageHandler(ClangdLSPServer ) : Server(Server) {}
+
+ bool onNotify(StringRef Method, json::Value Params) override {
+log("<-- {0}", Method);
+if (Method == "exit")
+ return false;
+if (Method == "$/cancelRequest")
+ onCancel(std::move(Params));
+else if (auto Handler = Notifications.lookup(Method))
+ Handler(std::move(Params));
+else
+ log("unhandled notification {0}", Method);
+return true;
+ }
+
+ bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
+log("<-- {0}({1})", Method, ID);
+if (auto Handler = Calls.lookup(Method))
+ Handler(std::move(Params), std::move(ID));
+else
+ Server.reply(ID, llvm::make_error("method not found",
+ ErrorCode::MethodNotFound));
+return true;
+ }
+
+ bool onReply(json::Value ID, Expected Result) override {
+// We ignore replies, just log them.
+if (Result)
+ log("<-- reply({0})", ID);
+else
+ log("<-- reply({0}) error: {1}", ID, llvm::toString(Result.takeError()));
+return true;
+ }
+
+ // Bind an LSP method name to a call.
+ template
+ void bind(const char *Method,
+void (ClangdLSPServer::*Handler)(const Param &, Callback)) {
+Calls[Method] = [Method, Handler, this](json::Value RawParams,
+json::Value ID) {
+ Param P;
+ if (!fromJSON(RawParams, P)) {
+elog("Failed to decode {0} request.", Method);
+Server.reply(ID, make_error("failed to decode request",
+ ErrorCode::InvalidRequest));
+return;
+ }
+ trace::Span Tracer(Method);
+ SPAN_ATTACH(Tracer, "Params", RawParams);
+ auto *Trace = Tracer.Args; // We attach reply from another thread.
+ // Calls can be canceled by the client. Add cancellation context.
+ WithContext WithCancel(cancelableRequestContext(ID));
+ // FIXME: this function should assert it's called exactly once.
+ (Server.*Handler)(P, [this, ID, Trace](llvm::Expected Result) {
+if (Result) {
+ if (Trace)
+(*Trace)["Reply"] = *Result;
+ Server.reply(ID, json::Value(std::move(*Result)));
+} else {
+ auto Err = Result.takeError();
+ if (Trace)
+(*Trace)["Error"] = llvm::to_string(Err);
+ Server.reply(ID, std::move(Err));
+}
+ });
+};
+ }
+
+ // Bind an LSP method name to a notification.
+ template
+ void bind(const char *Method,
+void (ClangdLSPServer::*Handler)(const Param &)) {
+Notifications[Method] = [Method, Handler, this](json::Value RawParams) {
+ Param P;
+ if (!fromJSON(RawParams, P)) {
+elog("Failed to decode {0} request.", Method);
+return;
+ }
+ trace::Span Tracer(Method);
+ SPAN_ATTACH(Tracer, "Params", RawParams);
+ (Server.*Handler)(P);
+};
+ }
+
+private:
+ llvm::StringMap> Notifications;
+ llvm::StringMap> Calls;
+
+ // Method calls may be cancelled by ID, so keep track of their state.
+ // This needs a mutex: handlers may finish on a different thread, and
[clang-tools-extra] r344737 - [clangd] Lay JSONRPCDispatcher to rest.
Author: sammccall
Date: Thu Oct 18 05:32:04 2018
New Revision: 344737
URL: http://llvm.org/viewvc/llvm-project?rev=344737=rev
Log:
[clangd] Lay JSONRPCDispatcher to rest.
Summary:
Most of its functionality is moved into ClangdLSPServer.
The decoupling between JSONRPCDispatcher, ProtocolCallbacks, ClangdLSPServer
was never real, and only served to obfuscate.
Some previous implicit/magic stuff is now explicit:
- the return type of LSP method calls are now in the signature
- no more reply() that gets the ID using global context magic
- arg tracing no longer relies on RequestArgs::stash context magic either
This is mostly refactoring, but some deliberate fixes while here:
- LSP method params are now by const reference
- notifications and calls are now distinct namespaces.
(some tests had protocol errors and needed updating)
- we now reply to calls we failed to decode
- outgoing calls use distinct IDs
A few error codes and message IDs changed in unimportant ways (see tests).
Reviewers: ioeric
Subscribers: mgorny, ilya-biryukov, javed.absar, MaskRay, jkorous, arphaman,
jfb, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53387
Removed:
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/test/clangd/crash-non-added-files.test
clang-tools-extra/trunk/test/clangd/delimited-input-comment-at-the-end.test
clang-tools-extra/trunk/test/clangd/fixits-command.test
clang-tools-extra/trunk/test/clangd/rename.test
clang-tools-extra/trunk/test/clangd/spaces-in-delimited-input.test
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=344737=344736=344737=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Oct 18 05:32:04 2018
@@ -25,11 +25,9 @@ add_clang_library(clangDaemon
FuzzyMatch.cpp
GlobalCompilationDatabase.cpp
Headers.cpp
- JSONRPCDispatcher.cpp
JSONTransport.cpp
Logger.cpp
Protocol.cpp
- ProtocolHandlers.cpp
Quality.cpp
RIFF.cpp
SourceCode.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=344737=344736=344737=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct 18 05:32:04 2018
@@ -9,13 +9,14 @@
#include "ClangdLSPServer.h"
#include "Diagnostics.h"
-#include "JSONRPCDispatcher.h"
#include "SourceCode.h"
+#include "Trace.h"
#include "URI.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
using namespace clang::clangd;
using namespace clang;
@@ -80,7 +81,179 @@ CompletionItemKindBitset defaultCompleti
} // namespace
-void ClangdLSPServer::onInitialize(InitializeParams ) {
+// MessageHandler dispatches incoming LSP messages.
+// It handles cross-cutting concerns:
+// - serializes/deserializes protocol objects to JSON
+// - logging of inbound messages
+// - cancellation handling
+// - basic call tracing
+class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
+public:
+ MessageHandler(ClangdLSPServer ) : Server(Server) {}
+
+ bool onNotify(StringRef Method, json::Value Params) override {
+log("<-- {0}", Method);
+if (Method == "exit")
+ return false;
+if (Method == "$/cancelRequest")
+ onCancel(std::move(Params));
+else if (auto Handler = Notifications.lookup(Method))
+ Handler(std::move(Params));
+else
+ log("unhandled notification {0}", Method);
+return true;
+ }
+
+ bool onCall(StringRef Method, json::Value Params, json::Value ID) override {
+log("<-- {0}({1})", Method, ID);
+if (auto Handler = Calls.lookup(Method))
+ Handler(std::move(Params), std::move(ID));
+else
+ Server.reply(ID, llvm::make_error("method not found",
+ ErrorCode::MethodNotFound));
+return true;
+ }
+
+ bool onReply(json::Value ID, Expected Result) override {
+// We ignore replies, just log them.
+if (Result)
+ log("<-- reply({0})", ID);
+else
+ log("<-- reply({0}) error: {1}", ID,
[PATCH] D53387: [clangd] Lay JSONRPCDispatcher to rest.
sammccall marked an inline comment as done.
sammccall added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:148
+ if (Trace)
+(*Trace)["Reply"] = *Result;
+ Server.reply(ID, json::Value(std::move(*Result)));
ioeric wrote:
> do we really want to stash all responses in trace? It sounds like it could
> get pretty spammy (e.g. with completion results).
It's a fair point, I don't know the answer. I know I've looked at the replies,
but not all that often.
It's not new behavior though (it used to be in JSONRPCDispatcher), and I'd
rather not change it in this patch.
Comment at: clangd/ClangdLSPServer.cpp:409
+
+Reply(std::move(*Items));
+ },
ioeric wrote:
> We are not translating internal errors from ClangdServer to LSP errors
> anymore. It seems fine and makes code cleaner. Just want to ask if this is
> intentional.
We used to explicitly mark some of these with the "internal error" code, and
others with the "unknown error" code, without much consistency as to why.
I've tried to preserve the error codes that were semantically significant (e.g.
invalid params).
Comment at: clangd/ClangdLSPServer.h:34
+/// The server also supports $/cancelRequest (MessageHandler provides this).
+class ClangdLSPServer : private DiagnosticsConsumer {
public:
ioeric wrote:
> Just out of curiosity, why does `DiagnosticsConsumer` implemented in
> ClangdLSPServer instead of ClangdServer?
This is `clangd::DiagnosticsConsumer`, it's exactly the callback that
ClangdServer uses to send diagnostics to ClangdLSPServer.
So ClangdServer doesn't implement it but rather accepts it, because it doesn't
know what to do with the diagnostics.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53387
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53102: Support for the mno-tls-direct-seg-refs flag
nruslan added a comment. In https://reviews.llvm.org/D53102#1268272, @kristina wrote: > If the author doesn't mind I can just apply the style fix after patching and > then rebuild and run all the relevant tests (or would you prefer to do > that?). Seems easier than a new revision for an indentation change on one > line. @kristina : Thank you! Sure, that is fine. Go ahead. https://reviews.llvm.org/D53102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344736: [clangd] Names that are not spelled in source code
are reserved. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53374
Files:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.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
@@ -83,6 +83,9 @@
IsIndexedForCodeCompletion;
}
MATCHER(Deprecated, "") { return arg.Flags & Symbol::Deprecated; }
+MATCHER(ImplementationDetail, "") {
+ return arg.Flags & Symbol::ImplementationDetail;
+}
MATCHER(RefRange, "") {
const Ref = testing::get<0>(arg);
const Range = testing::get<1>(arg);
@@ -1037,6 +1040,21 @@
AllOf(QName("TestClangd"), Not(Deprecated();
}
+TEST_F(SymbolCollectorTest, ImplementationDetail) {
+ const std::string Header = R"(
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class Public {};
+ )";
+ runSymbolCollector(Header, /**/ "");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("X_Y_Decl"), ImplementationDetail()),
+ AllOf(QName("Public"), Not(ImplementationDetail();
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
@@ -45,17 +45,34 @@
[[deprecated]]
int _f() { return _X; }
+
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class MAC {};
)cpp");
+ Header.ExtraArgs = {"-DMAC=mac_name"};
+
auto Symbols = Header.headerSymbols();
auto AST = Header.build();
SymbolQualitySignals Quality;
Quality.merge(findSymbol(Symbols, "_X"));
EXPECT_FALSE(Quality.Deprecated);
+ EXPECT_FALSE(Quality.ImplementationDetail);
EXPECT_TRUE(Quality.ReservedName);
EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
+ Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
+ Quality.ImplementationDetail = false;
+ Quality.merge(
+ CodeCompletionResult((AST, "mac_name"), /*Priority=*/42));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
Symbol F = findSymbol(Symbols, "_f");
F.References = 24; // TestTU doesn't count references, so fake it.
Quality = {};
@@ -182,6 +199,10 @@
ReservedName.ReservedName = true;
EXPECT_LT(ReservedName.evaluate(), Default.evaluate());
+ SymbolQualitySignals ImplementationDetail;
+ ImplementationDetail.ImplementationDetail = true;
+ EXPECT_LT(ImplementationDetail.evaluate(), Default.evaluate());
+
SymbolQualitySignals WithReferences, ManyReferences;
WithReferences.References = 20;
ManyReferences.References = 1000;
Index: clang-tools-extra/trunk/clangd/Quality.h
===
--- clang-tools-extra/trunk/clangd/Quality.h
+++ clang-tools-extra/trunk/clangd/Quality.h
@@ -57,6 +57,7 @@
bool Deprecated = false;
bool ReservedName = false; // __foo, _Foo are usually implementation details.
// FIXME: make these findable once user types _.
+ bool ImplementationDetail = false;
unsigned References = 0;
enum SymbolCategory {
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
@@ -536,6 +536,8 @@
if (isIndexedForCodeCompletion(ND, Ctx))
S.Flags |= Symbol::IndexedForCodeCompletion;
+ if (isImplementationDetail())
+S.Flags |= Symbol::ImplementationDetail;
S.SymInfo = index::getSymbolInfo();
std::string FileURI;
if (auto DeclLoc = getTokenLocation(findNameLoc(), SM, Opts,
Index: clang-tools-extra/trunk/clangd/index/Index.h
===
--- clang-tools-extra/trunk/clangd/index/Index.h
+++
[clang-tools-extra] r344736 - [clangd] Names that are not spelled in source code are reserved.
Author: ioeric
Date: Thu Oct 18 05:23:05 2018
New Revision: 344736
URL: http://llvm.org/viewvc/llvm-project?rev=344736=rev
Log:
[clangd] Names that are not spelled in source code are reserved.
Summary:
These are often not expected to be used directly e.g.
```
TEST_F(Fixture, X) {
^ // "Fixture_X_Test" expanded in the macro should be down ranked.
}
```
Only doing this for sema for now, as such symbols are mostly coming from sema
e.g. gtest macros expanded in the main file. We could also add a similar field
for the index symbol.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D53374
Modified:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/Quality.cpp
clang-tools-extra/trunk/clangd/Quality.h
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/QualityTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=344736=344735=344736=diff
==
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Thu Oct 18 05:23:05 2018
@@ -11,6 +11,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Index/USRGeneration.h"
@@ -18,25 +19,34 @@ namespace clang {
namespace clangd {
using namespace llvm;
-SourceLocation findNameLoc(const clang::Decl* D) {
- const auto& SM = D->getASTContext().getSourceManager();
+// Returns true if the complete name of decl \p D is spelled in the source
code.
+// This is not the case for
+// * symbols formed via macro concatenation, the spelling location will
+// be ""
+// * symbols controlled and defined by a compile command-line option
+// `-DName=foo`, the spelling location will be "".
+bool isSpelledInSourceCode(const Decl *D) {
+ const auto = D->getASTContext().getSourceManager();
+ auto Loc = D->getLocation();
// FIXME: Revisit the strategy, the heuristic is limitted when handling
// macros, we should use the location where the whole definition occurs.
- SourceLocation SpellingLoc = SM.getSpellingLoc(D->getLocation());
- if (D->getLocation().isMacroID()) {
-std::string PrintLoc = SpellingLoc.printToString(SM);
+ if (Loc.isMacroID()) {
+std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
if (llvm::StringRef(PrintLoc).startswith("")) {
- // We use the expansion location for the following symbols, as spelling
- // locations of these symbols are not interesting to us:
- // * symbols formed via macro concatenation, the spelling location will
- // be ""
- // * symbols controlled and defined by a compile command-line option
- // `-DName=foo`, the spelling location will be "".
- SpellingLoc = SM.getExpansionRange(D->getLocation()).getBegin();
-}
+llvm::StringRef(PrintLoc).startswith(""))
+ return false;
}
- return SpellingLoc;
+ return true;
+}
+
+bool isImplementationDetail(const Decl *D) { return !isSpelledInSourceCode(D);
}
+
+SourceLocation findNameLoc(const clang::Decl* D) {
+ const auto = D->getASTContext().getSourceManager();
+ if (!isSpelledInSourceCode(D))
+// Use the expansion location as spelling location is not interesting.
+return SM.getExpansionRange(D->getLocation()).getBegin();
+ return SM.getSpellingLoc(D->getLocation());
}
std::string printQualifiedName(const NamedDecl ) {
Modified: clang-tools-extra/trunk/clangd/AST.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=344736=344735=344736=diff
==
--- clang-tools-extra/trunk/clangd/AST.h (original)
+++ clang-tools-extra/trunk/clangd/AST.h Thu Oct 18 05:23:05 2018
@@ -24,6 +24,11 @@ class Decl;
namespace clangd {
+/// Returns true if the declaration is considered implementation detail based
on
+/// heuristics. For example, a declaration whose name is not explicitly spelled
+/// in code is considered implementation detail.
+bool isImplementationDetail(const Decl *D);
+
/// Find the identifier source location of the given D.
///
/// The returned location is usually the spelling location where the name of
the
Modified: clang-tools-extra/trunk/clangd/Quality.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=344736=344735=344736=diff
==
---
[PATCH] D53374: [clangd] Names that are not spelled in source code are reserved.
ioeric updated this revision to Diff 170068.
ioeric marked an inline comment as done.
ioeric added a comment.
- add isImplementationDetail helper
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53374
Files:
clangd/AST.cpp
clangd/AST.h
clangd/Quality.cpp
clangd/Quality.h
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
unittests/clangd/QualityTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -83,6 +83,9 @@
IsIndexedForCodeCompletion;
}
MATCHER(Deprecated, "") { return arg.Flags & Symbol::Deprecated; }
+MATCHER(ImplementationDetail, "") {
+ return arg.Flags & Symbol::ImplementationDetail;
+}
MATCHER(RefRange, "") {
const Ref = testing::get<0>(arg);
const Range = testing::get<1>(arg);
@@ -1037,6 +1040,21 @@
AllOf(QName("TestClangd"), Not(Deprecated();
}
+TEST_F(SymbolCollectorTest, ImplementationDetail) {
+ const std::string Header = R"(
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class Public {};
+ )";
+ runSymbolCollector(Header, /**/ "");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("X_Y_Decl"), ImplementationDetail()),
+ AllOf(QName("Public"), Not(ImplementationDetail();
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/QualityTests.cpp
===
--- unittests/clangd/QualityTests.cpp
+++ unittests/clangd/QualityTests.cpp
@@ -45,17 +45,34 @@
[[deprecated]]
int _f() { return _X; }
+
+#define DECL_NAME(x, y) x##_##y##_Decl
+#define DECL(x, y) class DECL_NAME(x, y) {};
+DECL(X, Y); // X_Y_Decl
+
+class MAC {};
)cpp");
+ Header.ExtraArgs = {"-DMAC=mac_name"};
+
auto Symbols = Header.headerSymbols();
auto AST = Header.build();
SymbolQualitySignals Quality;
Quality.merge(findSymbol(Symbols, "_X"));
EXPECT_FALSE(Quality.Deprecated);
+ EXPECT_FALSE(Quality.ImplementationDetail);
EXPECT_TRUE(Quality.ReservedName);
EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
+ Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
+ Quality.ImplementationDetail = false;
+ Quality.merge(
+ CodeCompletionResult((AST, "mac_name"), /*Priority=*/42));
+ EXPECT_TRUE(Quality.ImplementationDetail);
+
Symbol F = findSymbol(Symbols, "_f");
F.References = 24; // TestTU doesn't count references, so fake it.
Quality = {};
@@ -182,6 +199,10 @@
ReservedName.ReservedName = true;
EXPECT_LT(ReservedName.evaluate(), Default.evaluate());
+ SymbolQualitySignals ImplementationDetail;
+ ImplementationDetail.ImplementationDetail = true;
+ EXPECT_LT(ImplementationDetail.evaluate(), Default.evaluate());
+
SymbolQualitySignals WithReferences, ManyReferences;
WithReferences.References = 20;
ManyReferences.References = 1000;
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -536,6 +536,8 @@
if (isIndexedForCodeCompletion(ND, Ctx))
S.Flags |= Symbol::IndexedForCodeCompletion;
+ if (isImplementationDetail())
+S.Flags |= Symbol::ImplementationDetail;
S.SymInfo = index::getSymbolInfo();
std::string FileURI;
if (auto DeclLoc = getTokenLocation(findNameLoc(), SM, Opts,
Index: clangd/index/Index.h
===
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -244,6 +244,8 @@
IndexedForCodeCompletion = 1 << 0,
/// Indicates if the symbol is deprecated.
Deprecated = 1 << 1,
+// Symbol is an implementation detail.
+ImplementationDetail = 1 << 2,
};
SymbolFlag Flags = SymbolFlag::None;
Index: clangd/Quality.h
===
--- clangd/Quality.h
+++ clangd/Quality.h
@@ -57,6 +57,7 @@
bool Deprecated = false;
bool ReservedName = false; // __foo, _Foo are usually implementation details.
// FIXME: make these findable once user types _.
+ bool ImplementationDetail = false;
unsigned References = 0;
enum SymbolCategory {
Index: clangd/Quality.cpp
===
--- clangd/Quality.cpp
+++ clangd/Quality.cpp
@@ -7,6 +7,7 @@
//
//===--===//
#include "Quality.h"
+#include "AST.h"
#include "FileDistance.h"
#include "URI.h"
#include
[PATCH] D53391: [clangd] Embed fixes as CodeAction, instead of clangd_fixes. Clean up serialization.
sammccall created this revision.
sammccall added reviewers: hokein, arphaman.
Herald added subscribers: cfe-commits, kadircet, jkorous, MaskRay, ioeric,
ilya-biryukov.
CodeAction provides us with a standard way of representing fixes inline, so
use it, replacing our existing ad-hoc extension.
After this, it's easy to serialize diagnostics using the structured
toJSON/Protocol.h mechanism rather than assembling JSON ad-hoc.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53391
Files:
clangd/ClangdLSPServer.cpp
clangd/Diagnostics.cpp
clangd/Diagnostics.h
clangd/Protocol.cpp
clangd/Protocol.h
test/clangd/fixits-embed-in-diagnostic.test
unittests/clangd/ClangdUnitTests.cpp
Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -199,11 +199,13 @@
// Transform dianostics and check the results.
std::vector>> LSPDiags;
- toLSPDiags(D, [&](clangd::Diagnostic LSPDiag,
-llvm::ArrayRef Fixes) {
-LSPDiags.push_back({std::move(LSPDiag),
-std::vector(Fixes.begin(), Fixes.end())});
- });
+ toLSPDiags(
+ D, URIForFile("/path/to/foo/bar/main.cpp"), ClangdDiagnosticOptions(),
+ [&](clangd::Diagnostic LSPDiag, llvm::ArrayRef Fixes) {
+LSPDiags.push_back(
+{std::move(LSPDiag),
+ std::vector(Fixes.begin(), Fixes.end())});
+ });
EXPECT_THAT(
LSPDiags,
Index: test/clangd/fixits-embed-in-diagnostic.test
===
--- test/clangd/fixits-embed-in-diagnostic.test
+++ test/clangd/fixits-embed-in-diagnostic.test
@@ -1,12 +1,12 @@
# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"clangdFixSupport":true}}},"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"codeActionsInline":true}}},"trace":"off"}}
---
{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}}
-# CHECK:"method": "textDocument/publishDiagnostics",
-# CHECK-NEXT:"params": {
-# CHECK-NEXT: "diagnostics": [
+# CHECK: "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT: "params": {
+# CHECK-NEXT:"diagnostics": [
# CHECK-NEXT: {
-# CHECK-NEXT:"clangd_fixes": [
+# CHECK-NEXT:"codeActions": [
# CHECK-NEXT: {
# CHECK-NEXT:"edit": {
# CHECK-NEXT: "changes": {
@@ -27,6 +27,7 @@
# CHECK-NEXT:]
# CHECK-NEXT: }
# CHECK-NEXT:},
+# CHECK-NEXT:"kind": "quickfix",
# CHECK-NEXT:"title": "change 'union' to 'struct'"
# CHECK-NEXT: }
# CHECK-NEXT:],
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -323,9 +323,8 @@
/// workspace.symbol.symbolKind.valueSet
llvm::Optional WorkspaceSymbolKinds;
- /// Whether the client accepts diagnostics with fixes attached using the
- /// "clangd_fixes" extension.
- /// textDocument.publishDiagnostics.clangdFixSupport
+ /// Whether the client accepts diagnostics with codeActions attached inline.
+ /// textDocument.publishDiagnostics.codeActionsInline.
bool DiagnosticFixes = false;
/// Whether the client accepts diagnostics with category attached to it
@@ -536,6 +535,7 @@
};
bool fromJSON(const llvm::json::Value &, DocumentSymbolParams &);
+struct CodeAction;
struct Diagnostic {
/// The range at which the message applies.
Range range;
@@ -560,7 +560,12 @@
/// An LSP extension that's used to send the name of the category over to the
/// client. The category typically describes the compilation stage during
/// which the issue was produced, e.g. "Semantic Issue" or "Parse Issue".
- std::string category;
+ llvm::Optional category;
+
+ /// Clangd extension: code actions related to this diagnostic.
+ /// Only with capability textDocument.publishDiagnostics.codeActionsInline.
+ /// (These actions can also be obtained using textDocument/codeAction).
+ llvm::Optional> codeActions;
};
llvm::json::Value toJSON(const Diagnostic &);
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -210,8 +210,8 @@
if (auto *Diagnostics = TextDocument->getObject("publishDiagnostics")) {
if (auto CategorySupport = Diagnostics->getBoolean("categorySupport"))
R.DiagnosticCategory = *CategorySupport;
- if (auto ClangdFixSupport =
[PATCH] D53387: [clangd] Lay JSONRPCDispatcher to rest.
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clangd/ClangdLSPServer.cpp:148
+ if (Trace)
+(*Trace)["Reply"] = *Result;
+ Server.reply(ID, json::Value(std::move(*Result)));
do we really want to stash all responses in trace? It sounds like it could get
pretty spammy (e.g. with completion results).
Comment at: clangd/ClangdLSPServer.cpp:184
+ mutable std::mutex RequestCancelersMutex;
+ llvm::StringMap> RequestCancelers;
+ unsigned NextRequestCookie = 0;
nit: while we are here, could you add some documentation for these two fields?
Comment at: clangd/ClangdLSPServer.cpp:409
+
+Reply(std::move(*Items));
+ },
We are not translating internal errors from ClangdServer to LSP errors anymore.
It seems fine and makes code cleaner. Just want to ask if this is intentional.
Comment at: clangd/ClangdLSPServer.h:34
+/// The server also supports $/cancelRequest (MessageHandler provides this).
+class ClangdLSPServer : private DiagnosticsConsumer {
public:
Just out of curiosity, why does `DiagnosticsConsumer` implemented in
ClangdLSPServer instead of ClangdServer?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53387
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
